I came across the HAVE_WORKING_LINK define in pg_config_manual.h. AFAICT, hard links are supported on Windows and Cygwin in the OS versions that we support, and pg_upgrade already contains the required shim. It seems to me we could normalize and simplify that, as in the attached patches. (Perhaps rename durable_link_or_rename() then.) I successfully tested on MSVC, MinGW, and Cygwin.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 2d14ebf68aeaa777c631655ef8159579106d592f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 27 Feb 2020 16:33:05 +0100
Subject: [PATCH 1/3] pg_standby: Don't use HAVE_WORKING_LINK

HAVE_WORKING_LINK is meant for hard links, mainly for Windows.  Here
it's used for soft links on Unix, and the functionality is optional
anyway, so we can just make it error out normally if needed.
---
 contrib/pg_standby/pg_standby.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
index 9784d7aef3..3567c86ac8 100644
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -144,10 +144,8 @@ CustomizableInitialize(void)
        switch (restoreCommandType)
        {
                case RESTORE_COMMAND_LINK:
-#ifdef HAVE_WORKING_LINK
                        SET_RESTORE_COMMAND("ln -s -f", WALFilePath, 
xlogFilePath);
                        break;
-#endif
                case RESTORE_COMMAND_COPY:
                default:
                        SET_RESTORE_COMMAND("cp", WALFilePath, xlogFilePath);
-- 
2.25.0

From 210362e7562ea98c21eedd88fd3ab99eabca7a00 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 27 Feb 2020 16:42:13 +0100
Subject: [PATCH 2/3] Move pg_upgrade's Windows link() implementation to
 AC_REPLACE_FUNCS

This was we can make use of it in other components as well, and it
fits better with the rest of the build system.
---
 configure                       | 13 ++++++++++++
 configure.in                    |  1 +
 src/bin/pg_upgrade/file.c       | 27 ++-----------------------
 src/bin/pg_upgrade/pg_upgrade.h |  2 --
 src/include/pg_config.h.in      |  3 +++
 src/include/port.h              |  4 ++++
 src/port/link.c                 | 35 +++++++++++++++++++++++++++++++++
 src/tools/msvc/Mkvcbuild.pm     |  2 +-
 src/tools/msvc/Solution.pm      |  1 +
 9 files changed, 60 insertions(+), 28 deletions(-)
 create mode 100644 src/port/link.c

diff --git a/configure b/configure
index d2c74e530c..1e8a93707f 100755
--- a/configure
+++ b/configure
@@ -15504,6 +15504,19 @@ esac
 
 fi
 
+ac_fn_c_check_func "$LINENO" "link" "ac_cv_func_link"
+if test "x$ac_cv_func_link" = xyes; then :
+  $as_echo "#define HAVE_LINK 1" >>confdefs.h
+
+else
+  case " $LIBOBJS " in
+  *" link.$ac_objext "* ) ;;
+  *) LIBOBJS="$LIBOBJS link.$ac_objext"
+ ;;
+esac
+
+fi
+
 ac_fn_c_check_func "$LINENO" "mkdtemp" "ac_cv_func_mkdtemp"
 if test "x$ac_cv_func_mkdtemp" = xyes; then :
   $as_echo "#define HAVE_MKDTEMP 1" >>confdefs.h
