* Paolo Bonzini wrote on Fri, Aug 13, 2010 at 03:23:06AM CEST:
> ---
Erm, you should be mentioning the files you change in your log entries,
and Libtool also doesn't auto-generate its ChangeLog (yet), so you need
that, too, also for all the other patches in the sysroot branch.
Sorry for not noticing that sooner.
> doc/libtool.texi | 18 ++++++++++++------
> libltdl/config/ltmain.m4sh | 21 +++++++++++++++++++++
> tests/sysroot.at | 3 +++
> --- a/doc/libtool.texi
> +++ b/doc/libtool.texi
> @@ -1721,12 +1721,18 @@ commands are also completed.
> @cindex finish mode
> @cindex mode, finish
>
> -...@dfn{finish} mode helps system administrators install libtool libraries
> -so that they can be located and linked into user programs.
> -
> -Each @var{mode-arg} is interpreted as the name of a library directory.
> -Running this command may require superuser privileges, so the
> -...@option{--dry-run} option may be useful.
> +...@dfn{finish} mode has two functions. One is to help system administrators
> +install libtool libraries so that they can be located and linked into
> +user programs. To invoke this functionality, pass the name of a library
> +directory as @var{mode-arg}. Running this command may require superuser
> +privileges, and the @option{--dry-run} option may be useful.
> +
> +The second is to facilitate transferring libtool libraries to a native
> +compilation environment after they were built in a cross-compilation
> +environment. Cross-compilation environments may rely on recent libtool
> +features, and running libtool in finish mode will make it easier to
> +work with older versions of libtool. This task is performed whenever
> +the @var{mode-arg} is a @samp{.la} file.
This is fine with me, but I think it wouldn't hurt if we mentioned
'sysroot' explicitly here. You can of course also just do this when
you add general description of --with-sysroot.
> --- a/libltdl/config/ltmain.m4sh
> +++ b/libltdl/config/ltmain.m4sh
> @@ -1418,6 +1418,27 @@ func_mode_finish ()
> fi
> done
>
> + if test -n "$libs"; then
> + tmpdir=`func_mktempdir`
> + if test -n "$lt_sysroot"; then
> + sysroot_regex=`$ECHO "$lt_sysroot" | $SED 's/[].[^$\\*|]/\\\\&/g'`
I think libltdl/config/general.m4sh could have
# Sed substitution that turns a string into a regex matching for the
# string literally.
sed_make_literal_regex='s/[].[^$\\*|]/\\\\&/g'
and use that also in func_cygming_dll_for_implib_fallback_core.
Hmpf, no, that won't work, there, /.../ is used as regex delimiter.
Using | here isn't a good idea because sed may interpret \| as
alternation or not; (and that means the regex in
func_cygming_dll_for_implib_fallback_core is slightly wrong, too).
So how about
# Sed substitution that turns a string into a regex matching for the
# string literally inside a regex delimited with a slash ('/').
sed_make_literal_regex='s/[].[^$\\*/]/\\\\&/g'
and then use slashes here:
> + sysroot_cmd="s|\([ ']\)$sysroot_regex|\1|g;"
Again, since this suggestion is also fixing existing code, it's fine
to postpone this. (I need to remember to not make people fix existing
code in their patches for new features.)
> + else
> + sysroot_cmd=
> + fi
> +
> + # Remove sysroot references
> + for lib in $libs; do
> + $opt_dry_run || {
> + sed -e "${sysroot_cmd} s/\([ ']-[LR]\)=/\1/g; s/\([ ']\)=/\1/g" $lib \
> + > $tmpdir/tmp-la
> + mv -f $tmpdir/tmp-la $lib
> + }
How about wrapping the whole loop in if $opt_dry_run and in the dry
case, print "removing references to $lt_sysroot and \`=' prefixes from
$libs"?
> + file="$outputname"
What's this line for?
> + done
> + $opt_dry_run || ${RM}r "$tmpdir"
> + fi
> +
OK with those fixed.
Thanks,
Ralf
