Closed Bug 561510 Opened 10 years ago Closed 9 years ago
warning LNK4044: unrecognized option '/L
../../dist/bin'; ignored when linking xul .dll
This appeared recently. Haven't tracked down a regression range yet.
Ted, can you point me to where libxul gets its build flags? I'm assuming there's just a missing space somewhere.
This was caused by bug 550823. Looking at config.mk (and the build logs), it appears that this pattern is used in many places and this warning is generated for the rest as well. Is the -L flag gcc-only?
MSVC uses -libpath:DIR instead of -L DIR
How does this look? If you like this, I'll file a follow-up bug for fixing the other instances like this.
Assignee: nobody → ryanvm
Attachment #443250 - Flags: review?(ted.mielczarek)
Comment on attachment 443250 [details] [diff] [review] patch v1 Why does this use $(DIST)/bin anyway? Most things we link from $(DIST)/lib. If it really needs to be /bin, you should use EXPAND_LIBNAME_PATH. If not, just use EXPAND_LIBNAME. (Sorry, I know this isn't your code originally, but we might as well fix it right.)
Attachment #443250 - Flags: review?(ted.mielczarek) → review-
Ben/Micah, is there any particular reason dist/bin was used in the original patch we should be aware of?
(In reply to comment #7) > Ben/Micah, is there any particular reason dist/bin was used in the original > patch we should be aware of? I put dist/bin as that's where I saw it installed and it worked :). It seems to be in dist/lib as well, so from my perspective, I have no problem. I was unaware of the convention of using dist/lib, but, it was my first patch.
Ryan, still working on this?
I found that the EXPAND_LIBNAME_PATH function is designed to solve exactly this problem. I'm not sure if we should use $(DIST)/lib or $(LIBXUL_DIST)/lib. I use $(DIST)/lib in this patch because it's seems to be where $(IMPORT_LIBRARY) or $(SHARED_LIBRARY) is installed: http://mxr.mozilla.org/mozilla-central/source/config/rules.mk#916
This MXR query shows two occurrences of -L$(LIBXUL_DIST)/lib $(MOZALLOC_LIB) \ that probably should also be fixed: http://mxr.mozilla.org/mozilla-central/search?string=MOZALLOC_LIB
toolkit/components/ctypes/tests/Makefile.in also has an "unrecognized option" linker warning if --enable-tests is specified. So I also fixed it and xpcom/build/Makefile.in.
Comment on attachment 462708 [details] [diff] [review] Use EXPAND_LIBNAME_PATH instead of EXPAND_MOZLIBNAME, v2 The two instances of -L$(LIBXUL_DIST)/lib $(MOZALLOC_LIB) \ come from bug 550371. Ginn, could you please test this patch on Solaris? Thanks.
Attachment #462708 - Flags: review?(ginn.chen)
Comment on attachment 462708 [details] [diff] [review] Use EXPAND_LIBNAME_PATH instead of EXPAND_MOZLIBNAME, v2 built successfully on Solaris
Attachment #462708 - Flags: review?(ginn.chen) → review+
Attachment #462708 - Flags: approval2.0?
Assignee: ryanvm → wtc
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
You need to log in before you can comment on or make changes to this bug.