Re: [PATCH] tests: feed -no-undefined when linking libtool libraries

On 2012-09-19 11:26, Peter Rosin wrote:
> On 2012-09-19 09:31, Peter Rosin wrote:
>> * tests/runpath-in-lalib.at: Make sure shared libraries are created
>> on Windows by passing -no-undefined. Otherwise libb.la fails to record
>> a dependency on liba.la, and the final link of the program then fails
>> with undefined symbols.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
>> ---
>>  tests/runpath-in-lalib.at |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> Ok to push?
>> Or maybe the failure is deeper than this? Should libb.la record a
>> dependency on liba.la even if libb.la is static only?
>>
>> The relevant difference in libb.la with this patch is this (I have
>> elided changes to dlopen and library_names which are empty when
>> no shared library is built):
>>
>> @@ -17,7 +17,7 @@
>>  inherited_linker_flags=''
>>  
>>  # Libraries that this one depends upon.
>> -dependency_libs=' -R/home/peda/libtool/git/cygwin/tests/testsuite.dir/047/foobar '
>> +dependency_libs=' -R/home/peda/libtool/git/cygwin/tests/testsuite.dir/047/foobar  /home/peda/libtool/git/cygwin/tests/testsuite.dir/047/liba.la'
>>  
>>  # Names of additional weak libraries provided by this library
>>  weak_library_names=''
> 
> I likely is deeper, it seems this is a regression since 2.4.2.

I have bisected this regression to 962aa919f51cdf8e2cee4fb2d1d9bafa34d50887
syntax-check: fix violations and implement sc_prohibit_test_const_follows_var.

I looked through that insanely huge patch and it was not fun. I did
manage to find a couple of problems:

-    if test "$pic_mode" = no && test "$deplibs_check_method" != pass_all; then
+    if test yes = "$pic_mode" && test pass_all != "$deplibs_check_method"; then

-	    if test "$prev" = dlprefiles; then
+	    if test dlfiles = "$prev"; then

-	    if test "x`$SED 1q $export_symbols`" != xEXPORTS; then
+	    if test EXPORTS = "`$SED 1q $export_symbols`"; then

-if test "x[$]$2" = xyes; then
+if test yes != "[$]$2"; then

However, my eyes must have glazed over because it is not enough to fix those
bugs.

Comparing to master, I notice that:

* The export_symbols change has a fixup in
  b804ffabee2ce373d9bac6ae2b235ec68e0b55e8
  fixup: restore EXPORTS test
* The "x[$]$2" change has a fixup in
  11869b9c9eb8bcc8cb6a615141f522a447377324
  m4: fix logic error leading to -fno-rtti being added wrongly.

I have removed a long rant on my opinion of the offending patch, it
would do no good anyway...

Bottom line: More eyes needed on that patch!

Ok to push the below?

Cheers,
Peter

From 79d4c09db4317f2f96ba7cbbfc06a8a9da0ff984 Mon Sep 17 00:00:00 2001
From: Peter Rosin <[email protected]>
Date: Wed, 19 Sep 2012 14:23:26 +0200
Subject: [PATCH] fixup: restore stomped tests

Commit v2.4.2-120-g962aa91
syntax-check: fix violations and implement sc_prohibit_test_const_follows_var
inadvertedly stomped some comparisons.

* build-aux/ltmain.m4sh (func_mode_compile): Reverse test when checking
if non-PIC is attempted in shared libraries.
(func_mode_link): Check for dlprefiles, not dlfiles.
---
 build-aux/ltmain.m4sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh
index 14f3c37..69722ef 100644
--- a/build-aux/ltmain.m4sh
+++ b/build-aux/ltmain.m4sh
@@ -1350,7 +1350,7 @@ func_mode_compile ()
       pic_mode=default
       ;;
     esac
-    if test yes = "$pic_mode" && test pass_all != "$deplibs_check_method"; then
+    if test no = "$pic_mode" && test pass_all != "$deplibs_check_method"; then
       # non-PIC code in shared libraries is not supported
       pic_mode=default
     fi
@@ -5151,7 +5151,7 @@ func_mode_link ()
            fi

            # CHECK ME:  I think I busted this.  -Ossama
-           if test dlfiles = "$prev"; then
+           if test dlprefiles = "$prev"; then
              # Preload the old-style object.
              func_append dlprefiles " $pic_object"
              prev=