Closed Bug 644987 Opened 14 years ago Closed 14 years ago

Wrong EXPAND_LIBNAME on mingw compilation

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla6

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix v1.0 (obsolete) (deleted) — Splinter Review
Its GCC case was (probably accidentally) removed by bug 584474 part 9, resulting in wrong linking arguments on mingw.
Attachment #521797 - Flags: review?(mh+mozilla)
Comment on attachment 521797 [details] [diff] [review] fix v1.0 That was deliberate. What exactly is the problem with mingw?
Attachment #521797 - Flags: review?(mh+mozilla) → review-
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.
(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?
(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.
Attached patch fix v1.1 (deleted) — Splinter Review
A patch with comment changes
Attachment #521797 - Attachment is obsolete: true
Attachment #528288 - Flags: review?(mh+mozilla)
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
Attachment #528288 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: