Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard to read.

From: Peter Smith <[email-masked]>
Date: Thu, 12 Dec 2024 13:32:19 +1100

On Thu, Dec 12, 2024 at 12:41 PM Tom Lane <[email-masked]> wrote:
>
> Peter Smith <[email-masked]> writes:
> > While reviewing a patch for another pg_createsubscriber thread I found
> > that multiple TAP tests have a terrible wrapping where the command
> > options and their associated oparg are separated on different lines
> > instead of paired together nicely. This makes it unnecessarily
> > difficult to see what the test is doing.
>
> I think that is mostly the fault of pgperltidy.  We did change
> the options we use with it awhile back, so maybe now it will honor
> your manual changes to its line-wrapping choices.  But I wouldn't
> bet on that.  Did you check what happens if you run the modified
> code through pgperltidy?
>
> (If the answer is bad, we could look into making further changes so
> that pgperltidy won't override these decisions.  But there's no point
> in manually patching this if it'll just get undone.)
>

Thanks for your suggestion. As you probably suspected, the answer is bad.

I ran pgperltidy on the "fixed" file:
[postgres@CentOS7-x64 oss_postgres_misc]$
src/tools/pgindent/pgperltidy
src/bin/pg_basebackup/t/040_pg_createsubscriber.pl

This reverted it to how it currently looks on master.

The strange thing is there are other commands in that file very
similar to the ones I had changed but those already looked good, yet
they remained unaffected by the pgperltidy. Why?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Tom Lane <[email-masked]>
Date: Wed, 11 Dec 2024 21:46:26 -0500

> they remained unaffected by the pgperltidy. Why?

You sure it's not just luck-of-the-draw?  I think that perltidy
is just splitting the lines based on length, so sometimes related
options would be kept together and sometimes not.

			regards, tom lane

---

From: Peter Smith <[email-masked]>
Date: Thu, 12 Dec 2024 14:13:40 +1100

> options would be kept together and sometimes not.

TBH, I have no idea what logic perltidy uses. I did find some
configurations here [1] (are those what it pgperltidy uses?) but those
claim max line length is 78 which I didn;t come anywhere near
exceeding.

After some more experimentation, I've noticed that it is trying to
keep only 2 items on each line. So whether it looks good or not seems
to depend if there is an even or odd number of options without
arguments up-front. Maybe those perltidy "tightness" switches?

So, AFAICT I can workaround the perltidy wrapping just by putting all
the noarg options at the bottom of the command, then all the
option/optarg pairs (ie 2s) will stay together. I can post another
patch to do it this way unless you think it is too hacky.

======
[1] https://github.com/postgres/postgres/blob/master/src/tools/pgindent/perltidyrc

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Michael Paquier <[email-masked]>
Date: Thu, 12 Dec 2024 12:53:13 +0900

On Thu, Dec 12, 2024 at 02:13:40PM +1100, Peter Smith wrote:
> claim max line length is 78 which I didn't come anywhere near
> exceeding.

Gave up trying to understand its internals and its rules years ago.
It's complicated enough that we require a specific version of the tool
for the tree with a custom PATH :D

> arguments up-front. Maybe those perltidy "tightness" switches?

Don't recall so even under Perl-Tidy-20230309, because it comes down
to the number of elements in these arrays and the length of their
values, not the specific values in each element of the array.

> patch to do it this way unless you think it is too hacky.

This trick works for me if that makes the long list of option easier
to read.  With two elements of the array perl line, I would just put
some --dry-run or --verbose at the end of their respective arrays.
--
Michael

---

From: Peter Smith <[email-masked]>
Date: Thu, 12 Dec 2024 15:24:42 +1100

On Thu, Dec 12, 2024 at 2:53 PM Michael Paquier <[email-masked]> wrote:
...

> some --dry-run or --verbose at the end of their respective arrays.

Hi Michael.

Yes, that is the workaround that I was proposing.

PSA v2-0001. This time it can survive pgperltidy unchanged.

In passing I also removed the duplicated '--verbose' and changed the
'Pub2' case mentioned in the original post.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Michael Paquier <[email-masked]>
Date: Thu, 12 Dec 2024 14:02:13 +0900

> PSA v2-0001. This time it can survive pgperltidy unchanged.

Confirmed.  It looks to apply cleanly to v17 as well.  Better to
backpatch to avoid conflict frictions, even if it's cosmetic.

> 'Pub2' case mentioned in the original post.

Right.  I didn't notice this dup in "run pg_createsubscriber on node
S".  All the commands of the test seem to be in order with what you've
attached, so LGTM.
--
Michael

---

From: Alvaro Herrera <[email-masked]>
Date: Thu, 12 Dec 2024 11:02:12 +0100

> backpatch to avoid conflict frictions, even if it's cosmetic.

I'd rather use #<<< and #>>> markers.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Thu, 12 Dec 2024 12:04:16 +0000

> Yes, that is the workaround that I was proposing.

A better option, IMO, is to use the fat comma (=>) between options and
their values.  This makes it clear both to humans and perltidy that they
belong together, and we can put all the valueless options first without
things being rewrapped.

- ilmari

>From e972f1a5f87499390f401e7db2c079fb87533553 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <[email-masked]>
Date: Thu, 12 Dec 2024 11:56:15 +0000
Subject: [PATCH v3] Modify wrapping to make command options easier to read

Use fat comma after options that take values, both to make it clearer
to humans, and to stop perltidy from re-wrapping them.
---
 .../t/040_pg_createsubscriber.pl              | 169 +++++++++---------
 1 file changed, 89 insertions(+), 80 deletions(-)

diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 0a900edb656..0f7bb103177 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -168,41 +168,44 @@ sub generate_db
 # Run pg_createsubscriber on a promoted server
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_t->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_t->host, '--subscriber-port',
-		$node_t->port, '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_t->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_t->host,
+		'--subscriber-port' => $node_t->port,
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'target server is not in recovery');
 
 # Run pg_createsubscriber when standby is running
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'standby is up and running');
 
 # Run pg_createsubscriber on about-to-fail node F
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $node_f->data_dir,
-		'--publisher-server', $node_p->connstr($db1),
-		'--socketdir', $node_f->host,
-		'--subscriber-port', $node_f->port,
-		'--database', $db1,
-		'--database', $db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $node_f->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_f->host,
+		'--subscriber-port' => $node_f->port,
+		'--database' => $db1,
+		'--database' => $db2
 	],
 	'subscriber data directory is not a copy of the source database cluster');
 
