[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?) |
Date: |
Sun, 27 Jun 2010 22:43:24 +0200 |
User-agent: |
Mutt/1.5.20 (2010-04-22) |
* Charles Wilson wrote on Sun, Jun 27, 2010 at 02:51:21PM CEST:
> > In that case, Ralf's suggested libfile_$(transliterated implib name)
> > is used, because we have the .la file available which allows that
> > shortcut. The only time we need `dlltool --identify' is when
> > dlpreopening a non-libtool implib, where we have no .la file.
> > And the sed-fu is a fallback to THAT fallback.
>
>
> So...can I get a verdict? Is -dlpreopen not-an-la-file supposed to work?
I think Pierre's report was about using -dlpreopen file.la at link time,
but then not wanting to need the .la file at run time. I think that is
desirable. At link edit time, having a .la file is a reasonable thing I
think.
> > I'll note though that this patch makes me cringe: it introduces more
> > system-specific code in ltmain that is just bloat on other systems,
> > rather than in libtool.m4 where it belongs.
>
> Well, I did manage to come up with a mechanism to move most of
>
> func_cygming_dll_for_implib
> func_cygming_dll_for_implib_fallback
> func_cygming_dll_for_implib_fallback_core
> func_cygming_gnu_implib_p
> func_cygming_ms_implib_p
>
> into libtool.m4 (attached; only lightly tested pending Q above).
> However, I can't see how to move the other mods in func_generate_dlsyms
> and func_mode_link there.
>
> I tried to use Gary's _LT_PROG_XSI_REPLACE function, but using a sed
> script to create a sed script and all the quoting nightmares just made
> my head hurt. So, I have _LT_PROG_FUNCTION_REPLACE that uses the old
> 'copy half the script, emit the new function content, copy the rest of
> the script' algorithm.
I don't see this method as the new method of choice. We already have a
mechanism for years to transport values to the libtool script with
_LT_DECLs and _LT_TAGDECLs, and at least for small code snippets, that
should be used. I'd probably be more confident with code in ltmain that
you did test rather than a new transplanting method that has not been
tested well, and thus by definition has bugs.
> > Does this patch have any relation to Pierre Ossman's "Preloading without
> > .la" patch? http://thread.gmane.org/gmane.comp.gnu.libtool.general/7071
> > His copyright papers are through now, so we can look at that patch.
>
> Well, I *think* Pierre's patch would break cygwin -- but that'd be true
> with or without this patch. (The following statement in libltdl is not
> true, for cygwin when -dlpreload and --disable-static):
>
> /* Preloaded modules are always named according to their old
> archive name. */
>
> because at least currently, the second entry in the
> LTX_preloaded_symbols array is "cygfoo-N.dll" in those circumstances,
> not "libfoo.a".
Well yeah, this confusion and interface non-well-definedness is bad, no?
> The title of Pierre's thread is a bit confusing (at least to me). What
> he is talking about is using libltdl at runtime, with a variety of names
> refering to the same library (module, module.la, module.a, etc). That's
> why HE means by "Preloading without .la". My Question above is about
> *build* time, when you're trying to specify -dlpreopen not-a-.la.
OK.
> >> +func_tr_sh ()
> >> +{
> >> + case "$1" in
> >
> > No double-quotes needed here.
>
> Even if $1 might have spaces?
Yes. The shell does not do word splitting on the right hand side of an
assignment and in the argument to 'case'. Just try it:
foo="with space"; case $foo in *space*) echo whoo;; esac
> It's a pathname:
>
> func_tr_sh "$dlprefile"
>
> But, I guess, since dlprefile obtained as a member of a space-separated
> list, it BETTER not have any spaces in it. OK.
Irrelevant.
> >> + eval '$ECHO ": $dlprefile_dlbasename" >> "$nlist"'
> >> + eval '$ECHO ": $name " >> "$nlist"'
> >
> > Likewise.
>
> Those are (copies, adaptations of) pre-existing code:
>
> - $opt_dry_run || {
> - eval '$ECHO ": $name " >> "$nlist"'
> - eval "$NM $dlprefile 2>/dev/null | $global_symbol_pipe >>
> '$nlist'"
>
> I don't feel comfortable folding in a change like that as part of this
> patch, but I'd be happy to supply a separate follow-on patch to change
> them all at once.
Don't worry. I'm working on a related change anyway that fixes a lot of
the eval stuff.
> >> + fi
> >> + eval "$NM $dlprefile 2>/dev/null | $global_symbol_pipe |
> >> + $SED -e '/I __imp/d' -e 's/I __nm_/D /;s/_nm__//' >>
> >> '$nlist'"
> >
> > This can probably have the outer double quotes removed, eval moved to
> > $global_symbol_pipe, and quotes around $nlist removed. Actually, don't
> > change this, because it might be the eval isn't needed at all; I will
> > check this and change all uses in libtool then.
>
> Again, this is a copy of pre-existing code, so...I'll leave this to you.
OK.
> >> +# func_cygming_dll_for_implib_fallback_core SECTION_NAME LIBNAMEs
> >> +# Echos the name of the DLL associated with the
> >> +# specified import library.
> >
> > You'd save a fork if this function stores its result in a variable.
> > I'd just use sharedlib_from_linklib_result for that.
>
> I can't actually do this. The problem is that the function is simply a
> big pipeline:
>
> objdump $1 | sed | sed
>
> So, the caller (func_cygming_dll_for_implib_fallback) invokes as
>
> sharedlib_from_linklib_result=`func_cygming_dll_for_implib_fallback_core
> ...`
>
> which is a fork, it is true. But for
> func_cygming_dll_for_implib_fallback_core to put the result in a
> variable, IT would have to do:
>
> sharedlib_from_linklib_result=`objdump $1 | sed | sed`
>
> which is...an extra fork. So it's six of one, a half-dozen of the
> other. (And, I found that I had difficulty trying to express it as
> `objdump $1 | sed |sed` with such a complicated sed expression. sh was
> not happy.
>
> That's actually the REASON I split this operation into a main function
> and a _core function.
OK, fine like this then. Thanks for the explanation, and sorry for not
seeing that.
> >> +func_cygming_dll_for_implib_fallback_core ()
> >> +{
> >> + $opt_debug
> >> + $OBJDUMP -s --section "$1" "$2" 2>/dev/null |
> >> + sed '/^Contents of section '"$1"':/{
> >
> > Can $1 contain slash, dollar, ^, characters?
>
> Yes. In fact, it only ever has one of two values: '.idata$6' and
> '.idata$7'.
Well then the characters which are special in sed regexes need to be
escaped, otherwise a .idata$6 will never match: $ matches end of line,
there is never a 6 after an end of line. A period OTOH matches any
character. To generally escape a string so that sed matches it
literally:
match_literal=`$ECHO "string" | sed 's/[].[^$\\*|]/\\\\&/g'`
sed "/$match_literal/..."
(untested, from Automake). Or you just additionally pass a regex.
>
> >> + # Of those that remain, print the first one.
> >> + sed -e '/^\./d' -e '/^.\./d' | sed -n -e '1p'
> >
> > Can't you join the last three sed scripts? Replace the last commmand /./p
> > of the third-to-last script with
> > /^\./d
> > /^.\./d
> > p
> > q
> >
> > and be done with it. Right?
>
> No, it doesn't work. The best I can do is consolidate them into a single
> separate expression:
>
> sed -e '/^\./d;/^.\./d;q'
OK thanks.
> > What is the eval for here? If it is needed, it should probably be
> > before the assignment, a la
> > eval "func_cygming_gnu_implib_tmp=..."
> >
> > or, if just for the symbol pipe, then just inside there, but the
> > backslash before $global_symbol_pipe really makes no sense to me.
> ...
> >> + func_cygming_ms_implib_tmp=`eval "\$NM \$1 | \$global_symbol_pipe |
> >> grep '_NULL_IMPORT_DESCRIPTOR'"`
> >
> > Likewise.
> >
>
> It's only needed for $global_symbol_pipe.
Yeah, that's the bugger I'm fixing currently.
I'll note again that you do have OK in the manner I wrote in my last
review.
Thanks,
Ralf