This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] scripts/test-installation.pl: Handle NSS crypto libraries [BZ #21940]


On 12/14/2017 07:10 AM, Florian Weimer wrote:
> On 10/18/2017 07:06 AM, Rical Jasan wrote:
>> 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;
>> +    }
> 
> This looks fine to me.  Could you commit it with a ChangeLog entry?

I have the resources available to set up burner systems now, so after
reacquainting myself with this patch, I set about testing it.  Not only
is test-installation.pl the very last thing run during `make install',
but it _only_ happens when prefix=/usr and DESTDIR is not set, so even
testing the patch requires bombing the system libc.

The above tidbit actually makes the situation worse because instead of
having some "uninitialized value" warnings printed to stderr by Perl
with an otherwise successful installation, that "Unexpected dependency"
check will cause `make install' to fail, at the very end, after
everything has already been installed.

What needs to happen is to make libfreebl3 an expected dependency, as
well as allowing for unversioned sonames.  I've attached a patch that
works for me, but it definitely stems from hacking just enough to make
it work.  There are some design choices to be made, so others need to
weigh in on whether they think what I did is appropriate.

The process starts with shlib-versions, which defines valid versions of
shared libraries, both installed by glibc as well as external
dependencies.  Note that in this file, versions are not prefixed by a
dot (important later).  The choice I made here was to simply not define
a version.

scripts/soversions.awk uses shlib-versions.v to create soversions.i in
the build root.  I'm not sure where shlib-versions.v comes from, but it
is a generated file as well.  I chose to tag the special case of no
version with an arbitrary version of "undef" because if it is left blank
at this point, the whitespace-delimited fields in soversions.i will be
parsed incorrectly when generating soversions.mk in Makeconfig.

Correspondingly, Makeconfig is given a special case for the "undef"
version, which generates soversions.mk lines absent of any versioning.
There is some interesting voodoo here with what appears to be a
superfluous dot preceding the version, but I tried stripping it to
condense things and wound up with a broken installation with programs
that couldn't find "libc.so6" at runtime, so I was convinced I should
keep my unversioned soname propagation as an independent, exceptional case.

Lastly, scripts/test-installation.pl populates its %versions hash by
matching lines in soversions.mk with versions prefixed with a dot, so I
made the dot optional.  This results in an empty string for the version
of unversioned libraries, but that is specific to the library name, and
should be OK.  This is where we meet the above diff, which will match an
unversioned library, so those empty strings will result in equality for
a given library, and since an entry was defined for the library in the
%versions hash, it isn't an unexpected dependency either.

There is some ordering in soversions.awk that I don't quite understand
the purpose of, but just appending libfreebl3 to shlib-versions seems to
have worked.

I'm concerned about the fact test-installation.pl is only run at the end
of `make install'; if it fails at that point, you're screwed anyway, so
it seems to be of limited utility.  I wonder if it's possible to adapt
it to run after a `make install' with prefix!=/usr or DESTDIR set.  That
can be a separate patch, though, and I haven't played with it.

I can resubmit this to the list as an RFC if you think that would help
visibility.  Testing requires a special setup, and I'm not sure how much
of the automated testing infrastructure out there actually exercises
this script, or would be affected by the attached patch's changes.  The
length of the path from shlib-versions to test-installation.pl suggests
to me there are other things that depend on the framework.

Rical
diff --git a/Makeconfig b/Makeconfig
index 80c498e33b..d9e9003c4b 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -1110,6 +1110,8 @@ $(common-objpfx)soversions.mk: $(common-objpfx)soversions.i $(..)Makeconfig
 	   case $$number in \
 	     [0-9]*) echo "$$lib.so-version=.$$number"; \
 		     echo "all-sonames+=$$lib=$$lib.so\$$($$lib.so-version)";;\
+	     undef)  echo "$$lib.so-version="; \
+		     echo "all-sonames+=$$lib=$$lib.so";;\
 	     *)	     echo "$$lib.so-version=$$number"; \
 		     echo "all-sonames+=$$lib=\$$($$lib.so-version)";;\
 	   esac; \
diff --git a/scripts/soversions.awk b/scripts/soversions.awk
index 247f061bc3..676c8692f6 100644
--- a/scripts/soversions.awk
+++ b/scripts/soversions.awk
@@ -14,6 +14,8 @@ $1 == "DEFAULT" {
   sub(/=.*$/, "", lib);
   sub(/^.*=/, "", number);
   if (lib in numbers) next;
+  if (number == "")
+    number = "undef";
   numbers[lib] = number;
   order[lib] = ++order_n;
   if (NF > 1) {
diff --git a/scripts/test-installation.pl b/scripts/test-installation.pl
index 45c666b0a2..b77d7c4020 100755
--- a/scripts/test-installation.pl
+++ b/scripts/test-installation.pl
@@ -112,7 +112,8 @@ while (<SOVERSIONS>) {
   next if (/^all-sonames/);
   chop;
   if (/^lib/) {
-    ($name, $version)= /^lib(.*)\.so-version=\.(.*)$/;
+    ($name, $version)= /^lib(.*)\.so-version=\.?(.*)$/;
+    $version = "" if ! defined $version;
     # Filter out some libraries we don't want to link:
     # - nss_ldap since it's not yet available
     # - libdb1 since it conflicts with libdb
@@ -123,7 +124,8 @@ while (<SOVERSIONS>) {
     next if ($build_mathvec == 0 && $name eq "mvec");
     if ($name ne "nss_ldap" && $name ne "db1"
 	&& $name ne "thread_db"
-	&& $name ne "nss_test1" && $name ne "libgcc_s") {
+	&& $name ne "nss_test1" && $name ne "nss_test2"
+	&& $name ne "libgcc_s") {
       $link_libs .= " -l$name";
       $versions{$name} = $version;
     }
@@ -171,14 +173,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";
diff --git a/shlib-versions b/shlib-versions
index b9cb99d2fb..e73d4270ac 100644
--- a/shlib-versions
+++ b/shlib-versions
@@ -75,3 +75,7 @@ libgcc_s=1
 
 # The vector math library
 libmvec=1
+
+# libcrypt is linked against this when built with --enable-nss-crypt.
+# It does not use a versioned soname.
+libfreebl3=

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]