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