pg_upgrade: Default new bindir to pg_upgrade location

Make the directory where the pg_upgrade binary resides the default for
new bindir, as running the pg_upgrade binary from where the new
cluster is installed is a very common scenario.  Setting this as the
defauly bindir for the new cluster will remove the need to provide it
explicitly via -B in many cases.

To support directories being missing from option parsing, extend the
directory check with a missingOk mode where the path must be filled at
a later point before being used.  Also move the exec_path check to
earlier in setup to make sure we know the new cluster bindir when we
scan for required executables.

This removes the exec_path from the OSInfo struct as it is not used
anywhere.

Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us

diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 82886760f108eb6309ebc561a380b4d51e83ece7..3b69db909fec0779b4cb72f3b197c4ea4ed3deaa 100644 (file)
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -89,6 +89,7 @@
       <term><option>-B</option> <replaceable>bindir</replaceable></term>
       <term><option>--new-bindir=</option><replaceable>bindir</replaceable></term>
       <listitem><para>the new PostgreSQL executable directory;
+      default is the directory where <application>pg_upgrade</application> resides;
       environment variable <envar>PGBINNEW</envar></para></listitem>
      </varlistentry>
 

diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index ff1bcaa1a1c42d277eaffea8f4e5a36583ff915c..7e3d3f1bb27586992c92623baaf37d9bff3787e9 100644 (file)
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -23,7 +23,8 @@
 static void usage(void);
 static void check_required_directory(char **dirpath,
                                                                         const char *envVarName, bool useCwd,
-                                                                        const char *cmdLineOption, const char *description);
+                                                                        const char *cmdLineOption, const char *description,
+                                                                        bool missingOk);
 #define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false"
 
 
@@ -251,15 +252,15 @@ parseCommandLine(int argc, char *argv[])
 
        /* Get values from env if not already set */
        check_required_directory(&old_cluster.bindir, "PGBINOLD", false,
-                                                        "-b", _("old cluster binaries reside"));
+                                                        "-b", _("old cluster binaries reside"), false);
        check_required_directory(&new_cluster.bindir, "PGBINNEW", false,
-                                                        "-B", _("new cluster binaries reside"));
+                                                        "-B", _("new cluster binaries reside"), true);
        check_required_directory(&old_cluster.pgdata, "PGDATAOLD", false,
-                                                        "-d", _("old cluster data resides"));
+                                                        "-d", _("old cluster data resides"), false);
        check_required_directory(&new_cluster.pgdata, "PGDATANEW", false,
-                                                        "-D", _("new cluster data resides"));
+                                                        "-D", _("new cluster data resides"), false);
        check_required_directory(&user_opts.socketdir, "PGSOCKETDIR", true,
-                                                        "-s", _("sockets will be created"));
+                                                        "-s", _("sockets will be created"), false);
 
 #ifdef WIN32
 
@@ -293,7 +294,8 @@ usage(void)
        printf(_("  pg_upgrade [OPTION]...\n\n"));
        printf(_("Options:\n"));
        printf(_("  -b, --old-bindir=BINDIR       old cluster executable directory\n"));
-       printf(_("  -B, --new-bindir=BINDIR       new cluster executable directory\n"));
+       printf(_("  -B, --new-bindir=BINDIR       new cluster executable directory (default\n"
+                        "                                same directory as pg_upgrade)"));
        printf(_("  -c, --check                   check clusters only, don't change any data\n"));
        printf(_("  -d, --old-datadir=DATADIR     old cluster data directory\n"));
        printf(_("  -D, --new-datadir=DATADIR     new cluster data directory\n"));
@@ -351,13 +353,15 @@ usage(void)
  *     useCwd            - true if OK to default to CWD
  *     cmdLineOption - the command line option for this directory
  *     description   - a description of this directory option
