[CF2016-9] Allow spaces in working path on tap-tests

Started by Kyotaro HORIGUCHIover 9 years ago11 messages
#1Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
1 attachment(s)

Hello, this is just an entry mail for the next CF.

The tap-test fails when the soruce directoy containing spaces. I
accidentially hit this by a Jenkins project with the name "test
project".

The function system_log() is safe for such parameters but
backup() uses it in wrong way. On the other hand,
enable_restoring() and enable_archiving() forgets to quote such a
path containing spaces as already done for Windows. I don't see
the similar problem in other places.

regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Allow-spaces-in-working-directory-path-on-TAP-tests.patchtext/x-patch; charset=us-asciiDownload
From 4c38a3e95da1abb5360a80b9ad646c9c6309bd9b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Mon, 4 Jul 2016 15:18:51 +0900
Subject: [PATCH] Allow spaces in working directory path on TAP-tests.

Several tap tests fails when the soruce directory path is containing
spaces. This patch fixes it.
---
 src/test/perl/PostgresNode.pm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 636dfec..5767919 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -475,7 +475,8 @@ sub backup
 	my $name        = $self->name;
 
 	print "# Taking pg_basebackup $backup_name from node \"$name\"\n";
-	TestLib::system_or_bail("pg_basebackup -D $backup_path -p $port -x");
+	TestLib::system_or_bail('pg_basebackup', '-D', $backup_path,
+							'-p', $port, '-x');
 	print "# Backup finished\n";
 }
 
@@ -763,7 +764,7 @@ sub enable_restoring
 	my $copy_command =
 	  $TestLib::windows_os
 	  ? qq{copy "$path\\\\%f" "%p"}