@@ -216,14 +219,15 @@ sub generate_db
 # Run pg_createsubscriber on node C (P -> S -> C)
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_c->data_dir, '--publisher-server',
-		$node_s->connstr($db1), '--socketdir',
-		$node_c->host, '--subscriber-port',
-		$node_c->port, '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_c->data_dir,
+		'--publisher-server' => $node_s->connstr($db1),
+		'--socketdir' => $node_c->host,
+		'--subscriber-port' => $node_c->port,
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'primary server is in recovery');
 
@@ -239,14 +243,16 @@ sub generate_db
 $node_s->stop;
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--database' => $db1,
+		'--database' => $db2,
+
 	],
 	'primary contains unmet conditions on node P');
 # Restore default settings here but only apply it after testing standby. Some
@@ -268,14 +274,15 @@ sub generate_db
 });
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'standby contains unmet conditions on node S');
 $node_s->append_conf(
@@ -321,19 +328,20 @@ sub generate_db
 # dry run mode on node S
 command_ok(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
-		'--dry-run', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--publication',
-		'pub1', '--publication',
-		'pub2', '--subscription',
-		'sub1', '--subscription',
-		'sub2', '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--publication' => 'pub1',
+		'--publication' => 'pub2',
+		'--subscription' => 'sub1',
+		'--subscription' => 'sub2',
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'run pg_createsubscriber --dry-run on node S');
 
@@ -346,32 +354,33 @@ sub generate_db
 # pg_createsubscriber can run without --databases option
 command_ok(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--dry-run', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--replication-slot',
-		'replslot1'
+		'pg_createsubscriber',
+		'--verbose',
+		'--dry-run',
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--replication-slot' => 'replslot1',
 	],
 	'run pg_createsubscriber without --databases');
 
 # Run pg_createsubscriber on node S
 command_ok(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
-		'--verbose', '--pgdata',
-		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr($db1), '--socketdir',
-		$node_s->host, '--subscriber-port',
-		$node_s->port, '--publication',
-		'pub1', '--publication',
-		'Pub2', '--replication-slot',
-		'replslot1', '--replication-slot',
-		'replslot2', '--database',
-		$db1, '--database',
-		$db2
+		'pg_createsubscriber',
+		'--verbose',
+		'--recovery-timeout' => "$PostgreSQL::Test::Utils::timeout_default",
+		'--pgdata' => $node_s->data_dir,
+		'--publisher-server' => $node_p->connstr($db1),
+		'--socketdir' => $node_s->host,
+		'--subscriber-port' => $node_s->port,
+		'--publication' => 'pub1',
+		'--publication' => 'pub2',
+		'--replication-slot' => 'replslot1',
+		'--replication-slot' => 'replslot2',
+		'--database' => $db1,
+		'--database' => $db2,
 	],
 	'run pg_createsubscriber on node S');
 