+ *     missingOk         - true if OK that both dirpath and envVarName are not existing
  *
  * We use the last two arguments to construct a meaningful error message if the
  * user hasn't provided the required directory name.
  */
 static void
 check_required_directory(char **dirpath, const char *envVarName, bool useCwd,
-                                                const char *cmdLineOption, const char *description)
+                                                const char *cmdLineOption, const char *description,
+                                                bool missingOk)
 {
        if (*dirpath == NULL || strlen(*dirpath) == 0)
        {
@@ -373,6 +377,8 @@ check_required_directory(char **dirpath, const char *envVarName, bool useCwd,
                                pg_fatal("could not determine current directory\n");
                        *dirpath = pg_strdup(cwd);
                }
+               else if (missingOk)
+                       return;
                else
                        pg_fatal("You must identify the directory where the %s.\n"
                                         "Please use the %s command-line option or the %s environment variable.\n",

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index d1975aab2bc2c9ad3bde1eab3cc18a9519e73ca3..5e617e97c0f0bc659a5105d5dcb0a169a71c3227 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -204,14 +204,29 @@ main(int argc, char **argv)
 static void
 setup(char *argv0, bool *live_check)
 {
-       char            exec_path[MAXPGPATH];   /* full path to my executable */
-
        /*
         * make sure the user has a clean environment, otherwise, we may confuse
         * libpq when we connect to one (or both) of the servers.
         */
        check_pghost_envvar();
 
+       /*
+        * In case the user hasn't specified the directory for the new binaries
+        * with -B, default to using the path of the currently executed pg_upgrade
+        * binary.
+        */
+       if (!new_cluster.bindir)
+       {
+               char            exec_path[MAXPGPATH];
+
+               if (find_my_exec(argv0, exec_path) < 0)
+                       pg_fatal("%s: could not find own program executable\n", argv0);
+               /* Trim off program name and keep just path */
+               *last_dir_separator(exec_path) = '\0';
+               canonicalize_path(exec_path);
+               new_cluster.bindir = pg_strdup(exec_path);
+       }
+
        verify_directories();
 
        /* no postmasters should be running, except for a live check */
@@ -247,15 +262,6 @@ setup(char *argv0, bool *live_check)
                        pg_fatal("There seems to be a postmaster servicing the new cluster.\n"
                                         "Please shutdown that postmaster and try again.\n");
        }
-
-       /* get path to pg_upgrade executable */
-       if (find_my_exec(argv0, exec_path) < 0)
-               pg_fatal("%s: could not find own program executable\n", argv0);
-
-       /* Trim off program name and keep just path */
-       *last_dir_separator(exec_path) = '\0';
-       canonicalize_path(exec_path);
-       os_info.exec_path = pg_strdup(exec_path);
 }
 
 

diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 5d31750d867498f37bed3b425de472e45a867117..ca6a9efd9c4508d132668243d79ddca5fea149aa 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -314,7 +314,6 @@ typedef struct
 typedef struct
 {
        const char *progname;           /* complete pathname for this program */
-       char       *exec_path;          /* full path to my executable */
        char       *user;                       /* username for clusters */
        bool            user_specified; /* user specified on command-line */
        char      **old_tablespaces;    /* tablespaces */

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 78820247b35282687076c4060b52a51d668cbefb..7e44747e39de10a61f290532c453bb1e097cacfd 100644 (file)
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -220,7 +220,7 @@ PGDATA="$BASE_PGDATA"
 
 standard_initdb 'initdb'
 
-pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -B "$bindir" -p "$PGPORT" -P "$PGPORT"
+pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "$PGPORT" -P "$PGPORT"
 
 # make sure all directories and files have group permissions, on Unix hosts
 # Windows hosts don't support Unix-y permissions.

diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 5495066b4d98639d07276ec80097c0ab1f322d49..33d8fb5daaf0d7dcf437ef331183850340caafbe 100644 (file)
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -604,7 +604,7 @@ sub upgradecheck
        print "\nRunning pg_upgrade\n\n";
        @args = (
                'pg_upgrade', '-d', "$data.old", '-D', $data, '-b',
-               $bindir,      '-B', $bindir);
+               $bindir);
        system(@args) == 0 or exit 1;
        print "\nStarting new cluster\n\n";
        @args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');