Re: [PATCH 7/7] Prefer $NM @file over calculating the cmd line length.

[Peter Rosin]

Hi Ralf,

Den 2010-09-12 10:01 skrev Ralf Wildenhues:
>> +          if { test "$len" -lt "$max_cmd_len" \
>> +               || test "$max_cmd_len" -le -1; } \
>> +             && ! $tool_conversion_needed_and_working
> 
> `! $command' is not portable, please reverse the logic or use
> test foo = bar or so.
> 
> The logic is flawed here, you are testing potentially-undefined $len.
> Did you mean to reverse the && order (the shell does short-circuiting
> of || and && so that would seem appropriate here)?

Argghh. I had one version which I scrapped pretty quickly because I saw
that problem, but I apparently forgot about it again.

>> +          then
>>              func_show_eval "$cmd" 'exit $?'
> 
> This code has eval problems, but they were before your patch, so I guess
> they are not on you to fix.

And even if so, I would definitely not want to squash such an unrelated
and potentially problematic change in with this patch.

Anyway, here's a new attempt addressing the above if-crap and the other
nits that I didn't bother to quote.

I have tested stresstest.at on MSYS/gcc, MinGW/MSVC, Cygwin/gcc and
Cygwin/MSVC. Passes all over.

In the logs I see this on MSYS and for Cygwin/gcc:
   libtool: link: dumpbin -symbols  various/relative/paths ...
and
   libtool: link: dumpbin -symbols  /various/abs/paths ...

and on Cygwin/MSVC I see:
   libtool: link: dumpbin -symbols @various/relative/paths ...
and
   libtool: link: dumpbin -symbols @C:/other/abs/paths ...

So it is working as designed and not by coincidence.

Ok to push this time?

Cheers,
Peter


>From a5e4bb2eb0a47ea72ccc31a43e149a1cb6a557b8 Mon Sep 17 00:00:00 2001
From: Peter Rosin <address@hidden>
Date: Mon, 13 Sep 2010 09:48:57 +0200
Subject: [PATCH] Prefer $NM @file when the toolchain isn't native to $build.

* libltdl/config/ltmain.m4sh (func_mode_link): Avoid calculating
the command line length and take the @file branch *if* the file
names needs to be converted for the toolchain and the @file
branch works.  Fixes problems in stresstest.at when doing a cross
from Cygwin to MinGW using MinGW native tools (a.k.a. a "faked"
cross), and for the specific case of MSVC on Cygwin it makes the
test pass.

Signed-off-by: Peter Rosin <address@hidden>
---
 ChangeLog                  |   11 +++++++++++
 libltdl/config/ltmain.m4sh |   21 +++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c7d0336..ddd9c3e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2010-09-13  Peter Rosin  <address@hidden>
+
+       Prefer $NM @file when the toolchain isn't native to $build.
+       * libltdl/config/ltmain.m4sh (func_mode_link): Avoid calculating
+       the command line length and take the @file branch *if* the file
+       names needs to be converted for the toolchain and the @file
+       branch works.  Fixes problems in stresstest.at when doing a cross
+       from Cygwin to MinGW using MinGW native tools (a.k.a. a "faked"
+       cross), and for the specific case of MSVC on Cygwin it makes the
+       test pass.
+
 2010-09-12  Peter Rosin  <address@hidden>
 
        * .gitignore: Ignore *.obj files for MSVC (and w32 in general).
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 6036f4f..18f0f39 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -7349,10 +7349,23 @@ EOF
            save_ifs="$IFS"; IFS='~'
            for cmd1 in $cmds; do
              IFS="$save_ifs"
-             eval cmd=\"$cmd1\"
-             func_len " $cmd"
-             len=$func_len_result
-             if test "$len" -lt "$max_cmd_len" || test "$max_cmd_len" -le -1; 
then
+             # Take the normal branch if the nm_file_list_spec branch
+             # doesn't work or if tool conversion is not needed.
+             case $nm_file_list_spec~$to_tool_file_cmd in
+               *~func_convert_file_noop | *~func_convert_file_msys_to_w32 | ~*)
+                 try_normal_branch=yes
+                 eval cmd=\"$cmd1\"
+                 func_len " $cmd"
+                 len=$func_len_result
+                 ;;
+               *)
+                 try_normal_branch=no
+                 ;;
+             esac
+             if test $try_normal_branch = yes \
+                && { test "$len" -lt "$max_cmd_len" \
+                     || test "$max_cmd_len" -le -1; }
+             then
                func_show_eval "$cmd" 'exit $?'
                skipped_export=false
              elif test -n "$nm_file_list_spec"; then
-- 
1.7.1