-	  : qq{cp $path/%f %p};
+	  : qq{cp "$path/%f" "%p"};
 
 	$self->append_conf(
 		'recovery.conf', qq(
@@ -791,7 +792,7 @@ sub enable_archiving
 	my $copy_command =
 	  $TestLib::windows_os
 	  ? qq{copy "%p" "$path\\\\%f"}
-	  : qq{cp %p $path/%f};
+	  : qq{cp "%p" "$path/%f"};
 
 	# Enable archive_mode and archive_command on node
 	$self->append_conf(
-- 
1.8.3.1

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Kyotaro HORIGUCHI (#1)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Mon, Jul 4, 2016 at 4:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Hello, this is just an entry mail for the next CF.

The tap-test fails when the soruce directoy containing spaces. I
accidentially hit this by a Jenkins project with the name "test
project".

The function system_log() is safe for such parameters but
backup() uses it in wrong way. On the other hand,
enable_restoring() and enable_archiving() forgets to quote such a
path containing spaces as already done for Windows. I don't see
the similar problem in other places.

Good catch, your fix looks good to me. I am noticing as well that the
invocations of pg_ctl, pg_dumpall, pg_upgrade and diff are likely
broken the same way in vcregress.pl.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#2)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Mon, Jul 4, 2016 at 4:29 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Jul 4, 2016 at 4:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Hello, this is just an entry mail for the next CF.

The tap-test fails when the soruce directoy containing spaces. I
accidentially hit this by a Jenkins project with the name "test
project".

The function system_log() is safe for such parameters but
backup() uses it in wrong way. On the other hand,
enable_restoring() and enable_archiving() forgets to quote such a
path containing spaces as already done for Windows. I don't see
the similar problem in other places.

Good catch, your fix looks good to me. I am noticing as well that the
invocations of pg_ctl, pg_dumpall, pg_upgrade and diff are likely
broken the same way in vcregress.pl.

And as is the command built for zic.exe in Install.pm, no? $target is
normally an absolute path per the call of Install().
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#3)
1 attachment(s)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Mon, Jul 4, 2016 at 4:44 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

And as is the command built for zic.exe in Install.pm, no? $target is
normally an absolute path per the call of Install().

Attached is the patch I have in mind. After more investigation zic.exe
is indeed broken, $target can be a full path, and if it contains a
space things blow up. The commands of vcregress upgradecheck need a
cleanup as well. I have merged both patches together and the part for
src/tools/msvc needs a backpatch. Separating both things is trivial
anyway as the MSVC and the TAP stuff are on two completely separate
paths.
--
Michael

Attachments:

fix-perl-system.patchtext/x-diff; charset=US-ASCII; name=fix-perl-system.patchDownload
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 636dfec..5767919 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -475,7 +475,8 @@ sub backup
 	my $name        = $self->name;
 
 	print "# Taking pg_basebackup $backup_name from node \"$name\"\n";
-	TestLib::system_or_bail("pg_basebackup -D $backup_path -p $port -x");
+	TestLib::system_or_bail('pg_basebackup', '-D', $backup_path,
+							'-p', $port, '-x');
 	print "# Backup finished\n";
 }
 
@@ -763,7 +764,7 @@ sub enable_restoring
 	my $copy_command =
 	  $TestLib::windows_os
 	  ? qq{copy "$path\\\\%f" "%p"}
-	  : qq{cp $path/%f %p};
+	  : qq{cp "$path/%f" "%p"};
 
 	$self->append_conf(
 		'recovery.conf', qq(
@@ -791,7 +792,7 @@ sub enable_archiving
 	my $copy_command =
 	  $TestLib::windows_os
 	  ? qq{copy "%p" "$path\\\\%f"}
-	  : qq{cp %p $path/%f};
+	  : qq{cp "%p" "$path/%f"};
 
 	# Enable archive_mode and archive_command on node
 	$self->append_conf(
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 031719d..29a4c3e 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -383,10 +383,19 @@ sub GenerateTimezoneFiles
 	$mf =~ /^TZDATA\s*:?=\s*(.*)$/m
 	  || die "Could not find TZDATA row in timezone makefile\n";
 	my @tzfiles = split /\s+/, $1;
-	unshift @tzfiles, '';
+
 	print "Generating timezone files...";
-	system("$conf\\zic\\zic -d \"$target/share/timezone\" "
-		  . join(" src/timezone/data/", @tzfiles));
+
+	my @args = ("$conf/zic/zic",
+				'-d',
+				"$target/share/timezone");
+	foreach (@tzfiles)
+	{
+		my $tzfile = $_;
+		push(@args, "src/timezone/data/$tzfile")
+	}
+
+	system(@args);
 	print "\n";
 }
 
@@ -634,9 +643,10 @@ sub CopyIncludeFiles
 		next unless (-d "src/include/$d");
 
 		EnsureDirectories("$target/include/server/$d");
-		system(
-qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"}
-		) && croak("Failed to copy include directory $d\n");
+		my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+				 "src\\include\\$d\\*.h",
+				 "$ctarget\\include\\server\\$d\\");
+		system(@args) && croak("Failed to copy include directory $d\n");
 	}
 	closedir($D);
 
@@ -689,9 +699,11 @@ sub GenerateNLSFiles
 
 			EnsureDirectories($target, "share/locale/$lang",
 				"share/locale/$lang/LC_MESSAGES");
-			system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-			) && croak("Could not run msgfmt on $dir\\$_");
+			my @args = ("$nlspath\\bin\\msgfmt",
+			   '-o',
+			   "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+			   $_);
+			system(@args) && croak("Could not run msgfmt on $dir\\$_");
 			print ".";
 		}
 	}
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 075279a..0a95bcb 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -412,35 +412,42 @@ sub upgradecheck
 	print "\nRunning initdb on old cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nStarting old cluster\n\n";
-	system("pg_ctl start -l $logdir/postmaster1.log -w") == 0 or exit 1;
+	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log", '-w');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up data for upgrading\n\n";
 	installcheck();
 
 	# now we can chdir into the source dir
 	chdir "$topdir/src/bin/pg_upgrade";
 	print "\nDumping old cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump1.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping old cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	$ENV{PGDATA} = "$data";
 	print "\nSetting up new cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nRunning pg_upgrade\n\n";
-	system("pg_upgrade -d $data.old -D $data -b $bindir -B $bindir") == 0
+	@args = ('pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
+			 '-B', $bindir);
+	system(@args) == 0
 	  or exit 1;
 	print "\nStarting new cluster\n\n";
-	system("pg_ctl -l $logdir/postmaster2.log -w start") == 0 or exit 1;
+	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", '-w', 'start');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up stats on new cluster\n\n";
 	system(".\\analyze_new_cluster.bat") == 0 or exit 1;
 	print "\nDumping new cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump2.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping new cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	print "\nDeleting old cluster\n\n";
 	system(".\\delete_old_cluster.bat") == 0 or exit 1;
 	print "\nComparing old and new cluster dumps\n\n";
 
-	system("diff -q $tmp_root/dump1.sql $tmp_root/dump2.sql");
+	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
+	system(@args);
 	$status = $?;
 	if (!$status)
 	{
#5Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
In reply to: Michael Paquier (#4)
Re: [CF2016-9] Allow spaces in working path on tap-tests

Hello,

At Tue, 5 Jul 2016 13:44:08 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSt2W30tE12eRq7KGB_FPcBpXDX2Zh8XeH2QHFY9Vfb8Q@mail.gmail.com>

On Mon, Jul 4, 2016 at 4:44 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

And as is the command built for zic.exe in Install.pm, no? $target is
normally an absolute path per the call of Install().

Attached is the patch I have in mind. After more investigation zic.exe
is indeed broken, $target can be a full path, and if it contains a
space things blow up. The commands of vcregress upgradecheck need a
cleanup as well. I have merged both patches together and the part for
src/tools/msvc needs a backpatch. Separating both things is trivial
anyway as the MSVC and the TAP stuff are on two completely separate
paths.

Agreed. Grep'ing "system" in the source tree, I see no more place
where needs the same fix.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Michael Paquier
michael.paquier@gmail.com
In reply to: Kyotaro HORIGUCHI (#5)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Tue, Jul 5, 2016 at 6:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:

Agreed. Grep'ing "system" in the source tree, I see no more place
where needs the same fix.

Same conclusion here. I have added this stuff to the official patch tracker:
https://commitfest.postgresql.org/10/663/
I can as well produce patches for back-branches, feel free to ping me
if necessary.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kyotaro HORIGUCHI (#5)
Re: [CF2016-9] Allow spaces in working path on tap-tests

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

At Tue, 5 Jul 2016 13:44:08 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSt2W30tE12eRq7KGB_FPcBpXDX2Zh8XeH2QHFY9Vfb8Q@mail.gmail.com>

Attached is the patch I have in mind. After more investigation zic.exe
is indeed broken, $target can be a full path, and if it contains a
space things blow up. The commands of vcregress upgradecheck need a
cleanup as well. I have merged both patches together and the part for
src/tools/msvc needs a backpatch. Separating both things is trivial
anyway as the MSVC and the TAP stuff are on two completely separate
paths.

Agreed. Grep'ing "system" in the source tree, I see no more place
where needs the same fix.

This seemed like a bug fix to me, so I went ahead and pushed it. I don't
have any ability to test the Windows parts, so it's possible I missed
something in the back-patching; please review.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#7)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Sun, Jul 10, 2016 at 5:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seemed like a bug fix to me ...

Yes, it is.

... so I went ahead and pushed it. I don't
have any ability to test the Windows parts, so it's possible I missed
something in the back-patching; please review.

Thanks! What you have pushed looks fine to me. Also, the portion for
src/tools/msvc needs to go further down, I should have precised that
earlier. Do you want a patch for that?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#8)
Re: [CF2016-9] Allow spaces in working path on tap-tests

Michael Paquier <michael.paquier@gmail.com> writes:

Thanks! What you have pushed looks fine to me. Also, the portion for
src/tools/msvc needs to go further down, I should have precised that
earlier. Do you want a patch for that?

Yes, please --- I thought it'd all gotten done.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#9)
3 attachment(s)
Re: [CF2016-9] Allow spaces in working path on tap-tests

On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Thanks! What you have pushed looks fine to me. Also, the portion for
src/tools/msvc needs to go further down, I should have precised that
earlier. Do you want a patch for that?

Yes, please --- I thought it'd all gotten done.

OK, here are patches for 9.1, 9.2 and 9.3.
--
Michael

Attachments:

fix-perl-system-91.patchtext/x-patch; charset=US-ASCII; name=fix-perl-system-91.patchDownload
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index b779b31..bbb5c39 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -268,12 +268,19 @@ sub GenerateTimezoneFiles
     my $conf = shift;
     my $mf = read_file("src/timezone/Makefile");
     $mf =~ s{\\\s*[\r\n]+}{}mg;
-    $mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA row in timezone makefile\n";
+    $mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA line in timezone makefile\n";
     my @tzfiles = split /\s+/,$1;
-    unshift @tzfiles,'';
     print "Generating timezone files...";
-    system(
-        "$conf\\zic\\zic -d \"$target/share/timezone\" " . join(" src/timezone/data/", @tzfiles));
+    my @args = ("$conf/zic/zic",
+                '-d',
+                "$target/share/timezone");
+    foreach (@tzfiles)
+    {
+        my $tzfile = $_;
+        push(@args, "src/timezone/data/$tzfile")
+    }
+
+    system(@args);
     print "\n";
 }
 
@@ -490,8 +497,10 @@ sub CopyIncludeFiles
         next unless (-d "src/include/$d");
 
         EnsureDirectories("$target/include/server/$d");
-        system(qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"})
-          && croak("Failed to copy include directory $d\n");
+        my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+                    "src\\include\\$d\\*.h",
+                    "$ctarget\\include\\server\\$d\\");
+        system(@args) && croak("Failed to copy include directory $d\n");
     }
     closedir($D);
 
@@ -545,9 +554,11 @@ sub GenerateNLSFiles
             $lang = $1;
 
             EnsureDirectories($target, "share/locale/$lang", "share/locale/$lang/LC_MESSAGES");
-            system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-            )&& croak("Could not run msgfmt on $dir\\$_");
+            my @args = ("$nlspath\\bin\\msgfmt",
+                        '-o',
+                        "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+                        $_);
+            system(@args) && croak("Could not run msgfmt on $dir\\$_");
             print ".";
         }
     }
fix-perl-system-92.patchtext/x-patch; charset=US-ASCII; name=fix-perl-system-92.patchDownload
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 7205e23..671d860 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -309,12 +309,21 @@ sub GenerateTimezoneFiles
 	my $mf     = read_file("src/timezone/Makefile");
 	$mf =~ s{\\\s*[\r\n]+}{}mg;
 	$mf =~ /^TZDATA\s*:?=\s*(.*)$/m
-	  || die "Could not find TZDATA row in timezone makefile\n";
+	  || die "Could not find TZDATA line in timezone makefile\n";
 	my @tzfiles = split /\s+/, $1;
-	unshift @tzfiles, '';
+
 	print "Generating timezone files...";
-	system("$conf\\zic\\zic -d \"$target/share/timezone\" "
-		  . join(" src/timezone/data/", @tzfiles));
+
+	my @args = ("$conf/zic/zic",
+				'-d',
+				"$target/share/timezone");
+	foreach (@tzfiles)
+	{
+		my $tzfile = $_;
+		push(@args, "src/timezone/data/$tzfile")
+	}
+
+	system(@args);
 	print "\n";
 }
 
