prevent 006_transfer_modes.pl from leaving files behind

Started by Nathan Bossartabout 1 year ago6 messageshackers
Jump to latest
#1Nathan Bossart
nathandbossart@gmail.com

The other pg_upgrade tests chdir to tmp_check prior to running pg_upgrade
to avoid leaving behind delete_old_cluster.{sh,bat}. 006_transfer_modes.pl
should, too. However, this test is a little different because it loops
over all of the available transfer modes and runs pg_upgrade for each one
supported by the platform. From my testing and code analysis, it seems
sufficient to change the directory once at the beginning of the test, but
we could alternatively save the current directory and change back to it in
each iteration to be safe.

Thoughts?

--
nathan

Attachments:

v1-0001-Fix-the-006_transfer_modes.pl-test.patchtext/plain; charset=us-asciiDownload+5-1
#2Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#1)
Re: prevent 006_transfer_modes.pl from leaving files behind

On Mon, Apr 07, 2025 at 04:45:52PM -0500, Nathan Bossart wrote:

The other pg_upgrade tests chdir to tmp_check prior to running pg_upgrade
to avoid leaving behind delete_old_cluster.{sh,bat}. 006_transfer_modes.pl
should, too. However, this test is a little different because it loops
over all of the available transfer modes and runs pg_upgrade for each one
supported by the platform. From my testing and code analysis, it seems
sufficient to change the directory once at the beginning of the test, but
we could alternatively save the current directory and change back to it in
each iteration to be safe.

Thoughts?

Hmm. Doing one chdir at the beginning of the test should be OK,
because we don't do a move to a different directory within the test
for each transfer mode tested. So your patch looks like the simplest
thing to do to avoid the generation of these files. Note that
delete_old_cluster.sh would be left around even if not using a VPATH
build with ./configure (your commit message does not mention that).
Even if .gitignore discards it, the file is here.
--
Michael

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Michael Paquier (#2)
Re: prevent 006_transfer_modes.pl from leaving files behind

On 2025-04-07 Mo 7:41 PM, Michael Paquier wrote:

On Mon, Apr 07, 2025 at 04:45:52PM -0500, Nathan Bossart wrote:

The other pg_upgrade tests chdir to tmp_check prior to running pg_upgrade
to avoid leaving behind delete_old_cluster.{sh,bat}. 006_transfer_modes.pl
should, too. However, this test is a little different because it loops
over all of the available transfer modes and runs pg_upgrade for each one
supported by the platform. From my testing and code analysis, it seems
sufficient to change the directory once at the beginning of the test, but
we could alternatively save the current directory and change back to it in
each iteration to be safe.

Thoughts?

Hmm. Doing one chdir at the beginning of the test should be OK,
because we don't do a move to a different directory within the test
for each transfer mode tested. So your patch looks like the simplest
thing to do to avoid the generation of these files. Note that
delete_old_cluster.sh would be left around even if not using a VPATH
build with ./configure (your commit message does not mention that).
Even if .gitignore discards it, the file is here.

I don't think that matters. In non-vpath builds we expect the source
directory to be scribbled on. All sorts of things might be left around.

cheers

andrew

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#3)
Re: prevent 006_transfer_modes.pl from leaving files behind

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-04-07 Mo 7:41 PM, Michael Paquier wrote:

delete_old_cluster.sh would be left around even if not using a VPATH
build with ./configure (your commit message does not mention that).
Even if .gitignore discards it, the file is here.

I don't think that matters. In non-vpath builds we expect the source
directory to be scribbled on. All sorts of things might be left around.

It's okay as long as .gitignore ignores it and "make clean" removes it.

When/if we give up makefile builds, a lot of these maintenance
issues will go away...

regards, tom lane

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#4)
Re: prevent 006_transfer_modes.pl from leaving files behind

On 2025-04-08 Tu 10:17 AM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-04-07 Mo 7:41 PM, Michael Paquier wrote:

delete_old_cluster.sh would be left around even if not using a VPATH
build with ./configure (your commit message does not mention that).
Even if .gitignore discards it, the file is here.

I don't think that matters. In non-vpath builds we expect the source
directory to be scribbled on. All sorts of things might be left around.

It's okay as long as .gitignore ignores it and "make clean" removes it.

When/if we give up makefile builds, a lot of these maintenance
issues will go away...

Maybe some will, but I observed the present issue on fairywren which is
using meson.

cheers

andrew

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

#6Nathan Bossart
nathandbossart@gmail.com
In reply to: Andrew Dunstan (#5)
Re: prevent 006_transfer_modes.pl from leaving files behind

Committed.

--
nathan