Thread: Re: [HACKERS] Cannot initdb in cvs tip

Re: [HACKERS] Cannot initdb in cvs tip

From
"Dave Page"
Date:

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: 19 June 2004 00:22
> To: Dave Page
> Cc: PostgreSQL-development
> Subject: Re: [HACKERS] Cannot initdb in cvs tip
>
> "Dave Page" <dpage@vale-housing.co.uk> writes:
> > I'm getting the following error when trying to initdb with CVS tip.
>
> > creating template1 database in
> C:/msys/1.0/local/pgsql/data/base/1 ...
> > ERROR:  could not open segment 1 of relation 1663/1/1255
> (target block
> > 26189776): No such file or directory
>
> The target block number is obviously broken :-(.  But maybe
> you have a build consistency problem --- did you try a make
> distclean and full rebuild?

OK, that cured that one - thanks.

> > although it says it's clearing the contents of the directory, in
> > actual fact it leaves the directory structure in place, thus a
> > subsequent initdb will not run without a manual clearup.
>
> Hm.  The rmtree() function in initdb.c is responsible for
> this, and I see it has WIN32-specific behavior, which is
> evidently wrong.
> Can you recommend a fix?

The current solution does an "rmdir /q /s $PGDATA" if the datadir was
created, and "del /q /s $PGDATA" if the directory already existed. The
second case  will not work, as del will not remove directories. AFAICS,
there is no easy way to do this using system() as rmdir won't accept
wildcards, so we can't do "del $PGDATA/* && rmdir $PGDATA/*".

It seems to me that the simple answer is to put Andrew's recursive
unlink code back in (as he suggested), which Bruce removed as rm etc.
were being used in commands/dbcommands.c (which should work fine under
Windows). Patch below....

Regards, Dave

*** initdb.c.orig    Sat Jun 19 22:15:28 2004
--- initdb.c    Sat Jun 19 23:02:10 2004
***************
*** 132,137 ****
--- 132,144 ----
  static void *xmalloc(size_t size);
  static char *xstrdup(const char *s);
  static bool rmtree(char *path, bool rmtopdir);
+
+ #ifdef WIN32
+ static int  init_unlink(const char *);
+ #else
+ #define init_unlink(x) unlink( (x) )
+ #endif   /* WIN32 */
+
  static char **replace_token(char **lines, char *token, char
*replacement);
  static char **readfile(char *path);
  static void writefile(char *path, char **lines);
***************
*** 245,264 ****
  static bool
  rmtree(char *path, bool rmtopdir)
  {
!     char        buf[MAXPGPATH + 64];

! #ifndef WIN32
!     /* doesn't handle .* files, but we don't make any... */
!     snprintf(buf, sizeof(buf), "rm -rf \"%s\"%s", path,
!              rmtopdir ? "" : "/*");
! #else
!     snprintf(buf, sizeof(buf), "%s /s /q \"%s\"",
!              rmtopdir ? "rmdir" : "del", path);
! #endif

!     return !system(buf);
  }


  /*
   * make a copy of the array of lines, with token replaced by
replacement
--- 252,349 ----
  static bool
  rmtree(char *path, bool rmtopdir)
  {
!     char        filepath[MAXPGPATH];
!     DIR           *dir;
!     struct dirent *file;
!     char      **filenames;
!     char      **filename;
!     int            numnames = 0;
!     struct stat statbuf;

!     /*
!      * we copy all the names out of the directory before we start
modifying
!      * it.
!      *
!      */
!
!     dir = opendir(path);
!     if (dir == NULL)
!         return false;

!     while ((file = readdir(dir)) != NULL)
!     {
!         if (strcmp(file->d_name, ".") != 0 &&
strcmp(file->d_name, "..") != 0)
!             numnames++;
!     }
!
!     rewinddir(dir);
!
!     filenames = xmalloc((numnames + 2) * sizeof(char *));
!     numnames = 0;
!
!     while ((file = readdir(dir)) != NULL)
!     {
!         if (strcmp(file->d_name, ".") != 0 &&
strcmp(file->d_name, "..") != 0)
!             filenames[numnames++] = xstrdup(file->d_name);
!     }
!
!     filenames[numnames] = NULL;
!
!     closedir(dir);
!
!     /* now we have the names we can start removing things */
!
!     for (filename = filenames; *filename; filename++)
!     {
!         snprintf(filepath, MAXPGPATH, "%s/%s", path, *filename);
!
!         if (stat(filepath, &statbuf) != 0)
!             return false;
!
!         if (S_ISDIR(statbuf.st_mode))
!         {
!             /* call ourselves recursively for a directory */
!             if (!rmtree(filepath, true))
!                 return false;
!         }
!         else
!         {
!             if (init_unlink(filepath) != 0)
!                 return false;
!         }
!     }
!
!     if (rmtopdir)
!     {
!         if (rmdir(path) != 0)
!             return false;
!     }
!
!     return true;
  }

