Обсуждение: Disable alternate locations on Win32

Поиск
Список
Период
Сортировка

Disable alternate locations on Win32

От
Bruce Momjian
Дата:
The following patch disables alternate locations on Win32 because it
doesn't have symlinks.
--
  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
Index: src/backend/commands/dbcommands.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/dbcommands.c,v
retrieving revision 1.112
diff -c -c -r1.112 dbcommands.c
*** src/backend/commands/dbcommands.c    4 Apr 2003 20:42:12 -0000    1.112
--- src/backend/commands/dbcommands.c    4 May 2003 04:32:59 -0000
***************
*** 174,179 ****
--- 174,184 ----
      /* don't call this in a transaction block */
      PreventTransactionChain((void *) stmt, "CREATE DATABASE");

+ #ifdef WIN32
+     if (dbpath != NULL)    /* platform has no symlinks */
+         elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
+ #endif
+
      /*
       * Check for db name conflict.    There is a race condition here, since
       * another backend could create the same DB name before we commit.
***************
*** 296,302 ****
--- 301,309 ----
      /* Make the symlink, if needed */
      if (alt_loc)
      {
+ #ifndef WIN32    /* already throws error on WIN32 above */
          if (symlink(alt_loc, nominal_loc) != 0)
+ #endif
              elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
                   nominal_loc, alt_loc);
      }

Re: Disable alternate locations on Win32

От
Tom Lane
Дата:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The following patch disables alternate locations on Win32 because it
> doesn't have symlinks.

Why not do it with one ifdef in one place?

***************
*** 296,302 ****
--- 301,309 ----
      /* Make the symlink, if needed */
      if (alt_loc)
      {
+ #ifndef WIN32
          if (symlink(alt_loc, nominal_loc) != 0)
              elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
                   nominal_loc, alt_loc);
+ #else
+         elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
+ #endif
      }

Also I wonder if this shouldn't be conditionalized on something like
HAVE_SYMLINKS rather than a hardwired platform check.

            regards, tom lane


Re: Disable alternate locations on Win32

От
Bruce Momjian
Дата:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > The following patch disables alternate locations on Win32 because it
> > doesn't have symlinks.
>
> Why not do it with one ifdef in one place?

It was done this way because they wanted to test earlier for failure,
and they didn't want the symlink call because they didn't have symlinks.
I didn't totally follow the code.

>       if (alt_loc)
>       {
> + #ifndef WIN32
>           if (symlink(alt_loc, nominal_loc) != 0)
>               elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
>                    nominal_loc, alt_loc);
> + #else
> +         elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
> + #endif
>       }
>
> Also I wonder if this shouldn't be conditionalized on something like
> HAVE_SYMLINKS rather than a hardwired platform check.

Yes, that would be better.

--
  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: Disable alternate locations on Win32

От
Bruce Momjian
Дата:
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > The following patch disables alternate locations on Win32 because it
> > > doesn't have symlinks.
> >
> > Why not do it with one ifdef in one place?
>
> It was done this way because they wanted to test earlier for failure,
> and they didn't want the symlink call because they didn't have symlinks.
> I didn't totally follow the code.

I checked this again and the test is at the top, while the symlink()
call is at the bottom after the database has already been copied.  I
think we do need to error out before we start copying the database.

> >       if (alt_loc)
> >       {
> > + #ifndef WIN32
> >           if (symlink(alt_loc, nominal_loc) != 0)
> >               elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
> >                    nominal_loc, alt_loc);
> > + #else
> > +         elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
> > + #endif
> >       }
> >
> > Also I wonder if this shouldn't be conditionalized on something like
> > HAVE_SYMLINKS rather than a hardwired platform check.
>
> Yes, that would be better.

OK, here is an applied patch that tests for symlink() rather than WIN32.

--
  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
Index: configure
===================================================================
RCS file: /cvsroot/pgsql-server/configure,v
retrieving revision 1.253
diff -c -c -r1.253 configure
*** configure    6 May 2003 23:33:52 -0000    1.253
--- configure    7 May 2003 03:45:18 -0000
***************
*** 10305,10311 ****