diff --git a/configure.in b/configure.in
index 0b0a871298..7c07cc4a3f 100644
--- a/configure.in
+++ b/configure.in
@@ -1697,6 +1697,7 @@ AC_REPLACE_FUNCS(m4_normalize([
        getpeereid
        getrusage
        inet_aton
+       link
        mkdtemp
        pread
        pwrite
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 6db7cce6a2..cc8a675d00 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -26,10 +26,6 @@
 #include "storage/checksum.h"
 #include "storage/checksum_impl.h"
 
-#ifdef WIN32
-static int     win32_pghardlink(const char *src, const char *dst);
-#endif
-
 
 /*
  * cloneFile()
@@ -151,7 +147,7 @@ void
 linkFile(const char *src, const char *dst,
                 const char *schemaName, const char *relName)
 {
-       if (pg_link_file(src, dst) < 0)
+       if (link(src, dst) < 0)
                pg_fatal("error while creating link for relation \"%s.%s\" 
(\"%s\" to \"%s\"): %s\n",
                                 schemaName, relName, src, dst, 
strerror(errno));
 }
@@ -369,29 +365,10 @@ check_hard_link(void)
        snprintf(new_link_file, sizeof(new_link_file), 
"%s/PG_VERSION.linktest", new_cluster.pgdata);
        unlink(new_link_file);          /* might fail */
 
-       if (pg_link_file(existing_file, new_link_file) < 0)
+       if (link(existing_file, new_link_file) < 0)
                pg_fatal("could not create hard link between old and new data 
directories: %s\n"
                                 "In link mode the old and new data directories 
must be on the same file system.\n",
                                 strerror(errno));
 
        unlink(new_link_file);
 }
-
-#ifdef WIN32
-/* implementation of pg_link_file() on Windows */
-static int
-win32_pghardlink(const char *src, const char *dst)
-{
-       /*
-        * CreateHardLinkA returns zero for failure
-        * http://msdn.microsoft.com/en-us/library/aa363860(VS.85).aspx
-        */
-       if (CreateHardLinkA(dst, src, NULL) == 0)
-       {
-               _dosmaperr(GetLastError());
-               return -1;
-       }
-       else
-               return 0;
-}
-#endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index b156b516cc..8b90cefbe0 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -65,7 +65,6 @@ extern char *output_files[];
 
 #ifndef WIN32
 #define pg_mv_file                     rename
-#define pg_link_file           link
 #define PATH_SEPARATOR         '/'
 #define PATH_QUOTE     '\''
 #define RM_CMD                         "rm -f"
@@ -76,7 +75,6 @@ extern char *output_files[];
 #define ECHO_BLANK     ""
 #else
 #define pg_mv_file                     pgrename
-#define pg_link_file           win32_pghardlink
 #define PATH_SEPARATOR         '\\'
 #define PATH_QUOTE     '"'
 #define RM_CMD                         "DEL /q"
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 4fa0f770aa..d758dfd36e 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -346,6 +346,9 @@
 /* Define to 1 if you have the `z' library (-lz). */
 #undef HAVE_LIBZ
 
+/* Define to 1 if you have the `link' function. */
+#undef HAVE_LINK
+
 /* Define to 1 if the system has the type `locale_t'. */
 #undef HAVE_LOCALE_T
 
diff --git a/src/include/port.h b/src/include/port.h
index 3be994b43c..29f3e39e5b 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -381,6 +381,10 @@ extern float pg_strtof(const char *nptr, char **endptr);
 #define strtof(a,b) (pg_strtof((a),(b)))
 #endif
 
+#ifndef HAVE_LINK
+extern int link(const char *src, const char *dst);
+#endif
+
 #ifndef HAVE_MKDTEMP
 extern char *mkdtemp(char *path);
 #endif
diff --git a/src/port/link.c b/src/port/link.c
new file mode 100644
index 0000000000..48a8a64025
--- /dev/null
+++ b/src/port/link.c
@@ -0,0 +1,35 @@
+/*-------------------------------------------------------------------------
+ *
+ * link.c
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *       src/port/link.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "c.h"
+
+#ifdef WIN32
+
+int
+link(const char *src, const char *dst)
+{
+       /*
+        * CreateHardLinkA returns zero for failure
+        * http://msdn.microsoft.com/en-us/library/aa363860(VS.85).aspx
+        */
+       if (CreateHardLinkA(dst, src, NULL) == 0)
+       {
+               _dosmaperr(GetLastError());
+               return -1;
+       }
+       else
+               return 0;
+}
+
+#endif
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 834c2c39d1..ec25042933 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -98,7 +98,7 @@ sub mkvcbuild
          chklocale.c explicit_bzero.c fls.c getpeereid.c getrusage.c 
inet_aton.c random.c
          srandom.c getaddrinfo.c gettimeofday.c inet_net_ntop.c kill.c open.c
          erand48.c snprintf.c strlcat.c strlcpy.c dirmod.c noblock.c path.c
-         dirent.c dlopen.c getopt.c getopt_long.c
+         dirent.c dlopen.c getopt.c getopt_long.c link.c
          pread.c pwrite.c pg_bitutils.c
          pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c
          pqsignal.c mkdtemp.c qsort.c qsort_arg.c quotes.c system.c
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 6b4a6eec2a..3b36e8ad0d 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -295,6 +295,7 @@ sub GenerateFiles
                HAVE_LIBXML2                                => undef,
                HAVE_LIBXSLT                                => undef,
                HAVE_LIBZ                   => $self->{options}->{zlib} ? 1 : 
undef,
+               HAVE_LINK                   => undef,
                HAVE_LOCALE_T               => 1,
                HAVE_LONG_INT_64            => undef,
                HAVE_LONG_LONG_INT_64       => 1,
-- 
2.25.0

From 51639b9dcc048d29ab6cf986378c24126a9666ce Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 27 Feb 2020 16:46:38 +0100
Subject: [PATCH 3/3] Remove HAVE_WORKING_LINK

Previously, hard links were not used on Windows and Cygwin, but they
support them just fine in currently supported OS versions, so we can
use them there as well.

Since all supported platforms now support hard links, we can remove
the alternative code paths.
---
 src/backend/storage/file/fd.c  | 12 ------------
 src/include/pg_config_manual.h |  7 -------
 2 files changed, 19 deletions(-)

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 34f7443110..2baadc2453 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -788,7 +788,6 @@ durable_link_or_rename(const char *oldfile, const char 
*newfile, int elevel)
        if (fsync_fname_ext(oldfile, false, false, elevel) != 0)
                return -1;
 
-#ifdef HAVE_WORKING_LINK
        if (link(oldfile, newfile) < 0)
        {
                ereport(elevel,
@@ -798,17 +797,6 @@ durable_link_or_rename(const char *oldfile, const char 
*newfile, int elevel)
                return -1;
        }
        unlink(oldfile);
-#else
-       /* XXX: Add racy file existence check? */
-       if (rename(oldfile, newfile) < 0)
-       {
-               ereport(elevel,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to 
\"%s\": %m",
-                                               oldfile, newfile)));
-               return -1;
-       }
-#endif
 
        /*
         * Make change persistent in case of an OS crash, both the new entry and
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index b4ce53300b..d74a8dd808 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -129,13 +129,6 @@
 #undef HAVE_UNIX_SOCKETS
 #endif
 
-/*
- * Define this if your operating system supports link()
- */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-#define HAVE_WORKING_LINK 1
-#endif
-
 /*
  * USE_POSIX_FADVISE controls whether Postgres will attempt to use the
  * posix_fadvise() kernel call.  Usually the automatic configure tests are
-- 
2.25.0

Reply via email to