Closed Bug 561510 Opened 10 years ago Closed 9 years ago

warning LNK4044: unrecognized option '/L../../dist/bin'; ignored when linking xul.dll

Categories

(Firefox Build System :: General, defect, minor)

x86
Windows 7
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla2.0b4

People

(Reporter: RyanVM, Assigned: wtc)

References

Details

(Whiteboard: [build_warning])

Attachments

(1 file, 2 obsolete files)

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?
Blocks: 550823
MSVC uses -libpath:DIR instead of -L DIR
Attached patch patch v1 (obsolete) — Splinter Review
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-
Whiteboard: [build_warning]
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
Attachment #443250 - Attachment is obsolete: true
Attachment #462701 - Flags: review?(benjamin)
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.
Attachment #462701 - Attachment is obsolete: true
Attachment #462708 - Flags: review?(benjamin)
Attachment #462701 - Flags: review?(benjamin)
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: review?(benjamin) → review+
Attachment #462708 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/6412d71fe958
Assignee: ryanvm → wtc
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.