On Fri, Sep 18, 2009 at 4:26 AM, Peter Rosin <p...@lysator.liu.se> wrote: > Hi Ralf, > > Den 2009-09-18 06:37 skrev Ralf Wildenhues: >> >> * Peter Rosin wrote on Thu, Sep 10, 2009 at 11:48:34AM CEST: >>> >>> Here's a couple of patches that implements support for -Wl, and >>> -Xlinker for MSVC. The first one >>> (rename-dashL_envvar-tolinker_envvar.patch) >>> is just a rename, to reduce confusion, >> >> In general, a rename from an ugly variable name like "dashL_envvar" is a >> good thing. However, it is also an API change that would normally need >> announcing in NEWS and asking ourselves why we messed up before and had >> to fix it. Luckily, this one is in pr-msvc-support only, not in master, >> so I don't have such a big problem with it. However, if you do it then >> please rename *all* instances of this ugliness (dashL_envvar_spec?). >> With that, I think the first patch is probably a good thing. > > I didn't know dashL_envvar was considered so ugly. Noone told me since > its introduction four years ago [1] so I was beginning to think that it > was maybe ok, but I admit to not having given it much thought lately. I > only changed it since the variable is now used for other things as well.
In my windows branch, I use link_search_path_spec as in: _LT_TAGDECL([], [link_search_path_spec], [1], [Flag to add a directory to the linker search path]) Then, somewhere in the "-L*" case of argument processing in func_mode_link. if test -n "$link_search_path_spec"; then this_deplib="$link_search_path_spec$dir" else this_deplib="-L$dir" fi where all cases of the existing "-L$dir" is replaced by "$this_deplib". Note also that there is an explicit case for "-LIBPATH:*" so that -LIBPATH can be used in user-defined environment variables for example. My windows branch seems to work ok for the PGI and Intel compilers on windows with a couple exceptions: * running executables (e.g. test programs for the testsuite) that use DLLs. * building DLLs with version information. Chris > > [1] http://lists.gnu.org/archive/html/libtool-patches/2005-08/msg00080.html > > How should I name dashL_envvar_spec? For MSVC it is set to '-LIBPATH:' > so that > > $ .../libtool --mode=link ... -L/foo/bar ... > > triggers a transformation into > > LINK=" -LIBPATH:C:/some/mount/foo/bar" cl ... > > So, linker_envvar is the name of the variable, and dashL_envvar_spec > is what to prepend when moving -L paths to that variable. Should I > just lose that variable altogether and hardcode the -LIBPATH: thing > in func_to_linker_envvar (aka func_dashL_to_envvar)? It's not as if > there are any other users of linker_envvar et al to consider at > this point... > > Is dashl_xform also a bad name? It's a sed expression that transforms > -lfoo into something more suitable (foo.lib in this case). Better > suggestion welcome (if it is indeed distasteful, I wouldn't know). > >>> and the second patch >>> (-Xlinker-msvc.patch) contains the new code... >> >> And the @lt_linker@ thing is again so ugly it makes me cringe. > > Oops, sorry, I didn't mean to scare you :-) > >> No, really, I don't want to see code like this in master, that will be >> unmaintainable a year down the road. Why are we not looking for -Wl, >> and transforming that if linker_envvar is nonempty? > > Because that would require $wl to be '-Wl,', which I thought was a bit > misleading. But I can do it that way if that's preferred, please > advise. > >> And why does the >> description of linker_envvar not state that it also holds other link >> flags (is that maybe *not* true for some other systems on which this >> variable is used)? > > After both these patches are applied, you have: > > @defvar linker_envvar > When linking, move all paths specified with @option{-L} options to > this variable, for toolchains where it makes sense to pass the library > search paths in an environment variable. Linker options passed with > @option{-Wl,} or @option{-Xlinker} may also be passed via this > environment variable if @var{wl} is set to @samp{@@lt_envvar@@}. > Normally disabled (i.e. @var{linker_envvar} empty). > @end defvar > > So, I think you might have only picked up the rename of the variable > and not the change in the description when its usage was actually > changed in the second patch? Or? > >> Does the expanded form of $wl ever get into .la files BTW? This would >> certainly make them unusable for another compiler later on (this is not >> something that is broken nor needs to be fixed with this patch, I'm just >> noting it here because I see it and this is one of the few systems where >> it makes a big difference.) > > No '-Wl,' options are added to neither inherited_linker_flags nor > dependency_libs I think, so we're cool there. I think. > >>> Ok for the pr-msvc-support branch? >> >> Well, to me that depends upon whether you want to rewrite this code for >> an eventual inclusion in master anyway or not. > > I'm willing to do a rewrite if that's what it's going to take, but I > prefer to do it before any "merge window" opens, and I need pointers on > what needs to be rewritten. Just having the branch as a dumping spot > for a pile of unacceptable patches is not much use. > >> Thanks, and sorry if that was a bit harsh, > > I can take the "heat", better harsh and timely than nothing at all... > > Cheers, > Peter > > > >