[PATCH] pg_upgrade: add --initdb option to create the new cluster automatically

My review. Overall, this looks really nice and a much needed feature. I
will definitely be using this a lot in the future :)

It cleanly compiled on top of 97a18c22 (from Fri Jul 10 22:34:24 2026), and
I was able to successfully do the upgrade via the new --initdb flag for
versions 11 through 18. Version 10 did not work, but that is not the fault
of this patch. But the output for completeness, since we still support
upgrading from v10:

Inspecting old cluster locale for new cluster creation
invalid connection option "max_protocol_version"

I also verified that it correctly maintains the checksum setting, the
locale, weird WAL segment sizes, and the new icu and builtin locale
options. However, the tests do none of those checks, and they really ought
to, as that's an important part of what this feature provides.

Extra initdb options can be passed via the existing -O flag and will be
forwarded to the initdb invocation.

I don't like this part, as there is not a 100% overlap between what
postmaster accepts and what initdb accepts. The -c options are fine, but
things like -O "-B 12345" cause a failure. I'd simply not add the -O
options here: if people really need them, they can run pg_upgrade without
the --initdb flag. Alternatively, only allow -c items, but that's a real
pain to code that.

+static void create_new_cluster_via_initdb(void);

Nice, I love descriptive function names.

+ * resolve_new_bindir()

Good idea, well executed.

+/* Refuse to overwrite an existing cluster. */

Move this up to just after the other check that initdb is present?