@@ -542,9 +551,10 @@ sub CopyIncludeFiles
 		next unless (-d "src/include/$d");
 
 		EnsureDirectories("$target/include/server/$d");
-		system(
-qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"}
-		) && croak("Failed to copy include directory $d\n");
+		my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+				 "src\\include\\$d\\*.h",
+				 "$ctarget\\include\\server\\$d\\");
+		system(@args) && croak("Failed to copy include directory $d\n");
 	}
 	closedir($D);
 
@@ -597,9 +607,11 @@ sub GenerateNLSFiles
 
 			EnsureDirectories($target, "share/locale/$lang",
 				"share/locale/$lang/LC_MESSAGES");
-			system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-			) && croak("Could not run msgfmt on $dir\\$_");
+			my @args = ("$nlspath\\bin\\msgfmt",
+			   '-o',
+			   "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+			   $_);
+			system(@args) && croak("Could not run msgfmt on $dir\\$_");
 			print ".";
 		}
 	}
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index adbfa9f..390de4e 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -270,7 +270,7 @@ sub upgradecheck
 	Install($tmp_install, $config);
 	# Install does a chdir, so change back after that
 	chdir $cwd;
-	my ($bindir,$libdir,$oldsrc,$newsrc) = 
+	my ($bindir,$libdir,$oldsrc,$newsrc) =
 	  ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir);
 	$ENV{PATH} = "$bindir;$ENV{PATH}";
 	my $data = "$tmp_root/data";
