From 154c7f4fe2ebd2acbaedc8ffd94b555c3b129b07 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Mon, 25 Mar 2019 23:57:25 +0100 Subject: [PATCH 3/4] Default new bindir to exec_path 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 is 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. --- doc/src/sgml/ref/pgupgrade.sgml | 1 + src/bin/pg_upgrade/option.c | 21 +++++++++++++-------- src/bin/pg_upgrade/pg_upgrade.c | 24 +++++++++++++++--------- src/bin/pg_upgrade/pg_upgrade.h | 1 - src/bin/pg_upgrade/test.sh | 2 +- 5 files changed, 30 insertions(+), 19 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 d76f27c9e8..b9f10db3bd 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,7 @@ 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 pg_upgrade binary dir)")); 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 +352,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 NULL * * 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 +376,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..82f7d8b919 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -212,6 +212,21 @@ setup(char *argv0, bool *live_check) */ 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) + { + 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. -- 2.14.1.145.gb3622a4ee