+ #ifdef WIN32
+
+ /* workaround for win32 unlink bug, not using logging like in
port/dirmod.c */
+
+ /* make sure we call the real unlink from MSVCRT */
+
+ #ifdef unlink
+ #undef unlink
+ #endif
+
+ static int
+ init_unlink(const char *path)
+ {
+     while (unlink(path))
+     {
+         if (errno != EACCES)
+             return -1;
+         Sleep(100);             /* ms */
+     }
+     return 0;
+ }
+ #endif   /* WIN32 */

  /*
   * make a copy of the array of lines, with token replaced by
replacement

Re: [HACKERS] Cannot initdb in cvs tip

From
John Hansen
Date:
On Sun, 2004-06-20 at 08:04, Dave Page wrote:
>
> > -----Original Message-----
> > From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> > Sent: 19 June 2004 00:22
> > To: Dave Page
> > Cc: PostgreSQL-development
> > Subject: Re: [HACKERS] Cannot initdb in cvs tip
> >
> > "Dave Page" <dpage@vale-housing.co.uk> writes:
> > > I'm getting the following error when trying to initdb with CVS tip.
> >
> > > creating template1 database in
> > C:/msys/1.0/local/pgsql/data/base/1 ...
> > > ERROR:  could not open segment 1 of relation 1663/1/1255
> > (target block
> > > 26189776): No such file or directory
> >
> > The target block number is obviously broken :-(.  But maybe
> > you have a build consistency problem --- did you try a make
> > distclean and full rebuild?
>
> OK, that cured that one - thanks.
>
> > > although it says it's clearing the contents of the directory, in
> > > actual fact it leaves the directory structure in place, thus a
> > > subsequent initdb will not run without a manual clearup.
> >
> > Hm.  The rmtree() function in initdb.c is responsible for
> > this, and I see it has WIN32-specific behavior, which is
> > evidently wrong.
> > Can you recommend a fix?
>
> The current solution does an "rmdir /q /s $PGDATA" if the datadir was
> created, and "del /q /s $PGDATA" if the directory already existed. The
> second case  will not work, as del will not remove directories. AFAICS,
> there is no easy way to do this using system() as rmdir won't accept
> wildcards, so we can't do "del $PGDATA/* && rmdir $PGDATA/*".
>
> It seems to me that the simple answer is to put Andrew's recursive
> unlink code back in (as he suggested), which Bruce removed as rm etc.
> were being used in commands/dbcommands.c (which should work fine under
> Windows). Patch below....
>

you could of course rmdir /s /q $PGDATA && mkdir $PGDATA if the purpose
is to leave the directory intact if it already existed prior to install.

Regards,

John
> Regards, Dave
>
> *** initdb.c.orig    Sat Jun 19 22:15:28 2004
> --- initdb.c    Sat Jun 19 23:02:10 2004
> ***************
> *** 132,137 ****
> --- 132,144 ----
>   static void *xmalloc(size_t size);
>   static char *xstrdup(const char *s);
>   static bool rmtree(char *path, bool rmtopdir);
> +
> + #ifdef WIN32
> + static int  init_unlink(const char *);
> + #else
> + #define init_unlink(x) unlink( (x) )
> + #endif   /* WIN32 */
> +
>   static char **replace_token(char **lines, char *token, char
> *replacement);
>   static char **readfile(char *path);
>   static void writefile(char *path, char **lines);
> ***************
> *** 245,264 ****
>   static bool
>   rmtree(char *path, bool rmtopdir)
>   {
> !     char        buf[MAXPGPATH + 64];
>
> ! #ifndef WIN32
> !     /* doesn't handle .* files, but we don't make any... */
> !     snprintf(buf, sizeof(buf), "rm -rf \"%s\"%s", path,
> !              rmtopdir ? "" : "/*");
> ! #else
> !     snprintf(buf, sizeof(buf), "%s /s /q \"%s\"",
> !              rmtopdir ? "rmdir" : "del", path);
> ! #endif
>
> !     return !system(buf);
>   }
>
>
>   /*
>    * make a copy of the array of lines, with token replaced by
> replacement
> --- 252,349 ----
>   static bool
>   rmtree(char *path, bool rmtopdir)
>   {
> !     char        filepath[MAXPGPATH];
> !     DIR           *dir;
> !     struct dirent *file;
> !     char      **filenames;
> !     char      **filename;
> !     int            numnames = 0;
> !     struct stat statbuf;
>
> !     /*
> !      * we copy all the names out of the directory before we start
> modifying
> !      * it.
> !      *
> !      */
> !
> !     dir = opendir(path);
> !     if (dir == NULL)
> !         return false;
>
> !     while ((file = readdir(dir)) != NULL)
> !     {
> !         if (strcmp(file->d_name, ".") != 0 &&
> strcmp(file->d_name, "..") != 0)
> !             numnames++;
> !     }
> !
> !     rewinddir(dir);
> !
> !     filenames = xmalloc((numnames + 2) * sizeof(char *));
> !     numnames = 0;
> !
> !     while ((file = readdir(dir)) != NULL)
> !     {
> !         if (strcmp(file->d_name, ".") != 0 &&
> strcmp(file->d_name, "..") != 0)
> !             filenames[numnames++] = xstrdup(file->d_name);
> !     }
> !
> !     filenames[numnames] = NULL;
> !
> !     closedir(dir);
> !
> !     /* now we have the names we can start removing things */
> !
> !     for (filename = filenames; *filename; filename++)
> !     {
> !         snprintf(filepath, MAXPGPATH, "%s/%s", path, *filename);
> !
> !         if (stat(filepath, &statbuf) != 0)
> !             return false;
> !
> !         if (S_ISDIR(statbuf.st_mode))
> !         {
> !             /* call ourselves recursively for a directory */
> !             if (!rmtree(filepath, true))
> !                 return false;
> !         }
> !         else
> !         {
> !             if (init_unlink(filepath) != 0)
> !                 return false;
> !         }
> !     }
> !
> !     if (rmtopdir)
> !     {
> !         if (rmdir(path) != 0)
> !             return false;
> !     }
> !
> !     return true;
>   }
>
> + #ifdef WIN32
> +
> + /* workaround for win32 unlink bug, not using logging like in
> port/dirmod.c */
> +
> + /* make sure we call the real unlink from MSVCRT */
> +
> + #ifdef unlink
> + #undef unlink
> + #endif
> +
> + static int
> + init_unlink(const char *path)
> + {
> +     while (unlink(path))
> +     {
> +         if (errno != EACCES)
> +             return -1;
> +         Sleep(100);             /* ms */
> +     }
> +     return 0;
> + }
> + #endif   /* WIN32 */
>
>   /*
>    * make a copy of the array of lines, with token replaced by
> replacement
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org

Re: [HACKERS] Cannot initdb in cvs tip

From
"Andrew Dunstan"
Date:
John Hansen said:
> On Sun, 2004-06-20 at 08:04, Dave Page wrote:
>> > > although it says it's clearing the contents of the directory, in
>> > > actual fact it leaves the directory structure in place, thus a
>> > > subsequent initdb will not run without a manual clearup.
>> >
>> > Hm.  The rmtree() function in initdb.c is responsible for
>> > this, and I see it has WIN32-specific behavior, which is
>> > evidently wrong.
>> > Can you recommend a fix?
>>
>> The current solution does an "rmdir /q /s $PGDATA" if the datadir was
>> created, and "del /q /s $PGDATA" if the directory already existed. The
>> second case  will not work, as del will not remove directories.
>> AFAICS, there is no easy way to do this using system() as rmdir won't
>> accept wildcards, so we can't do "del $PGDATA/* && rmdir $PGDATA/*".
>>
>> It seems to me that the simple answer is to put Andrew's recursive
>> unlink code back in (as he suggested), which Bruce removed as rm etc.
>> were being used in commands/dbcommands.c (which should work fine under
>> Windows). Patch below....
>>
>
> you could of course rmdir /s /q $PGDATA && mkdir $PGDATA if the purpose
> is to leave the directory intact if it already existed prior to
> install.
>

No we can't. This was discussed months ago, IIRC. The user might very well
not have the privileges necessary to delete the directory, and might not
have the privileges to recreate it if they do.

The direct recursive delete is not a lot of code, and I must confess I
*hate* having C programs calling system() for such tasks. One of my goals
in rewriting initdb in C was to avoid any calls at all to any external
program other than postgres itself.

cheers

andrew



Re: [HACKERS] Cannot initdb in cvs tip

From
Bruce Momjian
Date:
Dave, now that we are nearing beta, I think we need to correct the
initdb problem with removing the directory on Win32.  Would you code
this up as something that sits in /port/dirmod.c and have both initdb
and DROP DATABASE call the C routine rather than call rm -r/rmdir?  (I
think those are the only two.  DROP TABLESPACE?)

I wanted to keep a solution that was as native to the OS as possible,
but because we can't do that on Win32 and few people like the unix
system call to 'rm', it is time to clean it up.

One question --- why is there a sleep loop needed for unlink in your
patch?


---------------------------------------------------------------------------

Dave Page wrote:
>
>
> > -----Original Message-----
> > From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> > Sent: 19 June 2004 00:22
> > To: Dave Page
> > Cc: PostgreSQL-development
> > Subject: Re: [HACKERS] Cannot initdb in cvs tip
> >
> > "Dave Page" <dpage@vale-housing.co.uk> writes:
> > > I'm getting the following error when trying to initdb with CVS tip.
> >
> > > creating template1 database in
> > C:/msys/1.0/local/pgsql/data/base/1 ...
> > > ERROR:  could not open segment 1 of relation 1663/1/1255
> > (target block
> > > 26189776): No such file or directory
> >
> > The target block number is obviously broken :-(.  But maybe
> > you have a build consistency problem --- did you try a make
> > distclean and full rebuild?
>
> OK, that cured that one - thanks.
>
> > > although it says it's clearing the contents of the directory, in
> > > actual fact it leaves the directory structure in place, thus a
> > > subsequent initdb will not run without a manual clearup.
> >
> > Hm.  The rmtree() function in initdb.c is responsible for
> > this, and I see it has WIN32-specific behavior, which is
> > evidently wrong.
> > Can you recommend a fix?
>
> The current solution does an "rmdir /q /s $PGDATA" if the datadir was
> created, and "del /q /s $PGDATA" if the directory already existed. The
> second case  will not work, as del will not remove directories. AFAICS,
> there is no easy way to do this using system() as rmdir won't accept
> wildcards, so we can't do "del $PGDATA/* && rmdir $PGDATA/*".
>
> It seems to me that the simple answer is to put Andrew's recursive
> unlink code back in (as he suggested), which Bruce removed as rm etc.
> were being used in commands/dbcommands.c (which should work fine under
> Windows). Patch below....
>
> Regards, Dave
>
> *** initdb.c.orig    Sat Jun 19 22:15:28 2004
> --- initdb.c    Sat Jun 19 23:02:10 2004
> ***************
> *** 132,137 ****
> --- 132,144 ----
>   static void *xmalloc(size_t size);
>   static char *xstrdup(const char *s);
>   static bool rmtree(char *path, bool rmtopdir);
> +
> + #ifdef WIN32
> + static int  init_unlink(const char *);
> + #else
> + #define init_unlink(x) unlink( (x) )
> + #endif   /* WIN32 */
> +
>   static char **replace_token(char **lines, char *token, char
> *replacement);
>   static char **readfile(char *path);
>   static void writefile(char *path, char **lines);
> ***************
> *** 245,264 ****
>   static bool
>   rmtree(char *path, bool rmtopdir)
>   {
> !     char        buf[MAXPGPATH + 64];
>
> ! #ifndef WIN32
> !     /* doesn't handle .* files, but we don't make any... */
> !     snprintf(buf, sizeof(buf), "rm -rf \"%s\"%s", path,
> !              rmtopdir ? "" : "/*");
> ! #else
> !     snprintf(buf, sizeof(buf), "%s /s /q \"%s\"",
> !              rmtopdir ? "rmdir" : "del", path);
> ! #endif
>
> !     return !system(buf);
>   }
>
>
>   /*
>    * make a copy of the array of lines, with token replaced by
> replacement
> --- 252,349 ----
>   static bool
>   rmtree(char *path, bool rmtopdir)
>   {
> !     char        filepath[MAXPGPATH];
> !     DIR           *dir;
> !     struct dirent *file;
> !     char      **filenames;
> !     char      **filename;
> !     int            numnames = 0;
> !     struct stat statbuf;
>
> !     /*
> !      * we copy all the names out of the directory before we start
> modifying
> !      * it.
> !      *
> !      */
> !
> !     dir = opendir(path);
> !     if (dir == NULL)
> !         return false;
>
> !     while ((file = readdir(dir)) != NULL)
> !     {
> !         if (strcmp(file->d_name, ".") != 0 &&
> strcmp(file->d_name, "..") != 0)
> !             numnames++;
> !     }
> !
> !     rewinddir(dir);
> !
> !     filenames = xmalloc((numnames + 2) * sizeof(char *));
> !     numnames = 0;
> !
> !     while ((file = readdir(dir)) != NULL)
> !     {
> !         if (strcmp(file->d_name, ".") != 0 &&
> strcmp(file->d_name, "..") != 0)
> !             filenames[numnames++] = xstrdup(file->d_name);
> !     }
> !
> !     filenames[numnames] = NULL;
> !
> !     closedir(dir);
> !
> !     /* now we have the names we can start removing things */
> !
> !     for (filename = filenames; *filename; filename++)
> !     {
> !         snprintf(filepath, MAXPGPATH, "%s/%s", path, *filename);
> !
> !         if (stat(filepath, &statbuf) != 0)
> !             return false;
> !
> !         if (S_ISDIR(statbuf.st_mode))
> !         {
> !             /* call ourselves recursively for a directory */
> !             if (!rmtree(filepath, true))
> !                 return false;
> !         }
> !         else
> !         {
> !             if (init_unlink(filepath) != 0)
> !                 return false;
> !         }
> !     }
> !
> !     if (rmtopdir)
> !     {
> !         if (rmdir(path) != 0)
> !             return false;
> !     }
> !
> !     return true;
>   }
>
> + #ifdef WIN32
> +
> + /* workaround for win32 unlink bug, not using logging like in
> port/dirmod.c */
> +
> + /* make sure we call the real unlink from MSVCRT */
> +
> + #ifdef unlink
> + #undef unlink
> + #endif
> +
> + static int
> + init_unlink(const char *path)
> + {
> +     while (unlink(path))
> +     {
> +         if (errno != EACCES)
> +             return -1;
> +         Sleep(100);             /* ms */
> +     }
> +     return 0;
> + }
> + #endif   /* WIN32 */
>
>   /*
>    * make a copy of the array of lines, with token replaced by
> replacement
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] Cannot initdb in cvs tip

From
Andrew Dunstan
Date:

Bruce Momjian wrote:

>Dave, now that we are nearing beta, I think we need to correct the
>initdb problem with removing the directory on Win32.  Would you code
>this up as something that sits in /port/dirmod.c and have both initdb
>and DROP DATABASE call the C routine rather than call rm -r/rmdir?  (I
>think those are the only two.  DROP TABLESPACE?)
>
>

The small wrinkle here is that rmtree needs to make a copy of the file
names before it starts removing things. In the backend case that means
calling palloc() and friends - am I correct in assuming it is reasonable
to do this in whatever context happens to be current when rmtree is
called? (I promise to make it clean up nicely).

>I wanted to keep a solution that was as native to the OS as possible,
>but because we can't do that on Win32 and few people like the unix
>system call to 'rm', it is time to clean it up.
>
>One question --- why is there a sleep loop needed for unlink in your
>patch?
>
>
>
>

We will just be calling the existing pgunlink() (which has a sleep) in
the Windows cases, so this question becomes moot.

cheers

andrew

Re: [HACKERS] Cannot initdb in cvs tip

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> >I wanted to keep a solution that was as native to the OS as possible,
> >but because we can't do that on Win32 and few people like the unix
> >system call to 'rm', it is time to clean it up.
> >
> >One question --- why is there a sleep loop needed for unlink in your
> >patch?
> >
> >
> >
> >
>
> We will just be calling the existing pgunlink() (which has a sleep) in
> the Windows cases, so this question becomes moot.

Great.  Thanks.  Sorry I delayed addressing this for so long.

Should we look at replacing cp/copy in 7.6?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] Cannot initdb in cvs tip

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> The small wrinkle here is that rmtree needs to make a copy of the file
> names before it starts removing things. In the backend case that means
> calling palloc() and friends - am I correct in assuming it is reasonable
> to do this in whatever context happens to be current when rmtree is
> called?

Sure.

            regards, tom lane

Re: [HACKERS] Cannot initdb in cvs tip

From
Andrew Dunstan
Date:

Bruce Momjian wrote:

>Andrew Dunstan wrote:
>
>
>>>I wanted to keep a solution that was as native to the OS as possible,
>>>but because we can't do that on Win32 and few people like the unix
>>>system call to 'rm', it is time to clean it up.
>>>
>>>One question --- why is there a sleep loop needed for unlink in your
>>>patch?
>>>
>>>
>>>
>>>
>>>
>>>
>>We will just be calling the existing pgunlink() (which has a sleep) in
>>the Windows cases, so this question becomes moot.
>>
>>
>
>Great.  Thanks.  Sorry I delayed addressing this for so long.
>
>

Please check the enclosed patch to see if it does what you want.

>Should we look at replacing cp/copy in 7.6?
>
>
>

probably. Put it as a possible TODO maybe.

cheers

andrew


Index: src/Makefile.global.in
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/Makefile.global.in,v
retrieving revision 1.189
diff -c -r1.189 Makefile.global.in
*** src/Makefile.global.in    2 Jun 2004 21:05:52 -0000    1.189
--- src/Makefile.global.in    28 Jul 2004 17:29:31 -0000
***************
*** 340,346 ****
  #
  # substitute implementations of the C library

! LIBOBJS = @LIBOBJS@ exec.o noblock.o path.o pipe.o pgsleep.o pgstrcasecmp.o sprompt.o thread.o

  ifneq (,$(LIBOBJS))
  LIBS := -lpgport $(LIBS)
--- 340,346 ----
  #
  # substitute implementations of the C library

! LIBOBJS = @LIBOBJS@ dirmod.o exec.o noblock.o path.o pipe.o pgsleep.o pgstrcasecmp.o sprompt.o thread.o

  ifneq (,$(LIBOBJS))
  LIBS := -lpgport $(LIBS)
Index: src/backend/commands/dbcommands.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/dbcommands.c,v
retrieving revision 1.137
diff -c -r1.137 dbcommands.c
*** src/backend/commands/dbcommands.c    25 Jun 2004 21:55:53 -0000    1.137
--- src/backend/commands/dbcommands.c    28 Jul 2004 17:29:32 -0000
***************
*** 915,921 ****
      Relation rel;
      HeapScanDesc scan;
      HeapTuple tuple;
-     char buf[MAXPGPATH + 100];

      rel = heap_openr(TableSpaceRelationName, AccessShareLock);
      scan = heap_beginscan(rel, SnapshotNow, 0, NULL);
--- 915,920 ----
***************
*** 938,954 ****
              continue;
          }

! #ifndef WIN32
!         snprintf(buf, sizeof(buf), "rm -rf '%s'", dstpath);
! #else
!         snprintf(buf, sizeof(buf), "rmdir /s /q \"%s\"", dstpath);
! #endif
!         if (system(buf) != 0)
          {
              ereport(WARNING,
                  (errmsg("could not remove database directory \"%s\"",
                          dstpath),
-                  errdetail("Failing system command was: %s", buf),
                   errhint("Look in the postmaster's stderr log for more information.")));
          }

--- 937,947 ----
              continue;
          }

!         if (! rmtree(dstpath,true) )
          {
              ereport(WARNING,
                  (errmsg("could not remove database directory \"%s\"",
                          dstpath),
                   errhint("Look in the postmaster's stderr log for more information.")));
          }

Index: src/bin/initdb/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/initdb/Makefile,v
retrieving revision 1.41
diff -c -r1.41 Makefile
*** src/bin/initdb/Makefile    24 May 2004 01:01:37 -0000    1.41
--- src/bin/initdb/Makefile    28 Jul 2004 17:29:32 -0000
***************
*** 15,21 ****

  override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) $(CPPFLAGS)

! OBJS=    initdb.o exec.o

  all: submake-libpq submake-libpgport initdb

--- 15,21 ----

  override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) $(CPPFLAGS)

! OBJS=    initdb.o exec.o dirmod.o

  all: submake-libpq submake-libpgport initdb

***************
*** 25,30 ****
--- 25,33 ----
  exec.c: % : $(top_srcdir)/src/port/%
      rm -f $@ && $(LN_S) $< .

+ dirmod.c: % : $(top_srcdir)/src/port/%
+     rm -f $@ && $(LN_S) $< .
+
  install: all installdirs
      $(INSTALL_PROGRAM) initdb$(X) $(DESTDIR)$(bindir)/initdb$(X)

Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/initdb/initdb.c,v
retrieving revision 1.44
diff -c -r1.44 initdb.c
*** src/bin/initdb/initdb.c    19 Jul 2004 02:47:12 -0000    1.44
--- src/bin/initdb/initdb.c    28 Jul 2004 17:29:32 -0000
***************
*** 135,141 ****

  static void *xmalloc(size_t size);
  static char *xstrdup(const char *s);
- static bool rmtree(char *path, bool rmtopdir);
  static char **replace_token(char **lines, char *token, char *replacement);
  static char **readfile(char *path);
  static void writefile(char *path, char **lines);
--- 135,140 ----
***************
*** 241,270 ****
  }

  /*
-  * delete a directory tree recursively
-  * assumes path points to a valid directory
-  * deletes everything under path
-  * if rmtopdir is true deletes the directory too
-  */
- static bool
- rmtree(char *path, bool rmtopdir)
- {
-     char        buf[MAXPGPATH + 64];
-
- #ifndef WIN32
-     /* doesn't handle .* files, but we don't make any... */
-     snprintf(buf, sizeof(buf), "rm -rf \"%s\"%s", path,
-              rmtopdir ? "" : "/*");
- #else
-     snprintf(buf, sizeof(buf), "%s /s /q \"%s\"",
-              rmtopdir ? "rmdir" : "del", path);
- #endif
-
-     return !system(buf);
- }
-
-
- /*
   * make a copy of the array of lines, with token replaced by replacement
   * the first time it occurs on each line.
   *
--- 240,245 ----
Index: src/include/port.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/include/port.h,v
retrieving revision 1.45
diff -c -r1.45 port.h
*** src/include/port.h    23 Jul 2004 01:58:36 -0000    1.45
--- src/include/port.h    28 Jul 2004 17:29:32 -0000
***************
*** 148,153 ****
--- 148,155 ----
  #define unlink(path)        pgunlink(path)
  #endif

+ extern bool rmtree(char *path, bool rmtopdir);
+
  #ifdef WIN32

  /* open() replacement to allow delete of held files */
