From 959f6d6a1821b7d9068244f500dd80953e768d16 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 27 Jul 2019 07:56:20 +0200 Subject: [PATCH] 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 Reviewed-by: Peter Eisentraut Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us --- doc/src/sgml/ref/pgupgrade.sgml | 1 + src/bin/pg_upgrade/option.c | 22 ++++++++++++++-------- src/bin/pg_upgrade/pg_upgrade.c | 28 +++++++++++++++++----------- src/bin/pg_upgrade/pg_upgrade.h | 1 - src/bin/pg_upgrade/test.sh | 2 +- src/tools/msvc/vcregress.pl | 2 +- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 82886760f1..3b69db909f 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -89,6 +89,7 @@ bindir bindir the new PostgreSQL executable directory; + default is the directory where pg_upgrade resides; environment variable PGBINNEW diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index ff1bcaa1a1..7e3d3f1bb2 100644 --- 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 d1975aab2b..5e617e97c0 100644 --- 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 5d31750d86..ca6a9efd9c 100644 --- 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 78820247b3..7e44747e39 100644 --- 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 5495066b4d..33d8fb5daa 100644 --- 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'); -- 2.40.0