Re: Problem with pg_upgrade's directory write check on Windows - Mailing list pgsql-hackers

Robert Haas wrote:
> On Sat, Jul 23, 2011 at 9:14 AM, Bruce Momjian <bruce@momjian.us> wrote:
> > Andrew Dunstan wrote:
> >> > We do use access() in a few other places in our code, but not for
> >> > directory permission checks.
> >> >
> >> > Any ideas on a solution? ?Will checking stat() work? ?Do I have to try
> >> > creating a dummy file and delete it?
> >>
> >> That looks like the obvious solution - it's what came to my mind even
> >> before reading this sentence.
> >
> > Well, the easy fix is to see if ALL_DUMP_FILE
> > ("pg_upgrade_dump_all.sql") exists, and if so remove it, and if not,
> > create it and remove it.
> >
> > Should I fix this in pg_upgrade 9.1 for Windows or just in 9.2? ?The
> > check works fine on non-Windows.
>
> Seems worth back-patching to me.

Attached patch applied and backpatched to 9.1.  I was able to test both
code paths on my BSD machine by modifying the ifdefs.  I will have
EnterpriseDB do further testing.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
new file mode 100644
index a76c06e..3493696
*** a/contrib/pg_upgrade/exec.c
--- b/contrib/pg_upgrade/exec.c
***************
*** 16,21 ****
--- 16,24 ----
  static void check_data_dir(const char *pg_data);
  static void check_bin_dir(ClusterInfo *cluster);
  static void validate_exec(const char *dir, const char *cmdName);
+ #ifdef WIN32
+ static int win32_check_directory_write_permissions(void);
+ #endif


  /*
*************** verify_directories(void)
*** 97,113 ****

      prep_status("Checking current, bin, and data directories");

-     if (access(".", R_OK | W_OK
  #ifndef WIN32
!
!     /*
!      * Do a directory execute check only on Unix because execute permission on
!      * NTFS means "can execute scripts", which we don't care about. Also, X_OK
!      * is not defined in the Windows API.
!      */
!                | X_OK
  #endif
-                ) != 0)
          pg_log(PG_FATAL,
            "You must have read and write access in the current directory.\n");

--- 100,110 ----

      prep_status("Checking current, bin, and data directories");

  #ifndef WIN32
!     if (access(".", R_OK | W_OK | X_OK) != 0)
! #else
!     if (win32_check_directory_write_permissions() != 0)
  #endif
          pg_log(PG_FATAL,
            "You must have read and write access in the current directory.\n");

*************** verify_directories(void)
*** 119,124 ****
--- 116,147 ----
  }


+ #ifdef WIN32
+ /*
+  * win32_check_directory_write_permissions()
+  *
+  *    access() on WIN32 can't check directory permissions, so we have to
+  *    optionally create, then delete a file to check.
+  *        http://msdn.microsoft.com/en-us/library/1w06ktdy%28v=vs.80%29.aspx
+  */
+ static int
+ win32_check_directory_write_permissions(void)
+ {
+     int fd;
+
+     /*
+      *    We open a file we would normally create anyway.  We do this even in
+      *    'check' mode, which isn't ideal, but this is the best we can do.
+      */
+     if ((fd = open(GLOBALS_DUMP_FILE, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) < 0)
+         return -1;
+     close(fd);
+
+     return unlink(GLOBALS_DUMP_FILE);
+ }
+ #endif
+
+
  /*
   * check_data_dir()
   *