Wrong EXPAND_LIBNAME on mingw compilation

RESOLVED FIXED in mozilla6

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jacek Caban, Assigned: Jacek Caban)

Tracking

Trunk
mozilla6
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.87 KB, patch
glandium
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 521797 [details] [diff] [review]
fix v1.0

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-
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
(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.
(Assignee)

Comment 5

6 years ago
Created attachment 528288 [details] [diff] [review]
fix v1.1

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+
http://hg.mozilla.org/mozilla-central/rev/401962aff99d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.