644987 - Wrong EXPAND_LIBNAME on mingw compilation

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

Its GCC case was (probably accidentally) removed by bug 584474 part 9, resulting in wrong linking arguments on mingw.

Comment 1

[Mike Hommey [:glandium]]

Comment on attachment 521797 [details] [diff] [review]
fix v1.0

That was deliberate. What exactly is the problem with mingw?

Comment 2

[Jacek Caban]

Here is an example of the error (when crosscompiling using mingw on Linux):

/usr/bin/python2.7 /home/jacek/mozilla-build/mozilla-central/config/pythonpath.py -I../../../../config /home/jacek/mozilla-build/mozilla-central/config/expandlibs_exec.py --uselist --  i686-w64-mingw32-g++ -mno-cygwin   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long -fno-strict-aliasing -mms-bitfields -mstackrealign -pipe  -DDEBUG -D_DEBUG -DTRACING -g -o TestXREMakeCommandLineWin.exe TestXREMakeCommandLineWin.o -mconsole -static-libgcc -static-libstdc++    -L../../../../dist/bin -L../../../../dist/lib  -luuid -lgdi32 -lwinmm -lwsock32 libcomctl32.a libws2_32.a libshell32.a   
i686-w64-mingw32-g++: libcomctl32.a: No such file or directory
i686-w64-mingw32-g++: libws2_32.a: No such file or directory
i686-w64-mingw32-g++: libshell32.a: No such file or directory

Note that we use libcomctl32.a to link to comctl32, but the right syntax for system libraries is -lcomctl32. This argument is expanded by EXPAND_LIBNAME.

Comment 3

[Jacek Caban]

(In reply to comment #1)
> That was deliberate.

Could you please tell me the reason? My guess was that it was done to allow using EXPAND_LIBNAME for static libraries created in a middle of compilation. I went thru all its occurrences in the tree and I've found that all of them were for Windows system libraries (except one for AIX, but that's still a system library). System libraries syntax needs to be -l... and EXPAND_LIBNAME seems like exactly the place to do this. How would you prefer this to be fixed?

Comment 4

[Mike Hommey [:glandium]]

(In reply to comment #3)
> (In reply to comment #1)
> > That was deliberate.
> 
> Could you please tell me the reason? My guess was that it was done to allow
> using EXPAND_LIBNAME for static libraries created in a middle of compilation. I
> went thru all its occurrences in the tree and I've found that all of them were
> for Windows system libraries (except one for AIX, but that's still a system
> library). System libraries syntax needs to be -l... and EXPAND_LIBNAME seems
> like exactly the place to do this. How would you prefer this to be fixed?

Actually, now that I take a deeper look at it, EXPAND_LIBNAME is only ever used with system libraries, so it should just be good to do what your patch does. However, I now realize that we should really update the comments above.

Comment 5

[Jacek Caban]

Attachment: fix v1.1

A patch with comment changes

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 528288 [details] [diff] [review]
fix v1.1

Review of attachment 528288 [details] [diff] [review]:

r=glandium with the following nits applied

::: config/rules.mk
@@ +90,5 @@
 VPATH += $(LIBXUL_SDK)/lib
 endif
 
 # EXPAND_LIBNAME - $(call EXPAND_LIBNAME,foo)
+# expands to foo.lib or -lfoo, depending on linker arguments syntax

Replace lib with $(LIB_SUFFIX) and add "should only be used for system libraries"

@@ +96,2 @@
 # EXPAND_LIBNAME_PATH - $(call EXPAND_LIBNAME_PATH,foo,dir)
+# expands to dir/foo.lib

Replace lib with $(LIB_SUFFIX)

@@ +99,2 @@
 # EXPAND_MOZLIBNAME - $(call EXPAND_MOZLIBNAME,foo)
+# expands to $(DIST)/lib/foo.lib

likewise

Comment 7

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/401962aff99d