Index: src/port/dirmod.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/port/dirmod.c,v
retrieving revision 1.12
diff -c -r1.12 dirmod.c
*** src/port/dirmod.c    26 Feb 2004 02:59:26 -0000    1.12
--- src/port/dirmod.c    28 Jul 2004 17:29:32 -0000
***************
*** 15,30 ****
   *-------------------------------------------------------------------------
   */

- #ifndef TEST_VERSION
-
- #if defined(WIN32) || defined(__CYGWIN__)
-
-
  #ifndef FRONTEND
  #include "postgres.h"
  #else
  #include "postgres_fe.h"
  #endif
  #include "miscadmin.h"

  #undef rename
--- 15,37 ----
   *-------------------------------------------------------------------------
   */

  #ifndef FRONTEND
  #include "postgres.h"
  #else
  #include "postgres_fe.h"
  #endif
+
+ #include <unistd.h>
+ #include <dirent.h>
+ #include <sys/stat.h>
+
+ #define _(x) gettext((x))
+
+ #ifndef TEST_VERSION
+
+ #if defined(WIN32) || defined(__CYGWIN__)
+
+
  #include "miscadmin.h"

  #undef rename
***************
*** 105,110 ****
--- 112,277 ----

  #endif

+ #if defined(WIN32) || defined(__CYGWIN__)
+ #define rmt_unlink(path) pgunlink(path)
+ #else
+ #define rmt_unlink(path) unlink(path)
+ #endif
+
+ #ifdef FRONTEND
+
+ static void *
+ xmalloc(size_t size)
+ {
+     void       *result;
+
+     result = malloc(size);
+     if (!result)
+     {
+         fprintf(stderr, _("out of memory\n"));
+         exit(1);
+     }
+     return result;
+ }
+
+ static char *
+ xstrdup(const char *s)
+ {
+     char       *result;
+
+     result = strdup(s);
+     if (!result)
+     {
+         fprintf(stderr, _("out of memory\n"));
+         exit(1);
+     }
+     return result;
+ }
+
+ #define xfree(n) free(n)
+
+ #else
+
+ /* on the backend, use palloc and friends */
+
+ #define xmalloc(n) palloc(n)
+ #define xstrdup(n) pstrdup (n)
+ #define xfree(n) pfree(n)
+
+ #endif
+
+ /*
+  * deallocate memory used for filenames
+  */
+
+ static void
+ rmt_cleanup(char ** filenames)
+ {
+     char ** fn;
+
+     for (fn = filenames; *fn; fn++)
+         xfree(*fn);
+
+     xfree(filenames);
+ }
+
+
+
+ /*
+  * delete a directory tree recursively
+  * assumes path points to a valid directory
+  * deletes everything under path
+  * if rmtopdir is true deletes the directory too
+  *
+  */
+
+ bool
+ rmtree(char *path, bool rmtopdir)
+ {
+     char        filepath[MAXPGPATH];
+     DIR           *dir;
+     struct dirent *file;
+     char      **filenames;
+     char      **filename;
+     int            numnames = 0;
+     struct stat statbuf;
+
+     /*
+      * we copy all the names out of the directory before we start
+      * modifying it.
+      *
+      */
+
+     dir = opendir(path);
+     if (dir == NULL)
+         return false;
+
+     while ((file = readdir(dir)) != NULL)
+     {
+         if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
+             numnames++;
+     }
+
+     rewinddir(dir);
+
+     filenames = xmalloc((numnames + 2) * sizeof(char *));
+     numnames = 0;
+
+     while ((file = readdir(dir)) != NULL)
+     {
+         if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
+             filenames[numnames++] = xstrdup(file->d_name);
+     }
+
+     filenames[numnames] = NULL;
+
+     closedir(dir);
+
+     /* now we have the names we can start removing things */
+
+     for (filename = filenames; *filename; filename++)
+     {
+         snprintf(filepath, MAXPGPATH, "%s/%s", path, *filename);
+
+         if (stat(filepath, &statbuf) != 0)
+         {
+             rmt_cleanup(filenames);
+             return false;
+         }
+
+         if (S_ISDIR(statbuf.st_mode))
+         {
+             /* call ourselves recursively for a directory */
+             if (!rmtree(filepath, true))
+             {
+                 rmt_cleanup(filenames);
+                 return false;
+             }
+         }
+         else
+         {
+             if (rmt_unlink(filepath) != 0)
+             {
+                 rmt_cleanup(filenames);
+                 return false;
+             }
+         }
+     }
+
+     if (rmtopdir)
+     {
+         if (rmdir(path) != 0)
+         {
+             rmt_cleanup(filenames);
+             return false;
+         }
+     }
+
+     rmt_cleanup(filenames);
+     return true;
+ }
+
+
  #else



