On 7/14/20 1:31 AM, Michael Paquier wrote:
> On Fri, Jul 10, 2020 at 07:58:02AM -0400, Andrew Dunstan wrote:
>> After much frustration and gnashing of teeth here's a patch that allows
>> almost all the TAP tests involving symlinks to work as expected on all
>> Windows build environments, without requiring an additional Perl module.
>> I have tested this on a system that is very similar to that running
>> drongo and fairywren, with both msys2 and MSVC builds.
> Thanks Andrew for looking at the part with MSYS. The tests pass for
> me with MSVC. The trick with mklink is cool. I have not considered
> that, and the test code gets simpler.
>
> + my $cmd = qq{mklink /j "$newname" "$oldname"};
> + if ($Config{osname} eq 'msys')
> + {
> + # need some indirection on msys
> + $cmd = qq{echo '$cmd' | \$COMSPEC /Q};
> + }
> + note("dir_symlink cmd: $cmd");
> + system($cmd);
> From the quoting perspective, wouldn't it be simpler to build an array
> with all those arguments and call system() with @cmd?
This is the simplest invocation I found to be reliable on msys2 (and it
took me a long time to find). If you have a tested alternative please
let me know.
> +# Create files that look like temporary relations to ensure they are ignored
> +# in a tablespace.
> +my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
> This variable conflicts with a previous declaration, creating a
> warning.
>
> + skip "symlink check not implemented on Windows", 1
> + if ($windows_os);
> opendir(my $dh, "$pgdata/pg_tblspc") or die;
> I think that this would be cleaner with a SKIP block.
I don't understand this comment. The skip statement here is in a SKIP
block. In fact skip only works inside SKIP blocks. (perldoc Test::More
for details). Maybe you got confused by the diff format.
>
> +Portably create a symlink for a director. On Windows this creates a junction.
> +Elsewhere it just calls perl's builtin symlink.
> s/director/directory/
> s/junction/junction point/
fixed.
>
> <para>
> The TAP tests require the Perl module <literal>IPC::Run</literal>.
> This module is available from CPAN or an operating system package.
> + On Windows, <literal>Win32API::File</literal> is also required .
> </para>
> This part should be backpatched IMO.
I will do this in a separate backpatched commit.
>
> Some of the indentation is weird, this needs a cleanup with perltidy.
Done.
Revised patch attached.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 208df557b8..f674a7c94e 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -211,87 +211,93 @@ $node->command_fails(
'pg_basebackup tar with long name fails');
unlink "$pgdata/$superlongname";
-# The following tests test symlinks. Windows doesn't have symlinks, so
-# skip on Windows.
+# The following tests are for symlinks.
+
+# Move pg_replslot out of $pgdata and create a symlink to it.
+$node->stop;
+
+# Set umask so test directories and files are created with group permissions
+umask(0027);
+
+# Enable group permissions on PGDATA
+chmod_recursive("$pgdata", 0750, 0640);
+
+rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
+ or BAIL_OUT "could not move $pgdata/pg_replslot";
+dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
+ or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
+
+$node->start;
+
+# Create a temporary directory in the system location and symlink it
+# to our physical temp location. That way we can use shorter names
+# for the tablespace directories, which hopefully won't run afoul of
+# the 99 character length limit.
+my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
+dir_symlink "$tempdir", $shorter_tempdir;
+
+mkdir "$tempdir/tblspc1";
+my $realTsDir = TestLib::perl2host("$shorter_tempdir/tblspc1");
+my $real_tempdir = TestLib::perl2host($tempdir);
+$node->safe_psql('postgres',
+ "CREATE TABLESPACE tblspc1 LOCATION '$realTsDir';");
+$node->safe_psql('postgres',
+ "CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
+$node->command_ok(
+ [ 'pg_basebackup', '-D', "$real_tempdir/tarbackup2", '-Ft' ],
+ 'tar format with tablespaces');
+ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
+my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
+is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
+rmtree("$tempdir/tarbackup2");
+
+# Create an unlogged table to test that forks other than init are not copied.
+$node->safe_psql('postgres',
+ 'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;');
+
+my $tblspc1UnloggedPath = $node->safe_psql('postgres',
+ q{select pg_relation_filepath('tblspc1_unlogged')});
+
+# Make sure main and init forks exist
+ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
+ 'unlogged init fork in tablespace');
+ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
+
+# Create files that look like temporary relations to ensure they are ignored
+# in a tablespace.
+@tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+my $tblSpc1Id = basename(
+ dirname(
+ dirname(
+ $node->safe_psql(
+ 'postgres', q{select pg_relation_filepath('test1')}))));
+
+foreach my $filename (@tempRelationFiles)
+{
+ append_to_file(
+ "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+ 'TEMP_RELATION');
+}
+
+$node->command_fails(
+ [ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
+ 'plain format with tablespaces fails without tablespace mapping');
+
+$node->command_ok(
+ [
+ 'pg_basebackup', '-D',
+ "$tempdir/backup1", '-Fp',
+ "-T$realTsDir=$real_tempdir/tbackup/tblspc1"
+ ],
+ 'plain format with tablespaces succeeds with tablespace mapping');
+ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+
+# This symlink check is not supported on Windows as -l
+# doesn't work with junctions
SKIP:
{
- skip "symlinks not supported on Windows", 18 if ($windows_os);
-
- # Move pg_replslot out of $pgdata and create a symlink to it.
- $node->stop;
-
- # Set umask so test directories and files are created with group permissions
- umask(0027);
-
- # Enable group permissions on PGDATA
- chmod_recursive("$pgdata", 0750, 0640);
-
- rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
- or BAIL_OUT "could not move $pgdata/pg_replslot";
- symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
- or BAIL_OUT "could not symlink to $pgdata/pg_replslot";
-
- $node->start;
-
- # Create a temporary directory in the system location and symlink it
- # to our physical temp location. That way we can use shorter names
- # for the tablespace directories, which hopefully won't run afoul of
- # the 99 character length limit.
- my $shorter_tempdir = TestLib::tempdir_short . "/tempdir";
- symlink "$tempdir", $shorter_tempdir;
-
- mkdir "$tempdir/tblspc1";
- $node->safe_psql('postgres',
- "CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
- $node->safe_psql('postgres',
- "CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
- $node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
- 'tar format with tablespaces');
- ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
- my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
- is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
- rmtree("$tempdir/tarbackup2");
-
- # Create an unlogged table to test that forks other than init are not copied.
- $node->safe_psql('postgres',
- 'CREATE UNLOGGED TABLE tblspc1_unlogged (id int) TABLESPACE tblspc1;'
- );
-
- my $tblspc1UnloggedPath = $node->safe_psql('postgres',
- q{select pg_relation_filepath('tblspc1_unlogged')});
-
- # Make sure main and init forks exist
- ok( -f "$pgdata/${tblspc1UnloggedPath}_init",
- 'unlogged init fork in tablespace');
- ok(-f "$pgdata/$tblspc1UnloggedPath", 'unlogged main fork in tablespace');
-
- # Create files that look like temporary relations to ensure they are ignored
- # in a tablespace.
- my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
- my $tblSpc1Id = basename(
- dirname(
- dirname(
- $node->safe_psql(
- 'postgres', q{select pg_relation_filepath('test1')}))));
-
- foreach my $filename (@tempRelationFiles)
- {
- append_to_file(
- "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
- 'TEMP_RELATION');
- }
-
- $node->command_fails(
- [ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
- 'plain format with tablespaces fails without tablespace mapping');
-
- $node->command_ok(
- [
- 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
- "-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1"
- ],
- 'plain format with tablespaces succeeds with tablespace mapping');
- ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
+ skip "symlink check not implemented on Windows", 1
+ if ($windows_os);
opendir(my $dh, "$pgdata/pg_tblspc") or die;
ok( ( grep {
-l "$tempdir/backup1/pg_tblspc/$_"
@@ -300,65 +306,73 @@ SKIP:
} readdir($dh)),
"tablespace symlink was updated");
closedir $dh;
+}
+
+# Group access should be enabled on all backup files
+SKIP:
+{
+ skip "unix-style permissions not supported on Windows", 1
+ if ($windows_os);
- # Group access should be enabled on all backup files
ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
"check backup dir permissions");
+}
+
+# Unlogged relation forks other than init should not be copied
+my ($tblspc1UnloggedBackupPath) =
+ $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
+
+ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
+ 'unlogged init fork in tablespace backup');
+ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
+ 'unlogged main fork not in tablespace backup');
- # Unlogged relation forks other than init should not be copied
- my ($tblspc1UnloggedBackupPath) =
- $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g;
-
- ok(-f "$tempdir/tbackup/tblspc1/${tblspc1UnloggedBackupPath}_init",
- 'unlogged init fork in tablespace backup');
- ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
- 'unlogged main fork not in tablespace backup');
-
- # Temp relations should not be copied.
- foreach my $filename (@tempRelationFiles)
- {
- ok( !-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
- "[tblspc1]/$postgresOid/$filename not copied");
-
- # Also remove temp relation files or tablespace drop will fail.
- my $filepath =
- "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
-
- unlink($filepath)
- or BAIL_OUT("unable to unlink $filepath");
- }
-
- ok( -d "$tempdir/backup1/pg_replslot",
- 'pg_replslot symlink copied as directory');
- rmtree("$tempdir/backup1");
-
- mkdir "$tempdir/tbl=spc2";
- $node->safe_psql('postgres', "DROP TABLE test1;");
- $node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
- $node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
- $node->safe_psql('postgres',
- "CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
- $node->command_ok(
- [
- 'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
- "-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2"
- ],
- 'mapping tablespace with = sign in path');
- ok(-d "$tempdir/tbackup/tbl=spc2",
- 'tablespace with = sign was relocated');
- $node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
- rmtree("$tempdir/backup3");
-
- mkdir "$tempdir/$superlongname";
- $node->safe_psql('postgres',
- "CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
- $node->command_ok(
- [ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
- 'pg_basebackup tar with long symlink target');
- $node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
- rmtree("$tempdir/tarbackup_l3");
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+ ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+ "[tblspc1]/$postgresOid/$filename not copied");
+
+ # Also remove temp relation files or tablespace drop will fail.
+ my $filepath =
+ "$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+ unlink($filepath)
+ or BAIL_OUT("unable to unlink $filepath");
}
+ok( -d "$tempdir/backup1/pg_replslot",
+ 'pg_replslot symlink copied as directory');
+rmtree("$tempdir/backup1");
+
+mkdir "$tempdir/tbl=spc2";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/tbl=spc2");
+$node->safe_psql('postgres', "DROP TABLE test1;");
+$node->safe_psql('postgres', "DROP TABLE tblspc1_unlogged;");
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc1;");
+$node->safe_psql('postgres',
+ "CREATE TABLESPACE tblspc2 LOCATION '$realTsDir';");
+$realTsDir =~ s/=/\\=/;
+$node->command_ok(
+ [
+ 'pg_basebackup', '-D',
+ "$tempdir/backup3", '-Fp',
+ "-T$realTsDir=$real_tempdir/tbackup/tbl\\=spc2"
+ ],
+ 'mapping tablespace with = sign in path');
+ok(-d "$tempdir/tbackup/tbl=spc2", 'tablespace with = sign was relocated');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc2;");
+rmtree("$tempdir/backup3");
+
+mkdir "$tempdir/$superlongname";
+$realTsDir = TestLib::perl2host("$shorter_tempdir/$superlongname");
+$node->safe_psql('postgres',
+ "CREATE TABLESPACE tblspc3 LOCATION '$realTsDir';");
+$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+ 'pg_basebackup tar with long symlink target');
+$node->safe_psql('postgres', "DROP TABLESPACE tblspc3;");
+rmtree("$tempdir/tarbackup_l3");
+
$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
'pg_basebackup -R runs');
ok(-f "$tempdir/backupR/postgresql.auto.conf", 'postgresql.auto.conf exists');
@@ -496,7 +510,7 @@ my $file_corrupt2 = $node->safe_psql('postgres',
# set page header and block sizes
my $pageheader_size = 24;
-my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
+my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
# induce corruption
system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
index abdb07c558..5497e46056 100644
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -5,7 +5,7 @@ use PostgresNode;
use TestLib;
use Test::More;
-if ($^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/)
+if ($TestLib::is_msys2)
{
plan skip_all => 'High bit name tests fail on Msys2';
}
@@ -27,7 +27,7 @@ $ENV{PGCLIENTENCODING} = 'LATIN1';
# The odds of finding something interesting by testing all ASCII letters
# seem too small to justify the cycles of testing a fifth name.
my $dbname1 =
- 'regression'
+ 'regression'
. generate_ascii_string(1, 9)
. generate_ascii_string(11, 12)
. generate_ascii_string(14, 33)
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 3813543ee1..fff4758508 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -6,16 +6,7 @@ use warnings;
use File::Copy;
use File::Path qw(rmtree);
use TestLib;
-use Test::More;
-if ($windows_os)
-{
- plan skip_all => 'symlinks not supported on Windows';
- exit;
-}
-else
-{
- plan tests => 5;
-}
+use Test::More tests => 5;
use FindBin;
use lib $FindBin::RealBin;
@@ -36,7 +27,7 @@ sub run_test
# turn pg_wal into a symlink
print("moving $test_primary_datadir/pg_wal to $primary_xlogdir\n");
move("$test_primary_datadir/pg_wal", $primary_xlogdir) or die;
- symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
+ dir_symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
RewindTest::start_primary();
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index a7490d2ce7..cbe87f8684 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
check_mode_recursive
chmod_recursive
check_pg_config
+ dir_symlink
system_or_bail
system_log
run_log
@@ -84,10 +85,12 @@ our @EXPORT = qw(
command_checks_all
$windows_os
+ $is_msys2
$use_unix_sockets
);
-our ($windows_os, $use_unix_sockets, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $is_msys2, $use_unix_sockets, $tmp_check, $log_path,
+ $test_logfile);
BEGIN
{
@@ -114,6 +117,9 @@ BEGIN
# Must be set early
$windows_os = $Config{osname} eq 'MSWin32' || $Config{osname} eq 'msys';
+ # Check if this environment is MSYS2.
+ $is_msys2 = $^O eq 'msys' && `uname -or` =~ /^[2-9].*Msys/;
+
if ($windows_os)
{
require Win32API::File;
@@ -137,6 +143,10 @@ BEGIN
Set to true when running under Windows, except on Cygwin.
+=item C<$is_msys2>
+
+Set to true when running under MSYS2.
+
=back
=cut
@@ -152,7 +162,7 @@ INIT
# TESTDIR environment variable, which is normally set by the invoking
# Makefile.
$tmp_check = $ENV{TESTDIR} ? "$ENV{TESTDIR}/tmp_check" : "tmp_check";
- $log_path = "$tmp_check/log";
+ $log_path = "$tmp_check/log";
mkdir $tmp_check;
mkdir $log_path;
@@ -263,9 +273,10 @@ sub tempdir_short
=item perl2host()
-Translate a Perl file name to a host file name. Currently, this is a no-op
+Translate a virtual file name to a host file name. Currently, this is a no-op
except for the case of Perl=msys and host=mingw32. The subject need not
-exist, but its parent directory must exist.
+exist, but its parent or grandparent directory must exist unless cygpath is
+available.
=cut
@@ -273,6 +284,17 @@ sub perl2host
{
my ($subject) = @_;
return $subject unless $Config{osname} eq 'msys';
+ if ($is_msys2)
+ {
+ # get absolute, windows type path
+ my $path = qx{cygpath -a -w "$subject"};
+ if (!$?)
+ {
+ chomp $path;
+ return $path if $path;
+ }
+ # fall through if this didn't work.
+ }
my $here = cwd;
my $leaf;
if (chdir $subject)
@@ -283,7 +305,12 @@ sub perl2host
{
$leaf = '/' . basename $subject;
my $parent = dirname $subject;
- chdir $parent or die "could not chdir \"$parent\": $!";
+ if (!chdir $parent)
+ {
+ $leaf = '/' . basename($parent) . $leaf;
+ $parent = dirname $parent;
+ chdir $parent or die "could not chdir \"$parent\": $!";
+ }
}
# this odd way of calling 'pwd -W' is the only way that seems to work.
@@ -602,6 +629,40 @@ sub check_pg_config
=pod
+=item dir_symlink(oldname, newname)
+
+Portably create a symlink for a directory. On Windows this creates a junction
+point. Elsewhere it just calls perl's builtin symlink.
+
+=cut
+
+sub dir_symlink
+{
+ my $oldname = shift;
+ my $newname = shift;
+ if ($windows_os)
+ {
+ $oldname = perl2host($oldname);
+ $newname = perl2host($newname);
+ $oldname =~ s,/,\\,g;
+ $newname =~ s,/,\\,g;
+ my $cmd = qq{mklink /j "$newname" "$oldname"};
+ if ($Config{osname} eq 'msys')
+ {
+ # need some indirection on msys
+ $cmd = qq{echo '$cmd' | \$COMSPEC /Q};
+ }
+ system($cmd);
+ }
+ else
+ {
+ symlink $oldname, $newname;
+ }
+ die "No $newname" unless -e $newname;
+}
+
+=pod
+
=back
=head1 Test::More-LIKE METHODS
@@ -664,7 +725,7 @@ sub command_exit_is
# long as the process was not terminated by an exception. To work around
# that, use $h->full_results on Windows instead.
my $result =
- ($Config{osname} eq "MSWin32")
+ ($Config{osname} eq "MSWin32")
? ($h->full_results)[0]
: $h->result(0);
is($result, $expected, $test_name);
