* 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