-- 
2.47.1

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Thu, 12 Dec 2024 13:17:48 +0000

> belong together, and we can put all the valueless options first without

Here's a more thorough patch, that also applies the fat comma treatment
to other pg_createsubscriber invocations in the same file that don't
currently happen to be mangled by perltidy.  It also adds trailing
commas to the last item in multi-line command arrays, which is common
perl style.

- ilmari

>From 953d0c8ca8202d6f53af833fd53e3f6b1929fa77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <[email-masked]>
Date: Thu, 12 Dec 2024 11:56:15 +0000
Subject: [PATCH v4] Modify wrapping to make command options easier to read

Also remove pointless quoting of $PostgreSQL::Test::Utils::timeout_default.
---
 .../t/040_pg_createsubscriber.pl              | 255 +++++++++---------
 1 file changed, 135 insertions(+), 120 deletions(-)

diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 0a900edb656..369846db0d0 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -46,69 +46,75 @@ sub generate_db
 command_fails(['pg_createsubscriber'],
 	'no subscriber data directory specified');
 command_fails(
-	[ 'pg_createsubscriber', '--pgdata', $datadir ],
+	[ 'pg_createsubscriber', '--pgdata' => $datadir ],
 	'no publisher connection string specified');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
 	],
 	'no database name specified');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432',
-		'--database', 'pg1',
-		'--database', 'pg1'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
+		'--database' => 'pg1',
+		'--database' => 'pg1',
 	],
 	'duplicate database name');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432',
-		'--publication', 'foo1',
-		'--publication', 'foo1',
-		'--database', 'pg1',
-		'--database', 'pg2'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
+		'--publication' => 'foo1',
+		'--publication' => 'foo1',
+		'--database' => 'pg1',
+		'--database' => 'pg2',
 	],
 	'duplicate publication name');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432',
-		'--publication', 'foo1',
-		'--database', 'pg1',
-		'--database', 'pg2'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
+		'--publication' => 'foo1',
+		'--database' => 'pg1',
+		'--database' => 'pg2',
 	],
 	'wrong number of publication names');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432',
-		'--publication', 'foo1',
-		'--publication', 'foo2',
-		'--subscription', 'bar1',
-		'--database', 'pg1',
-		'--database', 'pg2'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
+		'--publication' => 'foo1',
+		'--publication' => 'foo2',
+		'--subscription' => 'bar1',
+		'--database' => 'pg1',
+		'--database' => 'pg2',
 	],
 	'wrong number of subscription names');
 command_fails(
 	[
-		'pg_createsubscriber', '--verbose',
-		'--pgdata', $datadir,
-		'--publisher-server', 'port=5432',
-		'--publication', 'foo1',
-		'--publication', 'foo2',
-		'--subscription', 'bar1',
-		'--subscription', 'bar2',
-		'--replication-slot', 'baz1',
-		'--database', 'pg1',
-		'--database', 'pg2'
+		'pg_createsubscriber',
+		'--verbose',
+		'--pgdata' => $datadir,
+		'--publisher-server' => 'port=5432',
+		'--publication' => 'foo1',
+		'--publication' => 'foo2',
+		'--subscription' => 'bar1',
+		'--subscription' => 'bar2',
+		'--replication-slot' => 'baz1',
+		'--database' => 'pg1',
+		'--database' => 'pg2',
 	],
 	'wrong number of replication slot names');
 