@@ -280,34 +280,41 @@ sub upgradecheck
 	print "\nRunning initdb on old cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nStarting old cluster\n\n";
-	system("pg_ctl start -l $logdir/postmaster1.log -w") == 0 or exit 1;
+	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log", '-w');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up data for upgrading\n\n";
 	installcheck();
 	# now we can chdir into the source dir
 	chdir "$topdir/contrib/pg_upgrade";
 	print "\nDumping old cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump1.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping old cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	$ENV{PGDATA} = "$data";
 	print "\nSetting up new cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nRunning pg_upgrade\n\n";
-	system("pg_upgrade -d $data.old -D $data -b $bindir -B $bindir") == 0 
+	@args = ('pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
+		 '-B', $bindir);
+	system(@args) == 0 or exit 1;
 	  or exit 1;
 	print "\nStarting new cluster\n\n";
-	system("pg_ctl -l $logdir/postmaster2.log -w start") == 0 or exit 1;
+	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", '-w', 'start');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up stats on new cluster\n\n";
 	system(".\\analyze_new_cluster.bat") == 0 or exit 1;
 	print "\nDumping new cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump2.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping new cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	print "\nDeleting old cluster\n\n";
 	system(".\\delete_old_cluster.bat") == 0 or exit 1;
 	print "\nComparing old and new cluster dumps\n\n";
 
