On 6/27/2010 4:43 PM, Ralf Wildenhues wrote: > * 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.
So...I *don't* need to worry about -dlpreopen not-an-.la? The issue is that I can't figure out how *current* libtool EVER gets here: (current master ltmain.m4sh:1984:func_generate_dlsyms) for dlprefile in $dlprefiles; do func_verbose "extracting global C symbols from \`$dlprefile'" func_basename "$dlprefile" name="$func_basename_result" $opt_dry_run || { eval '$ECHO ": $name " >> "$nlist"' eval "$NM $dlprefile 2>/dev/null | $global_symbol_pipe >> '$nlist'" } done in that situation, with anything IN $dlprefiles -- because in ltmain.m4sh, we have: 5764 if test "$linkmode" = prog; then 5765 dlfiles="$newdlfiles" 5766 fi 5767 if test "$linkmode" = prog || test "$linkmode" = lib; then 5768 dlprefiles="$newdlprefiles" 5769 fi and at this point, both newdlfiles and newdlfiles are empty, when the argument to libtool's -dlpreopen option is not a libtool .la library. So...we APPEAR to have a bunch of dead code. It obviously isn't SUPPOSED to be dead -- or it wouldn't be there. So, I can either keep (that is, commit) all of my new stuff in this patch, some of which will also be dead code, in anticipation of somebody tracking down WHY it and these existing snippets are (currently) dead, and brings them back to life. Or I can NOT commit any new (dead) code and commit only those bits that are presently "live", and wait until after the existing dead code is resurrected, and THEN add those particular bits that I'd've held back. Which? (Note that I didn't realize it was actually dead until I really started trying hard to exercise the dll-to-implib stuff /in situ/ in current libtool tip. This dll-to-implib stuff was necessary in the early days of this patch development, but it SEEMS like intervening changes have made it dead, along with other existing parts of libtool.) The code we're talking about is, specifically: >> 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 and the one location that calls it (via eval $shared_to_implib_cmd) -- plus the .m4 code that sets the value of shared_to_implib_cmd. Naturally I'd rerun all the tests on multiple platforms if I made such a major change. >> 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, Yeah, that's the problem. You complained that these functions added a lot of parse time to all the other platforms that would never use them, presumably because they were BIG functions and there were several of them. Presumably, the parse-time cost of small functions is low, unless there are a TON. Worst case, of course, is a TON of big functions that nobody but $myplatform uses. So...I tried to move the big ones to libtool.m4 (leaving only small stubs in ltmain.m4sh), because (a) they're the worst culprits in parse-time cost, and (b) they can't fit in _LT_DECLs anyway. > 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. That's fine. Private branches are easily deleted. But please, in the future, don't complain so strongly ([your patch] "makes me cringe") about architectural issues if you don't actually want to see them fixed: "system-specific code in ltmain...rather than in libtool.m4 where it belongs." I feel (more) discouraged now (than usual), having wasted so much time addressing a criticism of a patch that wasn't meant to be taken seriously. The reason I'm reacting so strongly here is because you've stated this same criticism many times over the years, in relation to win32 patches (mine and others) -- especially the cross compile patch which is next on the list. In fact, I have often wondered if the reason many of my patches -- and Peter's -- tend to languish so long is because of these aesthetic objections to the system-specific code that cyg/mingw seems to require... I keep wondering if folks think "gee, there must be a way to do that in m4...let me think about that a while". And a while becomes a month, and a month becomes three, and three becomes twelve...and the vapor perfect defeats the concrete good. Anyway, this patch AND that upcoming cross-compile patch both add several large system-specific new functions to ltmain.sh. Since you objected to them now, I figured I'd hear it again THEN, so...I took this opportunity to TRY and create the appropriate infrastructure to handle LARGE system-specific functions from m4. (Few if any of these functions are suitable candidates for single-line $foo_cmd _LT_DECL-style treatment, or even just few-line $foo_cmd) I won't bother to do that in the future. I'll let somebody else devise the blessed way to address the issue, and then I'll be glad to assist in adapting (hopefully soon-to-be-committed) sections of code that could exploit the new mechanisms. Whatever those mechanisms are, and whenever that actually happens. Unless, of course, the blessed way is to try to shoe-horn a 40-line sed expression into a $foo_cmd. That's just...crazy talk. At some point you've got to consider the difficulties in *debugging* these things, if they're stuffed into a blah~blah~blah~blah nightmare variable. It's hard enough putting complex function bodies into m4, given the re-autotooling you have to do between tests: VERY long edit-"compile"-test cycles. Debugging the eval of a giant $foo_cmd, whose CONTENTS are specified by the .m4? That's doubly bad. <shudder>. >> 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? Sure it is. But some of these considerations are hard to accommodate on win32 if the .la file is not available at runtime, AND the caller doesn't explicitly specify the actual DLL name when calling lt_dlopen. There isn't enough information otherwise for libltdl to know how to turn "module" into "cygmodule-27.dll". Accommodations for dlpreopen such as Pierre wants, appear to me to work against the normal dlopen needs... unless we subvert normal linker behavior on win32, and always turn -dlpreopen foo into -Wl,-Bstatic foo -Wl,-Bdynamic (implying -dlpreopen + -disable-static is verboten, which is precisely what this patch is trying to fix!) Anyway, if we're going to try and nail down these aspects of the API, I think that's a good thing to do for libltdl2 (whether Gary's or some other brainstorm). >>>> +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 I did try it. But I didn't use a variable, I did case john smith in john smith ) echo yes ;; esac and got a syntax error. I didn't realize the behavior was different with $vars vs. literals (or perhaps the difference is between interactive and non-interactive shells). >> 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. Ack. >>>> +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. I'll just pass a regex, and update the documentation accordingly. > I'll note again that you do have OK in the manner I wrote in my last > review. OK, I'll return to (take 7), subject to possibly ripping out a lot of it depending on the answer to the Q above. -- Chuck