Re: MSVC: Support for response files with $NM.

[Ralf Wildenhues]

Hi Peter,

* Peter Rosin wrote on Tue, Jul 06, 2010 at 11:27:31AM CEST:
> Den 2010-07-06 08:07 skrev Ralf Wildenhues:
> >Did you confirm that the debian/gcc case actually uses the @FILE code
> >path in the testsuite (should be in the low_cmd_len tests)?
> 
> No I didn't, but now I have and nm @FILE is used by at least export.at,
> deplib-in-subdir.at and stresstest.at. And it works just fine for
> me.

Thanks for verifying.

> >>--- a/libltdl/config/ltmain.m4sh
> >>+++ b/libltdl/config/ltmain.m4sh
> >>@@ -6719,14 +6719,30 @@ EOF
> >>        $opt_dry_run || $RM $export_symbols
> >>        cmds=$export_symbols_cmds
> >>        save_ifs="$IFS"; IFS='~'
> >>-       for cmd in $cmds; do
> >>+       for cmd1 in $cmds; do
> >>          IFS="$save_ifs"
> >>-         eval cmd=\"$cmd\"
> >>+         eval cmd=\"$cmd1\"
> >>          func_len " $cmd"
> >>          len=$func_len_result
> >>          if 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
> >
> >Actually, why even do the expensive test for long command lines, and not
> >just go for the spec file all the time?  I'm not sure we really want to
> >do this, as it hampers debugging a bit (the developer doesn't readily
> >see the contents of the response file), but it surely would be a bit
> >more efficient.
> 
> I wanted to change as little as possible. The expensive test was not
> added by the patch, so I don't feel too bad about that. I didn't
> even realize it was expensive, but if that's the case it is
> certainly possible to first test for nm_file_list_spec and only do
> the max_cmd_len test otherwise.

As I said, I'm not sure which behavior is more desirable; there is a
tradeoff performance vs. readability of build output to make here.

This was really meant as a point for discussion rather than a patch nit,
sorry for not being clear enough here.

> >>+           save_libobjs=$libobjs
> >>+           save_output=$output
> >>+           output=${output_objdir}/${output_la}.nm
> >>+           libobjs=$nm_file_list_spec$output
> >>+           func_append delfiles " $output"
> >>+           func_verbose "creating $NM input file list: $output"
> >>+           for obj in $save_libobjs; do
> >>+             $ECHO "$obj"
> >>+           done>  "$output"
> >>+           eval cmd=\"$cmd1\"
> >
> >This eval is wrong and shouldn't be necessary, func_show_eval below
> >already does an evaluation.  Please check that it is not needed.
> 
> The double eval was there before, so the patch just copied the
> existing style. Or didn't it? The code was:

Whatever.  It is wrong to do the double eval for anything here, it's
only that the func_len test needs one because func_show_eval already
does an eval and func_len doesn't (and shouldn't).  I'll fix it.

Thanks,
Ralf