-	system("diff -q $tmp_root/dump1.sql $tmp_root/dump2.sql");
+	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
+	system(@args);
 	$status = $?;
 	if (!$status)
 	{
fix-perl-system-93.patchtext/x-patch; charset=US-ASCII; name=fix-perl-system-93.patchDownload
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 5bd2637..4674ab3 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -316,12 +316,21 @@ sub GenerateTimezoneFiles
 	my $mf     = read_file("src/timezone/Makefile");
 	$mf =~ s{\\\s*[\r\n]+}{}mg;
 	$mf =~ /^TZDATA\s*:?=\s*(.*)$/m
-	  || die "Could not find TZDATA row in timezone makefile\n";
+	  || die "Could not find TZDATA line in timezone makefile\n";
 	my @tzfiles = split /\s+/, $1;
-	unshift @tzfiles, '';
+
 	print "Generating timezone files...";
-	system("$conf\\zic\\zic -d \"$target/share/timezone\" "
-		  . join(" src/timezone/data/", @tzfiles));
+
+	my @args = ("$conf/zic/zic",
+				'-d',
+				"$target/share/timezone");
+	foreach (@tzfiles)
+	{
+		my $tzfile = $_;
+		push(@args, "src/timezone/data/$tzfile")
+	}
+
+	system(@args);
 	print "\n";
 }
 
@@ -550,9 +559,10 @@ sub CopyIncludeFiles
 		next unless (-d "src/include/$d");
 
 		EnsureDirectories("$target/include/server/$d");
-		system(
-qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"}
-		) && croak("Failed to copy include directory $d\n");
+		my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+				 "src\\include\\$d\\*.h",
+				 "$ctarget\\include\\server\\$d\\");
+		system(@args) && croak("Failed to copy include directory $d\n");
 	}
 	closedir($D);
 
@@ -605,9 +615,11 @@ sub GenerateNLSFiles
 
 			EnsureDirectories($target, "share/locale/$lang",
 				"share/locale/$lang/LC_MESSAGES");
-			system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-			) && croak("Could not run msgfmt on $dir\\$_");
+			my @args = ("$nlspath\\bin\\msgfmt",
+			   '-o',
+			   "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+			   $_);
+			system(@args) && croak("Could not run msgfmt on $dir\\$_");
 			print ".";
 		}
 	}
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 244c2b1..033969d 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -282,35 +282,42 @@ sub upgradecheck
 	print "\nRunning initdb on old cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nStarting old cluster\n\n";
-	system("pg_ctl start -l $logdir/postmaster1.log -w") == 0 or exit 1;
+	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log", '-w');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up data for upgrading\n\n";
 	installcheck();
 
 	# now we can chdir into the source dir
 	chdir "$topdir/contrib/pg_upgrade";
 	print "\nDumping old cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump1.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping old cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	$ENV{PGDATA} = "$data";
 	print "\nSetting up new cluster\n\n";
 	standard_initdb() or exit 1;
 	print "\nRunning pg_upgrade\n\n";
-	system("pg_upgrade -d $data.old -D $data -b $bindir -B $bindir") == 0
+	@args = ('pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
+		 '-B', $bindir);
+	system(@args) == 0 or exit 1;
 	  or exit 1;
 	print "\nStarting new cluster\n\n";
-	system("pg_ctl -l $logdir/postmaster2.log -w start") == 0 or exit 1;
+	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", '-w', 'start');
+	system(@args) == 0 or exit 1;
 	print "\nSetting up stats on new cluster\n\n";
 	system(".\\analyze_new_cluster.bat") == 0 or exit 1;
 	print "\nDumping new cluster\n\n";
-	system("pg_dumpall -f $tmp_root/dump2.sql") == 0 or exit 1;
+	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
+	system(@args) == 0 or exit 1;
 	print "\nStopping new cluster\n\n";
 	system("pg_ctl -m fast stop") == 0 or exit 1;
 	print "\nDeleting old cluster\n\n";
 	system(".\\delete_old_cluster.bat") == 0 or exit 1;
 	print "\nComparing old and new cluster dumps\n\n";
 
-	system("diff -q $tmp_root/dump1.sql $tmp_root/dump2.sql");
+	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
+	system(@args);
 	$status = $?;
 	if (!$status)
 	{
#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#10)
Re: [CF2016-9] Allow spaces in working path on tap-tests

Michael Paquier <michael.paquier@gmail.com> writes:

On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yes, please --- I thought it'd all gotten done.

OK, here are patches for 9.1, 9.2 and 9.3.

Pushed, thanks.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers