Re: [HACKERS] Perl coding error in msvc build system?
On Tue, Feb 17, 2015 at 6:34 AM, Peter Eisentraut pete...@gmx.net wrote: This patch been reviewed by 4 people, resulting in 2 minor suggestions for changes (adding an m modifier, and removing a bogus last). It has 2 clear upvotes and 0 downvotes. I think it should be revised along the lines suggested and committed without further ado. My patch actually only covered the first of the two faulty locations I pointed out. Attached is a patch that also fixes the second one. I noticed that DetermineVisualStudioVersion() is never called with an argument, so I removed that branch altogether. +1 for those simplifications. And FWIW, it passed my series of tests with MSVC 2010. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
This patch been reviewed by 4 people, resulting in 2 minor suggestions
for changes (adding an m modifier, and removing a bogus last).
It has 2 clear upvotes and 0 downvotes.
I think it should be revised along the lines suggested and committed
without further ado.
My patch actually only covered the first of the two faulty locations I
pointed out. Attached is a patch that also fixes the second one. I
noticed that DetermineVisualStudioVersion() is never called with an
argument, so I removed that branch altogether.
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 39e41f6..714585f 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -71,17 +71,9 @@ sub DeterminePlatform
my $self = shift;
# Examine CL help output to determine if we are in 32 or 64-bit mode.
- $self-{platform} = 'Win32';
- open(P, cl /? 21 |) || die cl command not found;
- while (P)
- {
- if (/^\/favor:.+AMD64/)
- {
- $self-{platform} = 'x64';
- last;
- }
- }
- close(P);
+ my $output = `cl /? 21`;
+ $? 8 == 0 or die cl command not found;
+ $self-{platform} = ($output =~ /^\/favor:.+AMD64/m) ? 'x64' : 'Win32';
print Detected hardware platform: $self-{platform}\n;
}
diff --git a/src/tools/msvc/VSObjectFactory.pm b/src/tools/msvc/VSObjectFactory.pm
index d255bec..b83af40 100644
--- a/src/tools/msvc/VSObjectFactory.pm
+++ b/src/tools/msvc/VSObjectFactory.pm
@@ -92,30 +92,16 @@ sub CreateProject
sub DetermineVisualStudioVersion
{
- my $nmakeVersion = shift;
-
- if (!defined($nmakeVersion))
- {
-
-# Determine version of nmake command, to set proper version of visual studio
-# we use nmake as it has existed for a long time and still exists in current visual studio versions
- open(P, nmake /? 21 |)
- || croak
-Unable to determine Visual Studio version: The nmake command wasn't found.;
- while (P)
- {
- chomp;
- if (/(\d+)\.(\d+)\.\d+(\.\d+)?$/)
- {
-return _GetVisualStudioVersion($1, $2);
- }
- }
- close(P);
- }
- elsif ($nmakeVersion =~ /(\d+)\.(\d+)\.\d+(\.\d+)?$/)
+ # To determine version of Visual Studio we use nmake as it has
+ # existed for a long time and still exists in current Visual
+ # Studio versions.
+ my $output = `nmake /? 21`;
+ $? 8 == 0 or croak Unable to determine Visual Studio version: The nmake command wasn't found.;
+ if ($output =~ /(\d+)\.(\d+)\.\d+(\.\d+)?$/m)
{
return _GetVisualStudioVersion($1, $2);
}
+
croak
Unable to determine Visual Studio version: The nmake version could not be determined.;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
On Fri, Jan 23, 2015 at 4:17 PM, Brar Piening b...@gmx.de wrote: Am 23.01.2015 um 09:17 schrieb Abhijit Menon-Sen: At 2014-06-03 22:30:50 -0400, pete...@gmx.net wrote: I'm not sure whether the following coding actually detects any errors: Solution.pm: open(P, cl /? 21 |) || die cl command not found; Since nobody with a Windows system has commented, I'm just writing to say that from a Perl perspective, I agree with your analysis and the patch looks perfectly sensible. I can confirm it on my Windows system. Calling build from a console without nmake in the path I always get: Unable to determine Visual Studio version: The nmake version could not be determined. at src/tools/msvc/Mkvcbuild.pm line 63. This means that the following construct in VSObjectFactory.pm doesn't have the desired effect. open(P, nmake /? 21 |) || croak Unable to determine Visual Studio version: The nmake command wasn't found.; On the other hand complicacy is in the eye of the beholder. Perl constructs like the following get quite a few wtf's (http://www.osnews.com/story/19266/WTFs_m) from a simple-minded person like me. $? 8 == 0 or die cl command not found; However as it fixes a confirmed problem and as maintainance of perl code is an issue of its own, please go ahead. This patch been reviewed by 4 people, resulting in 2 minor suggestions for changes (adding an m modifier, and removing a bogus last). It has 2 clear upvotes and 0 downvotes. I think it should be revised along the lines suggested and committed without further ado. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
Why is the last; still there? AFAICS it matters in the old coding because of the while, but I don't think there is an equivalent looping construct in the new code. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
On 01/23/2015 03:00 PM, Alvaro Herrera wrote: Why is the last; still there? AFAICS it matters in the old coding because of the while, but I don't think there is an equivalent looping construct in the new code. You're right, I missed that. It shouldn't be there, and will produce an error like Can't last outside a loop block cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
Am 23.01.2015 um 09:17 schrieb Abhijit Menon-Sen: At 2014-06-03 22:30:50 -0400, pete...@gmx.net wrote: I'm not sure whether the following coding actually detects any errors: Solution.pm: open(P, cl /? 21 |) || die cl command not found; Since nobody with a Windows system has commented, I'm just writing to say that from a Perl perspective, I agree with your analysis and the patch looks perfectly sensible. I can confirm it on my Windows system. Calling build from a console without nmake in the path I always get: Unable to determine Visual Studio version: The nmake version could not be determined. at src/tools/msvc/Mkvcbuild.pm line 63. This means that the following construct in VSObjectFactory.pm doesn't have the desired effect. open(P, nmake /? 21 |) || croak Unable to determine Visual Studio version: The nmake command wasn't found.; On the other hand complicacy is in the eye of the beholder. Perl constructs like the following get quite a few wtf's (http://www.osnews.com/story/19266/WTFs_m) from a simple-minded person like me. $? 8 == 0 or die cl command not found; However as it fixes a confirmed problem and as maintainance of perl code is an issue of its own, please go ahead. Regards, Brar -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
At 2014-06-03 22:30:50 -0400, pete...@gmx.net wrote: I'm not sure whether the following coding actually detects any errors: Solution.pm: open(P, cl /? 21 |) || die cl command not found; Since nobody with a Windows system has commented, I'm just writing to say that from a Perl perspective, I agree with your analysis and the patch looks perfectly sensible. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl coding error in msvc build system?
On 01/23/2015 03:17 AM, Abhijit Menon-Sen wrote: At 2014-06-03 22:30:50 -0400, pete...@gmx.net wrote: I'm not sure whether the following coding actually detects any errors: Solution.pm: open(P, cl /? 21 |) || die cl command not found; Since nobody with a Windows system has commented, I'm just writing to say that from a Perl perspective, I agree with your analysis and the patch looks perfectly sensible. Not quite. This line: if ($output =~ /^\/favor:.+AMD64/) needs an m modifier on the regex, I think, so that the ^ matches any beginning of line, not just the first. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