+ snprintf(verfile, sizeof(verfile), "%s/PG_VERSION",

I suppose this is slightly better than just checking if the dir is empty or
not, as initdb itself will later fail if non-empty but does not contain
PG_VERSION. So a weak +1. Sort of has test coverage (but see below)

+ * Verify that initdb is present and executable before doing any work.

Also not tested at all (may be overkill to do so, but pointing out for
completeness)

+# Copyright (c) 2022-2025, PostgreSQL Global Development Group

s/2025/2026/ - or perhaps just say "2026" as this is a new file

+# --initdb must refuse to clobber an already-populated data directory.
+command_fails(

Would be nice if we confirmed this was failing due to the new PG_VERSION
check, versus the initdb "ain't gonna write to non empty dir" error.

Cheers,
Greg

Hi,

Hi,

This patch adds an --initdb option to pg_upgrade that automates the initdb step currently required before running pg_upgrade.

Problem
—————
Before running pg_upgrade, users must manually run initdb with options that exactly match the old cluster: WAL segment size, data checksum setting, encoding, and locale. Getting these right is error-prone. A mismatch results in a confusing check_control_data() failure after the user has already invested time in setting up the new cluster. A related question was raised before [1], where Jeff Davis discussed whether pg_upgrade should perform initdb itself rather than requiring a pre-initialized cluster.

Solution
————
With --initdb, pg_upgrade handles this automatically. It derives the WAL segment size and checksum setting from pg_control and invokes initdb with the correct flags. The option refuses to proceed if the new cluster already exists.

Thanks for the patch but I have a couple of concerns that will require
further discussion.

Since we include the initdb option to pg_upgrade it can cause orphan
directories that users must manually solve. Partial initdb failures
leave orphaned clusters.

Moreover, in the patch the old cluster is being started/stopped twice
meaning that the pg_upgrade needs to manage the states. This will
cause unpredictable behavior on retry and make things really hard to
debug I believe.

On the other side, the responsibility of pg_upgrade shouldn't contain
initializing a new cluster and deciding the locale,encoding, and
checksums. If the idea is accepted we should focus on documentation
and avoid the risks it will introduce during the upgrade.

Regards,
Demir.

Thanks for the review, Greg. Glad it's useful, and thanks for trying it
across the version range.

Attached is v2, which addresses your points:

- Added test coverage for the settings the feature is really about. The
test now initdb's the old cluster with data checksums and a non-default WAL
segment size, and after the upgrade checks that the new cluster inherited
the checksum setting, WAL segment size, encoding, collation, ctype, and
locale provider.

- The overwrite test now confirms the failure comes from pg_upgrade's own
PG_VERSION check ("already contains a database system") rather than
initdb's non-empty-directory error.

- Added a test for the "initdb not present" case (pointing --new-bindir at
an empty dir), since it was easy enough to cover.

- Reworked the -O handling. Rather than dropping it entirely, I took your
alternative: only "-c name=value" options are forwarded to initdb now, and
anything else is skipped with a warning. So -O "-c work_mem=..." works, and
-O "-B 12345" no longer breaks initdb.

That keeps GUC pass-through working without pretending initdb accepts the
full postmaster option set.

- Moved the "refuse to overwrite" check up to right after the
initdb-present check.

- Fixed the copyright year on the new test file.

On the v10 failure ("invalid connection option max_protocol_version"):
that comes from stock pg_upgrade, which adds max_protocol_version=3.0 to
the connection string for pre-PG11 servers (server.c). It's not introduced
by --initdb, the same connection is made in the normal flow, so I've left
it out of scope for this patch.

The patch is pgindent-clean and the TAP test passes. For convenience, the
branch is also on GitHub:

github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Fri, Jul 10, 2026 at 7:24 PM Greg Sabino Mullane <htamfids@gmail.com>
wrote:

I am not convinced that all this complexity and overhead isn't better suited
for external wrappers to pg_upgrade like Debian's pg_upgradecluster etc.

+	if (user_opts.initdb_new_cluster)
+		create_new_cluster_via_initdb();
+
 	adjust_data_dir(&new_cluster);

If pg_upgrade controls how initdb was invoked for the new cluster, shouldn't it
work such that adjust_data_dir isn't required?

+	/*
+	 * get_control_data() selects pg_resetwal vs. pg_resetxlog via
+	 * bin_version, which check_bindir() normally fills in later.  Seed it now
+	 * so the right binary name is used in this early call.
+	 */
+	if (old_cluster.bin_version == 0)
+		old_cluster.bin_version = old_cluster.major_version;
+
+	get_control_data(&old_cluster);

This can't be done unconditionally, the old cluster can still be running at
this point. For example if someone wants to do a live-check:

$ ./bin/pg_upgrade -b ./bin/ -B ./bin/ -d ./data_old/ -D ./data_new/ --check --initdb

The source cluster was not shut down cleanly, state reported as: "in production"
Failure, exiting

get_control_data is big, expensive, and really designed to be run once. I
don't think it's Ok to run it an extra time here without at least being able to
tell the later invocation that it has already been executed. Also,
check_bindir() as referred to in the comment does not exist.

+ prep_status("Inspecting old cluster locale for new cluster creation");
+ start_postmaster(&old_cluster, true);
+ get_template0_info(&old_cluster);
+ stop_postmaster(false);
+ check_ok();

Again, cannot be done unconditionally.

+	 * Users needing options that only the postmaster accepts can create the
+	 * new cluster manually and omit --initdb.

This should probably be expanded upon in the documentation.

--
Daniel Gustafsson

Hi Daniel,

Thanks for the review. v3 attached.

*On external wrapper vs. in-tree:* the point of --initdb is deriving the
new cluster's settings (WAL segment size, checksums, encoding, locale) from
the old cluster, exactly what people get wrong by hand and only discover
when check_control_data() fails. pg_upgrade already reads the old control
data, so doing it here reuses that rather than making a wrapper rediscover
it. I'm open to being convinced otherwise.

The rest:
*- --check --initdb:* you're right it was broken (ran before live-check was
determined). Since --check is read-only and --initdb creates the cluster,
the combination doesn't make sense — v3 rejects it at option parsing, which
removes the live-check problem entirely.
*- get_control_data() twice:* the early --initdb read populates the old
cluster's control data, so the later call in check_cluster_compatibility()
returns early when ctrl_ver is already set. New cluster is still read fresh.
*- adjust_data_dir(): *moved adjust_data_dir(&new_cluster) before the
initdb call so a config-only -D is resolved first.
*- check_bindir(): *meant get_bin_version(). Fixed.
*- Docs:* noted that only initdb-accepted settings are configured,
postmaster-only options need manual creation, and the --check restriction.

TAP test covers the --check rejection. pgindent-clean, tests pass.
https://github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb
Best regards,
Bohyun

On Tue, Jul 14, 2026 at 12:06 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 14 Jul 2026, at 16:42, Bohyun Lee <bohyun.lee@databricks.com> wrote:

Thanks for the update.

Right now most of the focus is on getting v19 ready to ship, I recommend
registering the patch in the commitfest app to make sure it's not lost (if you
already did I just failed to find it).

On external wrapper vs. in-tree: the point of --initdb is deriving the new cluster's settings (WAL segment size, checksums, encoding, locale) from the old cluster, exactly what people get wrong by hand and only discover when check_control_data() fails. pg_upgrade already reads the old control data, so doing it here reuses that rather than making a wrapper rediscover it. I'm open to being convinced otherwise.

Maybe. Since it won't be a mandatory step, we still need to document the
process properly and I see that as the first step. Currently the docs have
"use compatible initdb flags that match the old cluster" which clearly isn't
particularly helpful.

--
Daniel Gustafsson

Second review pass.

Cleanly compiles and all tests pass when applied to 637aa273e (Thu Jul 16
11:56:01 2026 -0400)

/*
* resolve_new_bindir()
*
* Idempotent helper: if new_cluster.bindir has not been set by the user via
* -B, derive it from the path of the currently executing pg_upgrade binary.
* Called early by create_new_cluster_via_initdb() so that the initdb path
* is available before verify_directories() runs.
*/

Do we really need the last two lines? This is also called during the
"normal" path, not just via create_new_cluster_via_initdb, so not sure why
we are pointing it out here.

prep_status("Inspecting old cluster locale for new cluster creation");

We are doing a lot more than locale though. How about:

prep_status("Examining old cluster settings");

# Use non-default settings that --initdb must carry over to the new

cluster

# (derived from the old cluster's pg_control): data checksums and a

non-default

# WAL segment size. We check below that the new cluster inherits them.
my $oldnode = PostgreSQL::Test::Cluster->new('old_node');
$oldnode->init(extra => [ '--data-checksums', '--wal-segsize' => '2' ]);

But checkums ARE the default now, so for a non-default setting, we should
do --no-data-checksums. Would be nice to check locale as well. That gets
tricky, but maybe --locale=C is nicely portable and pretty non-default?

my $conf = $newnode->data_dir . '/postgresql.conf';
open(my $fh, '>>', $conf) or die "could not open $conf: $!";

Easier to use $newnode->append_conf

print $fh "listen_addresses = ''\n";
print $fh "unix_socket_directories = '" . $newnode->host . "'\n";

What about Win32? See .e.g $use_tcp in Cluster.pm

* Forward only "-c name=value" options from -O to initdb. initdb accepts

I still think we just bail on supporting -O. The new code is an
improvement, but fails if there are legitimate spaces in any of the args:

$ pg_upgrade --initdb -O "-c transaction_isolation='repeatable read'"

Creating new cluster with initdb ignoring
non-"-c" option(s) passed via -O; only "-c" settings are forwarded to
initdb for --initdb
sh: 1: Syntax error: Unterminated quoted string

*failure*

Cheers,
Greg

Thanks both for the reviews. v4 attached, addressing all of Greg's
second-pass points and Daniel's documentation point.

*Greg:*

-

resolve_new_bindir(): dropped the last two comment lines.
-

prep_status text is now "Examining old cluster settings".
-

Test: since checksums are now the default, the old cluster is created
with --no-data-checksums (a genuinely non-default value) and --locale=C.
The post-upgrade checks confirm the new cluster inherited both, along with
the WAL segment size, encoding, collation, ctype, and locale provider.
-

