This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] scripts/test-installation.pl: Handle NSS crypto libraries [BZ #21940]
- From: Rical Jasan <ricaljasan at pacific dot net>
- To: Florian Weimer <fweimer at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 17 Oct 2017 22:06:16 -0700
- Subject: Re: [PATCH] scripts/test-installation.pl: Handle NSS crypto libraries [BZ #21940]
- Authentication-results: sourceware.org; auth=none
- References: <20170810123022.6DB7B439942E1@oldenburg.str.redhat.com> <1c7c68bc-9418-6507-d6cb-0d2648e25fd1@pacific.net> <81d2608e-6313-caeb-9667-28103387d852@redhat.com>
On 10/10/2017 02:26 AM, Florian Weimer wrote:
> On 10/07/2017 12:50 PM, Rical Jasan wrote:
>
>> I would use:
>>
>> /^\s*lib(\w+)\.so(?:\.([0-9\.]+))?\s*=>.*\.so(?:\.([0-9\.]+))?/
>> ^ ^ ^
>> to avoid matching "lib.so.".
>
> I made the change in the attached patch.
>
>>> $found{$name} = 1;
>>> - if ($versions{$name} ne $version1 || $version1 ne $version2) {
>>> + if (defined($version1) != defined($version2)
>>> + || defined($version1) != defined($versions{$name})
>>> + || (defined($versions{$name})
>>> + && ($versions{$name} ne $version1 || $version1 ne $version2))) {
>>>
>>> print "Library lib$name is not correctly installed.\n";
>>> print "Please check your installation!\n";
>>> print "Offending line of ldd output: $_\n";
>>
>> It might help readability to follow up the match with something like:
>>
>> next if ! (defined($name) && defined($version1) && defined($version2));
>> next if ! (exists $versions{$name}) && defined($versions{$name}));
>
> Is this really clearer? What I wanted to express is “error if the
> defined-ness state is different across the three value, and if
> $versions{$name} is defined, it must much both versions”. The original
> condition captures this fairly succinctly.
Converting to a natural language expression was exactly the trouble I
had. I've attached an alternative proposal. I'm not sure if the
"Unexpected dependency" message is correct, but the
existence/defined-ness check on $versions{$name} seems the important
bit, which makes the subsequent transitive version tests safe.
My system has unversioned NSS libraries, but I don't have a sandbox
handy to run a `make install' (which is when it looks like this script
is run), so it's untested.
>> (I seem to recall tests on hash entries creating them when they didn't
>> previously exist, but that may not matter here. Something other than
>> `next' may also be desirable, and the opportunity for more fine-grained
>> error messages is introduced, if useful.)
>
> I suspect the original script was written for Perl 4, which did not have
> the exists operator. I instinctively stuck to that baseline.
This script definitely wouldn't pass a compilation check using the
strict pragma; I noticed you slipped in a `my', but the whole thing
suffers from the same scoping issue. I see install.texi says Perl 5 is
recommended, and I noticed perl(1) says:
Using the "use strict" pragma ensures that all variables are properly
declared and prevents other misuses of legacy Perl features.
I can update the script (and others) to specifically address such
"miuses" in a separate patchset, if it's considered worthwhile.
Rical
> Anyway, I came up with something simpler, which sidesteps these issues.
>
>> Then you could retain the simpler conditional:
>>
>> if ($versions{$name} ne $version1 || $version1 ne $version2) {
>>
>> That also continues using string comparisons, which would be better in
>> case there are multiple "."'s.
>
> I only used != as the Boolean XOR operator on the numeric result from
> the defined operator.
>
> Thanks,
> Florian
>
> bug21940.patch
>
>
> The warning looked like this:
>
> Use of uninitialized value in string ne at
> …/scripts/test-installation.pl line 184, <LDD> line 24.
>
> It is triggered by this line of ldd output:
>
> libfreebl3.so => /lib64/libfreebl3.so (0x00007f055003c000)
>
> The other lines have a version in the soname:
>
> libanl.so.1 => /lib64/libanl.so.1 (0x00007f055023f000)
>
> 2017-10-10 Florian Weimer <fweimer@redhat.com>
>
> [BZ #21940]
> * scripts/test-installation.pl: Handle NSS crypto libaries in ldd
> output.
>
> diff --git a/scripts/test-installation.pl b/scripts/test-installation.pl
> index 74d25e1c8d..7eaeb1ae06 100755
> --- a/scripts/test-installation.pl
> +++ b/scripts/test-installation.pl
> @@ -176,10 +176,17 @@ open LDD, "ldd /tmp/test-prg$$ |"
> or die ("Couldn't execute ldd");
> while (<LDD>) {
> if (/^\s*lib/) {
> - ($name, $version1, $version2) =
> - /^\s*lib(\w*)\.so\.([0-9\.]*)\s*=>.*\.so\.([0-9\.]*)/;
> + # When libcrypt is linked against NSS, some of the referenced
> + # libraries do not have a trailing version in their soname.
> + my ($name, $version1, $version2) =
> + /^\s*lib(\w+)\.so(?:\.([0-9\.]+))?\s*=>.*\.so(?:\.([0-9\.]+))?/;
> $found{$name} = 1;
> - if ($versions{$name} ne $version1 || $version1 ne $version2) {
> + # Version strings are either undefined or non-empty.
> + $version1 = '' unless defined $version1;
> + $version2 = '' unless defined $version2;
> + my $vername = $versions{$name};
> + $vername = '' unless defined $vername;
> + if ($version1 ne $version2 || $vername ne $version1) {
> print "Library lib$name is not correctly installed.\n";
> print "Please check your installation!\n";
> print "Offending line of ldd output: $_\n";
diff --git a/scripts/test-installation.pl b/scripts/test-installation.pl
index 45c666b0a2..fcb7707889 100755
--- a/scripts/test-installation.pl
+++ b/scripts/test-installation.pl
@@ -171,14 +171,20 @@ if ($?) {
$ok = 1;
%found = ();
+$librgx = qr/lib(\w+)\.so(?:\.([0-9\.]+))?/;
open LDD, "ldd /tmp/test-prg$$ |"
or die ("Couldn't execute ldd");
while (<LDD>) {
- if (/^\s*lib/) {
+ if (m,^\s*${librgx}\s*=>\s*(?:/[^/]+)*/${librgx},) {
($name, $version1, $version2) =
- /^\s*lib(\w*)\.so\.([0-9\.]*)\s*=>.*\.so\.([0-9\.]*)/;
+ ($1, defined($2) ? $2 : '', defined($4) ? $4 : '');
$found{$name} = 1;
+ if (!exists $versions{$name}) {
+ print "Unexpected dependency: lib$name\n";
+ $ok = 0;
+ next;
+ }
if ($versions{$name} ne $version1 || $version1 ne $version2) {
print "Library lib$name is not correctly installed.\n";
print "Please check your installation!\n";