@@ -168,41 +174,44 @@ sub generate_db

 
@@ -216,14 +225,15 @@ sub generate_db
 
@@ -239,14 +249,16 @@ sub generate_db
 $node_s->stop;
 command_fails(
 	[
@@ -268,14 +280,15 @@ sub generate_db
@@ -321,19 +334,20 @@ sub generate_db
 
@@ -346,32 +360,33 @@ sub generate_db

-- 
2.47.1

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 12 Dec 2024 10:28:38 -0500

On 2024-12-12 Th 8:17 AM, Dagfinn Ilmari Mannsåker wrote:
> Dagfinn Ilmari Mannsåker<[email-masked]>  writes:
>
>> Peter Smith<[email-masked]>  writes:
>>
>>> On Thu, Dec 12, 2024 at 2:53 PM Michael Paquier<[email-masked]>  wrote:
>>> ...
>>>

+1 for this approach.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

---

From: Tom Lane <[email-masked]>
Date: Thu, 12 Dec 2024 10:35:15 -0500

> +1 for this approach.

Indeed, this is much nicer if it's something perltidy knows about.

However, I know we have the same issue in many other places.
Anyone feel like running through all the TAP scripts?

			regards, tom lane

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Thu, 12 Dec 2024 17:08:01 +0000

> Anyone feel like running through all the TAP scripts?

I can have a go in the next few days.  A quick grep spotted another
workaround in 027_stream_regress.pl: using paretheses around the option
and its value:

command_ok(
	[
		'pg_dump',
		('--schema', 'pg_catalog'),
		('-f', $outputdir . '/catalogs_primary.dump'),
		'--no-sync',
		('-p', $node_primary->port),
		'--no-unlogged-table-data',
		'regression'
	],
	'dump catalogs of primary server');

I think the fat comma is much nicer than this, so I'd like to convert
these too (and replace some of the concatenations with interpolation).

Technically the quotes aren't necessary around single-dash options
before => since unary minus works on strings as well as numbers, but
I'll leave them in for consistency.

- ilmari

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 12 Dec 2024 12:52:09 -0500

> I'll leave them in for consistency.

I'd rather get rid of those and just use the long options.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Thu, 12 Dec 2024 18:03:33 +0000

> I'd rather get rid of those and just use the long options.

Yeah, that is more self-documenting, so I'll do that while I'm at it. 

-- 
- ilmari

---

From: Michael Paquier <[email-masked]>
Date: Fri, 13 Dec 2024 09:40:43 +0900

> Yeah, that is more self-documenting, so I'll do that while I'm at it. 

Agreed.  I tend to prefer long options too.  That's less mapping to
think about the intentions behind some tests when reading their code.

The idea of using fat commas is neat, Ilmari.  I didn't suspect that
this one was possible to trick perltidy.
--
Michael

---

From: Peter Smith <[email-masked]>
Date: Fri, 13 Dec 2024 11:55:52 +1100

> Yeah, that is more self-documenting, so I'll do that while I'm at it.

Your fat-comma solution is much better than something I could have
come up with. Thanks for taking this on.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Mon, 13 Jan 2025 12:37:25 +1100

> Yeah, that is more self-documenting, so I'll do that while I'm at it.

Hi. This thread has been silent for 1 month. Just wondering, has any
progress been made on these changes?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Tue, 14 Jan 2025 18:03:23 +1100

> commas to the last item in multi-line command arrays, which is common

Hi,

In your v4 patch, there is a fragment (below) that replaces a  double
'--verbose' switch with just a single '--verbose'.

As I have only recently learned, the '--verbose'' switch has a
cumulative effect [1], so the original double '--verbose' was probably
deliberate so it should be kept that way.

~~

======
[1] https://www.postgresql.org/docs/devel/app-pgcreatesubscriber.html

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Mon, 20 Jan 2025 20:42:45 +0000

Hi Peter,

> deliberate so it should be kept that way.

I was going to say that if it were deliberate, I would have expected the
two `--verbose` swithces to be together, but then I noticed that the
--recovery-timeout switch was added later (commit 04c8634c0c4d), rather
haphazardly between them.  Still, I would have expected a comment as to
why this command was being invoked extra-verbosely, but I'll restore it
in the next version.

I've Cc-ed both Peter (the committer) and Euler (the author) in case
they have any insight.