Test: replaced open()/print() with append_conf, and the port/socket
settings now follow the framework's TCP-vs-Unix logic ($use_tcp), so
Windows works too.
-

-O: dropped support with --initdb entirely, as you originally suggested.
The partial "-c only" forwarding still broke on quoted values with spaces.
pg_upgrade now rejects -O + --initdb during option parsing, with a test and
a doc note.

*Daniel:*

-

Reworked the manual-initdb docs: the vague "use compatible initdb flags"
is now the specific settings that must match (WAL segment size, checksums,
encoding, locale), with a pointer to pg_controldata.

On in-tree vs. wrapper: these settings have to come from the old cluster's
control data, which pg_upgrade already reads, a wrapper would have to
rediscover the same information, and getting it wrong is exactly the
failure --initdb is meant to prevent.

The patch is pgindent-clean and the full pg_upgrade TAP suite passes
(including the new t/009_initdb_option.pl).

Registered in PG20-2. Branch on GitHub:
https://github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Thu, Jul 16, 2026 at 8:17 PM Greg Sabino Mullane <htamfids@gmail.com>
wrote:

Hi,

A few feedback about introducing the initdb option to pg_upgrade.

When I run the pg_upgrade command with the following options I'd
expect it to not create a directory and initdb.

```
pg_upgrade --check --initdb -d /old -D /new
```

So if the check is enabled it should print out the potential initdb
command/arguments/output and run the check. Because it can lead to
misunderstanding while executing the actual upgrade commands. And
operator/script has to remove the initdb directory before actual
upgrade.

When it comes to documentation we also need to add that the initdb
option starts the old server briefly to discover settings from
template0, then stops it before creating the new cluster. Moreover,
also mention that -B flag points outs to target version correctly
since there is a risk of doing an initdb with the wrong version. If -B
points to an older version, pg_upgrade may create the wrong cluster.

And the --initdb behaviour will introduce additional risks during the
upgrade. During the initdb the settings discovery connections are
allowed on database level and we don't have a guarantee that
checkpoint and vacuums won't start. This needs to be documented
clearly or we should consider eliminating them.

Regards,
Demir.

Bohyun Lee <bohyun.lee@databricks.com>, 17 Tem 2026 Cum, 14:10
tarihinde şunu yazdı:

On 17 Jul 2026, at 14:10, Bohyun Lee <bohyun.lee@databricks.com> wrote:

• Reworked the manual-initdb docs: the vague "use compatible initdb flags" is now the specific settings that must match (WAL segment size, checksums, encoding, locale), with a pointer to pg_controldata.

Regardless of the rest of the patch, I think we should apply something like
this to the docs. I propose you extract this portion into a separate 0001 in
this patchset so that it can be considered individually.

I haven't had a chance to look at the new version more than at skim-level but
the below caught my eye.

+	initPQExpBuffer(&cmd);
+	appendPQExpBuffer(&cmd, "\"%s/initdb\" -D \"%s\" -N",
+					  new_cluster.bindir, new_cluster.pgdata);
+	appendPQExpBuffer(&cmd, " -U \"%s\"", os_info.user);

This will inject the username specified in the (untrusted input) commandline
into the initdb command and execute it. While that will work in benign cases,
it seems like a Booby Tables pattern which is best left in the coding practices
of the past and in XKCD?

--
Daniel Gustafsson

On Wed, Aug 12, 2026 at 9:28 AM Hüseyin Demir <huseyin.d3r@gmail.com> wrote:

When I run the pg_upgrade command with the following options I'd
expect it to not create a directory and initdb.

```
pg_upgrade --check --initdb -d /old -D /new
```

I see your point. Maybe we don't allow --check with --initdb at all?

also mention that -B flag points outs to target version correctly

since there is a risk of doing an initdb with the wrong version. If -B
points to an older version, pg_upgrade may create the wrong cluster.

I'm not clear what pointing this out for --initdb only accomplishes. A
wrong -B is going to cause problems regardless of the mode.

And the --initdb behaviour will introduce additional risks during the

upgrade. During the initdb the settings discovery connections are
allowed on database level and we don't have a guarantee that
checkpoint and vacuums won't start. This needs to be documented
clearly or we should consider eliminating them.

Eliminating what exactly? And why would a vacuum or checkpoint cause
problems?

Cheers,
Greg

Hi,

When I run the pg_upgrade command with the following options I'd
expect it to not create a directory and initdb.

```
pg_upgrade --check --initdb -d /old -D /new
```

I see your point. Maybe we don't allow --check with --initdb at all?

Yes, that would make sense. My point is that, we should check if
--initdb is possible or not when the operator/tool executes it via the
--check option.

also mention that -B flag points outs to target version correctly
since there is a risk of doing an initdb with the wrong version. If -B
points to an older version, pg_upgrade may create the wrong cluster.

I'm not clear what pointing this out for --initdb only accomplishes. A wrong -B is going to cause problems regardless of the mode.

My point is that --new-bindir at the wrong major version's initdb, run
pg_upgrade --initdb. The wrong-version initdb succeeds and populates
new_cluster.pgdata fully. Then check_cluster_versions() fires
pg_fatal(), and the process exits with a fully-initialized,
wrong-version, orphaned data directory on disk — and no cleanup code
runs anywhere in the patch.

We need to enhance the patch so once it fails it should also clean up
the orphan directories. Because current failure in pg_upgrade doesn't
lead to orphan directories. Otherwise, if pg_upgrade fails and
operator wants to execute it again it won't be able to do it since the
directory already is not empty and initdb won't work. We need to
decide if pg_upgrade will wipe out orphan initdb directories if
pg_upgrade fails or leave it to the operator.

Summary of v5 changes

v4 had a shortcut that skipped re-reading the old cluster's control
data if it had already been read once. Turns out this was skipping a
real read later in the process that the upgrade actually depends on.
Removed the shortcut since it wasn't needed anyway, reading control
data is cheap.

Fixed the orphan directory problem. This was the main issue from the
review. If --initdb creates the new cluster but something fails
afterward (wrong binary version, etc.), the directory used to get left
behind, and a retry would fail because pg_upgrade refuses to overwrite
it. Now we automatically clean it up if something goes wrong, but only
up until the point where real data has actually been copied over.
After that, we leave it alone, because deleting real data is worse
than leaving an orphan folder.

--check --initdb now works as a dry run, instead of being blocked. It
tells you whether --initdb would succeed, checking the right binary
version, an empty target folder, and that the old cluster is
reachable, without actually creating anything. Added a version check
up front. As part of #3, we now check the new binary's version before
doing anything else. This is actually what stops the orphan directory
problem from happening in the first place, not just for the dry run.
If the version is wrong, we fail immediately, before touching the
disk.

Updated the docs to explain all of the above. Ran the full build and
test suite. New test file covers all the scenarios above, 26 out of 26
passing, no regressions elsewhere.

Patch attached. I've renamed the patch file to
v5-0001-pg_upgrade-initdb.patch to match our naming convention. No
content change, just the filename. Happy to walk through any of this
in more detail.

Regards,
Demir.

Patch attached. I've renamed the patch file to
v5-0001-pg_upgrade-initdb.patch to match our naming convention. No
content change, just the filename. Happy to walk through any of this
in more detail.

One more thing. While checking this against the CI build I found one
more small bug: a log message in the new --check --initdb dry-run path
had a trailing newline character it shouldn't have had, which tripped
an internal safety check and caused a crash — but only on that
stricter CI build, not on a normal build. Fixed by removing the extra
newline. v5 is now confirmed working on both build types.

Thanks, Demir, for v5 and the work on the orphan-directory cleanup and

