Re: [HACKERS] Cannot initdb in cvs tip

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Dave Page <dpage(at)vale-housing(dot)co(dot)uk>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, pgsql-patches(at)postgresql(dot)org
Subject: Re: [HACKERS] Cannot initdb in cvs tip
Date: 2004-07-28 08:29:28
Message-ID: 200407280829.i6S8TSo05040@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-hackers-win32 pgsql-patches


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(at)sss(dot)pgh(dot)pa(dot)us]
> > Sent: 19 June 2004 00:22
> > To: Dave Page
> > Cc: PostgreSQL-development
> > Subject: Re: [HACKERS] Cannot initdb in cvs tip
> >
> > "Dave Page" <dpage(at)vale-housing(dot)co(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Page 2004-07-28 08:50:14 Re: [HACKERS] Cannot initdb in cvs tip
Previous Message Gaetano Mendola 2004-07-28 07:39:57 Re: No mail?

Browse pgsql-hackers-win32 by date

  From Date Subject
Next Message Yuri B. Lukyanov 2004-07-28 11:29:43 now()
Previous Message Magnus Hagander 2004-07-28 08:04:17 Re: initdb crashing, signal 5, etc

Browse pgsql-patches by date

  From Date Subject
Next Message Dave Page 2004-07-28 08:50:14 Re: [HACKERS] Cannot initdb in cvs tip
Previous Message Christopher Kings-Lynne 2004-07-28 06:54:47 Re: USING INDEX TABLESPACE