Re: Disable alternate locations on Win32 - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: Disable alternate locations on Win32
Date
Msg-id 200305070351.h473pXZ29600@candle.pha.pa.us
Whole thread Raw
In response to Re: Disable alternate locations on Win32  (Bruce Momjian <pgman@candle.pha.pa.us>)
Responses Re: Disable alternate locations on Win32  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
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


pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Global variables in exec()
Next
From: Tom Lane
Date:
Subject: Re: Disable alternate locations on Win32