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');