Re: 010_pg_basebackup.pl vs multiple filesystems

Hi,

On 2024-07-07 09:10:48 -0400, Andrew Dunstan wrote:
> On 2024-07-07 Su 7:28 AM, Andrew Dunstan wrote:
> > I'll be happy to hear of one. I agree it's a mess.  Maybe we could test
> > that the temp directory is on the same device on Windows and skip the
> > test if not? You could still get the test to run by setting TMPDIR
> > and/or friends.

> Maybe we should just not try to rename the directory. Looking at the test
> I'm pretty sure the directory should be empty. Instead of trying to move it,
> let's just remove it, and recreate it in the tmp location.

Good catch, yes, that'd be much better!

> diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> index 489dde4adf..c0c334c6fc 100644
> --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> @@ -363,8 +363,8 @@ my $sys_tempdir =
> PostgreSQL::Test::Utils::tempdir_short;
>  # Elsewhere use $tempdir to avoid file system boundary issues with moving.
>  my $tmploc = $windows_os ? $sys_tempdir : $tempdir;

The comment would need a bit of editing, I guess. I think we should consider
just getting rid of the os-dependant switch now, it shouldn't be needed
anymore?

> -rename("$pgdata/pg_replslot", "$tmploc/pg_replslot")
> -  or BAIL_OUT "could not move $pgdata/pg_replslot";
> +rmtree("$pgdata/pg_replslot");

Should this perhaps be an rmdir, to ensure that we're not removing something
we don't want (e.g. somebody adding an earlier test for slots that then gets
broken by the rmtree)?

Greetings,

Andres Freund