Re: [HACKERS] Cannot initdb in cvs tip

From
Bruce Momjian
Date:
Looks good. I will have to do a little adjustment because dirmod is
currently only used by Win32.

Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>
> >Andrew Dunstan wrote:
> >
> >
> >>>I wanted to keep a solution that was as native to the OS as possible,
> >>>but because we can't do that on Win32 and few people like the unix
> >>>system call to 'rm', it is time to clean it up.
> >>>
> >>>One question --- why is there a sleep loop needed for unlink in your
> >>>patch?
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>We will just be calling the existing pgunlink() (which has a sleep) in
> >>the Windows cases, so this question becomes moot.
> >>
> >>
> >
> >Great.  Thanks.  Sorry I delayed addressing this for so long.
> >
> >
>
> Please check the enclosed patch to see if it does what you want.
>
> >Should we look at replacing cp/copy in 7.6?
> >
> >
> >
>
> probably. Put it as a possible TODO maybe.
>
> cheers
>
> andrew
>
>


>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [HACKERS] Cannot initdb in cvs tip

From
Bruce Momjian
Date:
Patch applied.  Thanks.

I made small adjustment for configure.

---------------------------------------------------------------------------


Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>
> >Andrew Dunstan wrote:
> >
> >
> >>>I wanted to keep a solution that was as native to the OS as possible,
> >>>but because we can't do that on Win32 and few people like the unix
> >>>system call to 'rm', it is time to clean it up.
> >>>
> >>>One question --- why is there a sleep loop needed for unlink in your
> >>>patch?
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >>We will just be calling the existing pgunlink() (which has a sleep) in
> >>the Windows cases, so this question becomes moot.
> >>
> >>
> >
> >Great.  Thanks.  Sorry I delayed addressing this for so long.
> >
> >
>
> Please check the enclosed patch to see if it does what you want.
>
> >Should we look at replacing cp/copy in 7.6?
> >
> >
> >
>
> probably. Put it as a possible TODO maybe.
>
> cheers
>
> andrew
>
>


>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073