On Thu, 13 Nov 2008 22:09:07 +0100, "Ralf Wildenhues" said: > OK, how about this. It is a slight backward incompatibility, but > not a large one: > - --verbose undoes --silent *and* enables verbose output (that one with > func_verbose), > - --no-silent *only* undoes --silent, > > It should still be bearable for the user, in the sense that if you > use --verbose rather than --no-silent, it's not a big problem. > And we don't have to think about what > --verbose --verbose --silent > > causes, we can just make the last one win. > > If you agree, then let's proceed this way. I don't mind who writes the > patch.
That sounds good to me. The help output would need a little re-wording: # --quiet, --silent don't print informational messages # -v, --verbose print informational messages (default) # --no-silent ??? I'll let you do that. <g> > > >> B) func_win32_libid() gives some confusing errors to users > > >> when (a) using recursive make, and (b) MAKEFLAGS does not > > >> contain $OBJDUMP. Add a diagnostic error message, rather > > >> than allowing $SED to die a horrible death. > [...] > > Actually, this may no longer be necessary given the _LT_DECL_OBJDUMP > > changes (I /said/ this was an old patch). Here's the thread: > > http://cygwin.com/ml/cygwin/2008-09/msg00552.html > > Ah, ok, thanks. I'll remove any of these bits from the revised patch(es). > > Well, one reason I sat on this for so long was the 'fallback' mechanism > > for deducing the dll name from the import library was just so...hideous. > > And it wasn't a fallback -- it was the only mechanism since I hadn't yet > > enhanced dlltool. > > Do you steer dlltool development, BTW? No. I've contributed a few patches over the years to dlltool and binutils, but that's it. > > The only reason to allow it is because (hopefully) that ugly fallback > > code can get flagged with a warning, and maybe in a year or so get > > removed. > > Sounds like a good idea. Of course, first I need to revise the dlltool patch and get it accepted; there have been some comments on the binutils list. > > Well, that, and it fixes a test that currently fails. > > Which one, and can you post output for failure as well as success with > the patch, please? demo-exec after demo-shared, in the old test suite. I'll post the output(s) tonight or tomorrow. > Hmm. I reviewed this whole function, and only when done I asked myself > this, more radical question: we go great lengths here to find out a > name. Iff we have a *.la file to go with the implib, can't we just > *know* the name? I mean, we produced that thing, it has the expected > name, no? That's what the *.la file was designed for: to not have to > look into the binary files for information. > > Or is this purely for import libraries not created with libtool (and > people who throw away *.la files)? The information (e.g. library to dlpreopen) is passed in $dlprefiles. But, if that filename is .la: func_mode_link(): ... dlfiles|dlprefiles) if test "$preload" = no; then # Add the symbol object into the linking commands. func_append compile_command " @SYMFILE@" func_append finalize_command " @SYMFILE@" preload=yes fi case $arg in *.la | *.lo) ;; # We handle these cases below. ... ...much later... *.la) # A libtool-controlled library. if test "$prev" = dlfiles; then # This library was specified with -dlopen. dlfiles="$dlfiles $arg" prev= elif test "$prev" = dlprefiles; then # The library was specified with -dlpreopen. dlprefiles="$dlprefiles $arg" prev= else deplibs="$deplibs $arg" fi continue ;; So far, so good. But then we eventually source the .la file, and end up here (this is, in fact, what's happening in the demo-shared case): # This library was specified with -dlpreopen. if test "$pass" = dlpreopen; then if test -z "$libdir" && test "$linkmode" = prog; then func_fatal_error "only libraries may -dlpreopen a convenience library: \`$lib'" fi # Prefer using a static library (so that no silly _DYNAMIC symbols # are required to link). if test -n "$old_library"; then newdlprefiles="$newdlprefiles $dir/$old_library" # Keep a list of preopened convenience libraries to check # that they are being used correctly in the link pass. test -z "$libdir" && \ dlpreconveniencelibs="$dlpreconveniencelibs $dir/$old_library" # Otherwise, use the dlname, so that lt_dlopen finds it. elif test -n "$dlname"; then newdlprefiles="$newdlprefiles $dir/$dlname" else newdlprefiles="$newdlprefiles $dir/$linklib" fi fi # $pass = dlpreopen We've stored the DLL name as just ONE of the entries in $newdlprefiles. But later, after we're done with all the $passes and are ready to actually perform the link -- and that means generating the symbol lists -- we scan back through $newdlprefiles (actually $dlprefiles, because there was a reassignment). During this scan thru $[new]dlprefiles we want to recover the $linklib that goes with each entry -- worse, not all entries in $[new]dlprefiles during that pass are DLLs, and so might not have a matching $linklib -- and we also no longer know the name of the .la file from which a particular entry in $newdlprefiles came from. There might not be one (do we allow -dlpreopen foo.a|foo.dll.a|foo.dll?). The point is, we perhaps STARTED with the .la file, but the whole point of the dlpreopen $pass is to replace each .la file in $dlprefiles with the name of the object from which the symbols should be extracted, to build the symbol table. So, pick one: either the DLL, or the import library (there is no static lib, the failure mode in question occurs when --disable-static). If you pick "DLL" -- then it's real hard to get the symbols (objdump ugliness, plus figuring out which ones are DATA). If you pick "implib" -- then it's real hard to get the correct DLL name (but not nearly as hard as extracting the correct symbols from the dll). But the name of the .la file is no longer available. I guess we could add another accumulator, $dl_linklib_prefiles [*], to populate during the dlpreopen $pass, that will have exactly as many entries as $[new]dlprefiles. These two accumulators would only in that when $[new]dlprefiles contains a DLL, $newdl_linklib_prefiles would contain the matching $linklib (or vice versa). And (on cygwin/mingw/cegcc?) we'd extract the symbols from the elements of $newdl_linklib_prefiles -- except when the elements of $newdlprefiles and $newdl_linklib_prefiles differ, the first entry of the symbol list would be replaced by the DLL name (e.g. the element from $newdlprefiles). [*] or the accumulator could be $dl_la_prefiles, but then it would need to have "magic" placeholder values corresponding to entries in the original $dlprefiles that were NOT .la files Is that really cleaner? > In summary, it'd be great if you could redo the patch(es) along the > comments in the previous message (but read below first). > Couple more nits inline: Ack. > After your patch, this idiom occurs three times in ltmain. Should we > factorize like this > > if func_import_lib "$dlprefile"; then ... > > with this? > > func_import_lib () > { > case `eval $file_magic_cmd \"\$dlprefile\" 2>/dev/null | $SED -e 10q` > in > *import*) : ;; > *) false ;; > esac > } > > If you agree, you can do this as an independent patch if you like > (preapproved). Not sure if the name should be func_win32_import_lib > rather. Sure. I think the function should be called func_win32_import_lib_p because it is a predicate. > > + dllname=`func_win32_dllname_for_implib "$dlprefile"` > > can you write func_win32_dllname_for_implib so that it stores its > result in a variable, so the caller doesn't need to fork? No problem. > > +func_win32_dllname_for_implib () > > +{ > > + $opt_debug > > + f_win32_d_for_i_implib="$1" > > + f_win32_d_for_i_can_use_dlltool=no > > + f_win32_d_for_i_dllname= > > + > > + # In recursive makes, DLLTOOL is often omitted from the > > + # passed-down MAKEFLAGS. As a courtesy, warn when this > > + # happens but don't fail; we have a workaround. > > + if test -z "${DLLTOOL}"; then > > + func_warning "\$DLLTOOL is not defined" > > See my comment on the win32-dll option. Ack. > > + else > > + # check for --identify option > > + if eval $DLLTOOL --help | $EGREP -- '--identify' >/dev/null ; then > > + f_win32_d_for_i_can_use_dlltool=yes > > + fi > > + fi > > This should be a configure test. Or, you could at least use a global > variable name here (and maybe factorize the test out into a separate > function) like > > if test -z "$dlltool_identify"; then > case `$DLLTOOL --help` in > *--identify*) dlltool_identify=: ;; > *) dlltool_identify=false ;; > esac > fi Either way. The advantage of a runtime test is the "system" libtool script (/usr/bin/libtool) would benefit without rebuilding if the system binutils were updated; but of course that slows down runtime operation. For "normal" usage -- an explicitly libtoolized package -- it makes no effective difference (other than speed of execution). On balance, a configure test seems better. > > + > > + # use fallback implementation when dlltool is not available, or > > + # does not have the --identify option. > > Or --identify did not work for some reason? Because otherwise this > line: > > > + if test -z "$f_win32_d_for_i_dllname"; then > > can just be an 'else' to the previous 'if'. Correct. If --identify is present but failed I wanted to go ahead and try the fallback code. But it's not likely that the gloriously ugly sed code will succeed where dlltool (with direct C access to libbfd) failed. So, 'else' it is... > > + # make sure argument is actually an import library > > + if eval $file_magic_cmd \"\$f_win32_d_for_i_implib\" 2>/dev/null | > > + $SED -e 10q | $EGREP "import" >/dev/null; then > > See func_import_lib above. Ack. > > + # In recursive makes, OBJDUMP is often omitted from the > > + # passed-down MAKEFLAGS. As a courtesy, flag an error when > > + # this happens (it's more humane than allowing the sed > > + # expression below to fail). > > + test -z "${OBJDUMP}" && func_fatal_error "\$OBJDUMP is not defined" > > See comment on win32-dll option. Ack. > > + f_win32_d_for_i_dllname=`$OBJDUMP -s --section '.idata$7' > > $f_win32_d_for_i_implib | > > + $SED -e '/^.\{43\}/!d' -e 's/^.\{43\}//' | > > + $SED -e ':a;N;$!ba;s/\n//g' -e 's/\.\.\.*//g' \ > > + -e 's/[ ][ ][ ]*//g' \ > > + -e 's/^[ ]*//' -e 's/[ ]*$//'` > [snip esoteric sed stuff] > > denoting the start of another object file? Also, we should be able to > finish the script as soon as we've found a match, no? Yes, and no. Stop-on-success was the way I originally coded the dlltool changes, but the suggestion here: http://sourceware.org/ml/binutils/2008-11/msg00085.html is to continue checking...and fail if more than one. It IS possible to have an import lib directly reference two (or more) DLLs (not counting forwarded symbols, which is a different thing) -- but IMO this is not likely except as some pathological corner case. Anyway, this would break libtool's symbol extraction anyway -- and the fix is...ugly. We'd have to parse the 'U __head_*_dll' symbol in each member of the .dll.a that contains a "real" symbol, match it up to the member that contains the 'I __head_cygz_dll' symbol, and take THAT member's 'U _*_dll_iname' symbol, and THEN locate the member that has the matching 'I _*_dll_iname'. This member presumbly contains the appropriate .idata$7 section, so now we have the specific DLL for the original 'real' symbol in question. Repeat for all symbols, building separate lists for each referenced DLL. Then, create two (or more) separate lt_${my_prefix}_LTX_preloaded_symbols arrays for each DLL and its symbols referenced by this frankenstein .dll.a. And try to do all that using portable sh code and only $SED and $AWK. No thanks. So, YES, the fallback code can certainly stop-on-success. > > > --- a/libltdl/m4/libtool.m4 > > +++ b/libltdl/m4/libtool.m4 > > > @@ -4185,12 +4186,12 @@ m4_if([$1], [CXX], [ > > ;; > > cygwin* | mingw* | cegcc*) > > _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | > > $global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 > > DATA/;/^.*[[ ]]__nm__/s/^.*[[ ]]__nm__\([[^ ]]*\)[[ ]][[^ ]]*/\1 > > DATA/;/^I[[ ]]/d;/^[[AITW]][[ ]]/s/.* //'\'' | sort | uniq > > > $export_symbols' > > + _LT_TAGVAR(exclude_expsyms, > > $1)=['_head_[A-Za-z0-9_]+_dll|[A-Za-z0-9_]+_dll_iname'] > > Now this change applies to cegcc as well. Hmm. True, but unless the format of the import libs emitted by cegcc differs in that _head_*_dll and *_dll_iname are allowed to be "real" symbols as opposed to book-keeping indirections for the compiler, it's probably correct for cegcc as well. > > - _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | > > $global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 > > DATA/'\'' | $SED -e '\''/^[[AITW]][[ ]]/s/.*[[ ]]//'\'' | sort | uniq > > > $export_symbols' > > + _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | > > $global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 > > DATA/;/^.*[[ ]]__nm__/s/^.*[[ ]]__nm__\([[^ ]]*\)[[ ]][[^ ]]*/\1 > > DATA/;/^I[[ ]]/d;/^[[AITW]][[ ]]/s/.* //'\'' | sort | uniq > > > $export_symbols' > > Can't we omit the /^.*[[ ]]__nm__/ before the 's' here? Cut-n-paste from the existing C++ version. It looks like they both could be modified as you suggest -- but surely there was a reason that apparent duplication was put there? I'll have to check the archives; I remember it took some doing to get this correct (which makes it a shame that the C part was lost sometime during the ->2.0 X ->2.2 development fiasco). This original change goes back to early 1.5 days, IIRC. > > + _LT_TAGVAR(exclude_expsyms, > > $1)=['_head_[A-Za-z0-9_]+_dll|[A-Za-z0-9_]+_dll_iname'] > > I'm actually not sure whether _GLOBAL__F[ID]_.* can appear on w32. > Do you know? They should happen with C++ code using constructors > and destructors IIRC. I couldn't find that symbol in cygxerces-c28.dll -- and the Xerces-C code uses lots of custom constructors and destructors. However, it doesn't have many (any?) static objects AFAIK; would that make a difference? -- Chuck