! for ac_func in cbrt dlopen fcvt fdatasync getpeereid memmove poll pstat setproctitle setsid sigprocmask sysconf utime
utimeswaitpid 
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  echo "$as_me:$LINENO: checking for $ac_func" >&5
--- 10305,10312 ----



!
! for ac_func in cbrt dlopen fcvt fdatasync getpeereid memmove poll pstat setproctitle setsid sigprocmask symlink
sysconfutime utimes waitpid 
  do
  as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
  echo "$as_me:$LINENO: checking for $ac_func" >&5
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql-server/configure.in,v
retrieving revision 1.244
diff -c -c -r1.244 configure.in
*** configure.in    24 Apr 2003 21:16:42 -0000    1.244
--- configure.in    7 May 2003 03:45:19 -0000
***************
*** 779,785 ****
  # SunOS doesn't handle negative byte comparisons properly with +/- return
  AC_FUNC_MEMCMP

! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid memmove poll pstat setproctitle setsid sigprocmask sysconf
utimeutimes waitpid]) 

  AC_CHECK_DECLS(fdatasync, [], [], [#include <unistd.h>])

--- 779,785 ----
  # SunOS doesn't handle negative byte comparisons properly with +/- return
  AC_FUNC_MEMCMP

! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid memmove poll pstat setproctitle setsid sigprocmask symlink
sysconfutime utimes waitpid]) 

  AC_CHECK_DECLS(fdatasync, [], [], [#include <unistd.h>])

Index: src/backend/commands/dbcommands.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/dbcommands.c,v
retrieving revision 1.113
diff -c -c -r1.113 dbcommands.c
*** src/backend/commands/dbcommands.c    4 May 2003 04:42:52 -0000    1.113
--- src/backend/commands/dbcommands.c    7 May 2003 03:45:21 -0000
***************
*** 174,181 ****
      /* don't call this in a transaction block */
      PreventTransactionChain((void *) stmt, "CREATE DATABASE");

! #ifdef WIN32
!     if (dbpath != NULL)    /* platform has no symlinks */
          elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
  #endif

--- 174,181 ----
      /* don't call this in a transaction block */
      PreventTransactionChain((void *) stmt, "CREATE DATABASE");

! #ifndef HAVE_SYMLINK
!     if (dbpath != NULL)
          elog(ERROR, "CREATE DATABASE: may not use an alternate location on this platform");
  #endif

***************
*** 301,307 ****
      /* Make the symlink, if needed */
      if (alt_loc)
      {
! #ifndef WIN32    /* already throws error on WIN32 above */
          if (symlink(alt_loc, nominal_loc) != 0)
  #endif
              elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
--- 301,307 ----
      /* Make the symlink, if needed */
      if (alt_loc)
      {
! #ifdef HAVE_SYMLINK    /* already throws error above */
          if (symlink(alt_loc, nominal_loc) != 0)
  #endif
              elog(ERROR, "CREATE DATABASE: could not link '%s' to '%s': %m",
Index: src/include/pg_config.h.in
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/pg_config.h.in,v
retrieving revision 1.45
diff -c -c -r1.45 pg_config.h.in
*** src/include/pg_config.h.in    24 Apr 2003 21:16:44 -0000    1.45
--- src/include/pg_config.h.in    7 May 2003 03:45:28 -0000
***************
*** 414,419 ****
--- 414,422 ----
  /* Define to 1 if you have the <SupportDefs.h> header file. */
  #undef HAVE_SUPPORTDEFS_H

+ /* Define to 1 if you have the `symlink' function. */
+ #undef HAVE_SYMLINK
+
  /* Define to 1 if you have the `sysconf' function. */
  #undef HAVE_SYSCONF


Re: Disable alternate locations on Win32

От
Tom Lane
Дата:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Why not do it with one ifdef in one place?

> I checked this again and the test is at the top, while the symlink()
> call is at the bottom after the database has already been copied.  I
> think we do need to error out before we start copying the database.

Huh?  We haven't copied anything at that point.  We have done the
mkdir() though.  On spec-compliant platforms it'd be possible to
switch the order of these operations, so that the symlink comes before
mkdir.  Does anyone know of a platform on which symlink() fails if the
referenced pathname doesn't exist yet?

            regards, tom lane