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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

This appeared recently. Haven't tracked down a regression range yet.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Ted, can you point me to where libxul gets its build flags? I'm assuming there's just a missing space somewhere.

Comment 2

[(not currently active) Ted Mielczarek]

The main files are:
http://mxr.mozilla.org/mozilla-central/source/toolkit/library/Makefile.in
http://mxr.mozilla.org/mozilla-central/source/toolkit/library/libxul-config.mk
http://mxr.mozilla.org/mozilla-central/source/toolkit/library/libxul-rules.mk

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 4

[Benjamin Smedberg]

MSVC uses -libpath:DIR instead of -L DIR

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Attachment: patch v1

How does this look? If you like this, I'll file a follow-up bug for fixing the other instances like this.

Comment 6

[(not currently active) 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.)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Ben/Micah, is there any particular reason dist/bin was used in the original patch we should be aware of?

Comment 8

[Micah Gersten]

(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.

Comment 9

[Justin Wood (:Callek)]

Ryan, still working on this?

Comment 10

[Wan-Teh Chang]

Attachment: Use EXPAND_LIBNAME_PATH instead of EXPAND_MOZLIBNAME

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

Comment 11

[Wan-Teh Chang]

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

Comment 12

[Wan-Teh Chang]

Attachment: Use EXPAND_LIBNAME_PATH instead of EXPAND_MOZLIBNAME, v2

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 13

[Wan-Teh Chang]

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.

Comment 14

[Ginn Chen]

Comment on attachment 462708 [details] [diff] [review]
Use EXPAND_LIBNAME_PATH instead of EXPAND_MOZLIBNAME, v2

built successfully on Solaris

Comment 15

[Wan-Teh Chang]

http://hg.mozilla.org/mozilla-central/rev/6412d71fe958