the --check dry run, and thanks Daniel and Greg for the continued

review. Attached is v6. It builds on Demir's v5 and folds in the

remaining points below. Per Daniel's suggestion it is now split:

0001 - docs only: the manual "Initialize the new cluster" clarification

0002 - the --initdb feature (Demir's v5 plus the fixes below)

Responses are all inline.

On Tue, Aug 12, 2026, Daniel Gustafsson wrote:

+ appendPQExpBuffer(&cmd, " -U \"%s\"", os_info.user);

This will inject the username specified in the (untrusted input)

commandline into the initdb command and execute it. [...] it seems

like a Booby Tables pattern [...]

The initdb command is now assembled with appendShellString()

for every externally-derived value (i.e., the data directory, username,

bindir, and the encoding/locale strings), so shell metacharacters can

no longer break out of an argument.

Regardless of the rest of the patch, I think we should apply

something like this to the docs. I propose you extract this portion

into a separate 0001 in this patchset so that it can be considered

individually.

0001 is docs-only and stands on its own. It also corrects the

earlier implication that encoding and locale must match -- pg_upgrade

copies those from the old cluster, so only the WAL segment size and the

data-checksum setting actually need to match, which 0001 now states

with a pointer to pg_controldata.

On Wed, Aug 13, 2026, Greg Sabino Mullane wrote:

I see your point. Maybe we don't allow --check with --initdb at all?

v5 resolved this as a dry run rather than a hard block. --check

--initdb reports the initdb command it would run and validates the

preconditions (correct new-binary version, empty target directory, old

cluster reachable) without creating the new cluster. v6 keeps that,

and the docs now describe it.

I'm not clear what pointing this out for --initdb only accomplishes.

A wrong -B is going to cause problems regardless of the mode.

I agree, and I believe this meets Demir's point about orphan directories.

v5 checks the new-binary version up front, so a wrong -B fails before
anything is

written. An atexit handler removes the created directory if a later

check fails, up until the point where real data has been copied. So

there is no need for a -B note specific to --initdb in the docs.

v6 carries both.

Eliminating what exactly? And why would a vacuum or checkpoint cause

problems?

The old server is started here with the same start_postmaster() that

pg_upgrade uses elsewhere -- including -b, which disables autovacuum --

only to read template0's encoding and locale, then stopped before the

new cluster is created. So autovacuum will not run, and a checkpoint

would be harmless. It is a brief, read-only start using pg_upgrade's

existing mechanism, so it adds no risk beyond the old-server starts

pg_upgrade already performs. The docs now mention this brief start.

On Sun, Aug 17, 2026, Hüseyin Demir wrote:

a log message in the new --check --initdb dry-run path had a trailing

newline character it shouldn't have had [...] Fixed [...]

Carried into v6.

Beyond the review points, v6 also tightens two things I noticed while

reworking this:

- The empty-directory guard now rejects any non-empty target directory,

not just one already containing a PG_VERSION file. This closes a gap

in the orphan-cleanup path. The cleanup handler removes the whole new

data directory on failure, so it must not run against a directory that

already held the user's files.

- Because --initdb reads template0 by briefly starting the old server, it

now checks in advance that the old server is shut down and fails with the

usual "shut down that postmaster" message, rather than failing later

when pg_resetwal refuses to run against a live server.

Branch on GitHub: github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb

Best regards,

Bohyun

On Mon, Aug 17, 2026 at 9:44 AM Hüseyin Demir <huseyin.d3r@gmail.com> wrote:

Hi,

v5 resolved this as a dry run rather than a hard block. --check
--initdb reports the initdb command it would run and validates the
preconditions (correct new-binary version, empty target directory, old
cluster reachable) without creating the new cluster. v6 keeps that,
and the docs now describe it.

Thanks, appreciated.

I'm not clear what pointing this out for --initdb only accomplishes.
A wrong -B is going to cause problems regardless of the mode.

I agree, and I believe this meets Demir's point about orphan directories.
v5 checks the new-binary version up front, so a wrong -B fails before anything is
written. An atexit handler removes the created directory if a later
check fails, up until the point where real data has been copied. So
there is no need for a -B note specific to --initdb in the docs.
v6 carries both.

Got it, great.

Eliminating what exactly? And why would a vacuum or checkpoint cause
problems?

The old server is started here with the same start_postmaster() that
pg_upgrade uses elsewhere -- including -b, which disables autovacuum --
only to read template0's encoding and locale, then stopped before the
new cluster is created. So autovacuum will not run, and a checkpoint
would be harmless. It is a brief, read-only start using pg_upgrade's
existing mechanism, so it adds no risk beyond the old-server starts
pg_upgrade already performs. The docs now mention this brief start.

Thanks for handling this.

Beyond the review points, v6 also tightens two things I noticed while
reworking this:

- The empty-directory guard now rejects any non-empty target directory,
not just one already containing a PG_VERSION file. This closes a gap
in the orphan-cleanup path. The cleanup handler removes the whole new
data directory on failure, so it must not run against a directory that
already held the user's files.

- Because --initdb reads template0 by briefly starting the old server, it
now checks in advance that the old server is shut down and fails with the
usual "shut down that postmaster" message, rather than failing later
when pg_resetwal refuses to run against a live server.

Good catches, I have no objections to v6.

Regards,
Demir.

Hi Bohyun,

I applied v6 on master (798bdcae89d) and tested it. The normal path
works. I made the old cluster with --no-data-checksums --wal-segsize=32
--locale=C and the new cluster got all three right.

0001 is a good doc fix by itself. It could go in separately.

I found six things in 0002. A script that reproduces all of them is
attached. Run it as "sh initdb_repro.sh /path/to/bin /path/to/srcdir".

1. t/009_initdb_option.pl is not listed in
src/bin/pg_upgrade/meson.build, so it never runs in a meson build. I
added the line and all 26 subtests pass.

2. The group access setting from initdb -g is not copied from the old
cluster. The old cluster is drwxr-x--- and the new one is drwx------.
The exit code is 0 and there is no warning. The checksum and WAL
segment size are checked by pg_upgrade, but nothing checks this one, so
the upgrade quietly drops something the user asked for. Any tool that
reads the data directory as a member of the postgres group will stop
working.

3. resolve_new_bindir() is a nice cleanup, but the new callers pass
os_info.progname. That is get_progname(argv[0]), so it has no directory
in it. find_my_exec() then searches PATH instead of using argv[0]. If
the install directory is not in PATH and -B is not given, --initdb stops
with "could not find own program executable". The same command without
--initdb works fine. This is the fallback that lets people skip -B, so
it should get argv[0], like setup() does.

4. The <pgdata>.initdb_log directory is never deleted. It stays after a
successful run and after --check, and --retain has no effect on it.
Everywhere else pg_upgrade puts its logs inside the new data directory
and deletes them in cleanup_output_dirs(). Putting this one next to
pgdata also means the parent directory has to be writable, which
pg_upgrade does not need today.

5. A small note on the empty directory check. It stops initdb from
running where there are files, but the cleanup still calls rmtree() on
the directory itself. So a directory the operator made by hand is gone
after a failed run. Nothing is lost, since it had to be empty. But if
only the contents were deleted, initdb could still retry and the
directory would keep its owner and mode.

6. Two things about --check --initdb. I saw that v5 turned this into a
dry run instead of blocking it. First, the live check problem Daniel
reported for v2 is back. With the old cluster running, plain --check
says "Clusters are compatible", while --check --initdb says "There seems
to be a postmaster servicing the old cluster". Second, the dry run
returns 0 for a cluster that cannot be upgraded. With a regproc column
in the old cluster, --check --initdb returns 0 and the real run returns
1.

I am happy to write patches for any of these.

Thanks,
Shihao