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
