Closed Bug 1224490 Opened 9 years ago Closed 9 years ago

Kill LIBXUL_DIST

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(2 files)

Before bug 1038639, LIBXUL_DIST meant either the libxul SDK directory or $DIST. There is no such distinction anymore, and LIBXUL_DIST is now always the same as $DIST. Which means all the dynamic-ish references to it can go away, and the variable itself can be removed as well and be replaced with $DIST.
Bug 1217015 will take out a few references, so we don't need to take care of them here.
Depends on: 1217015
We never use the variable in that case
Assignee: nobody → mh+mozilla
Attachment #8687135 - Flags: review?(mshal)
Attached patch Kill LIBXUL_DISTSplinter Review
Attachment #8687136 - Flags: review?(mshal)
Attachment #8687135 - Flags: review?(mshal) → review+
Comment on attachment 8687136 [details] [diff] [review]
Kill LIBXUL_DIST

>diff --git a/js/src/configure.in b/js/src/configure.in
>@@ -1619,23 +1619,23 @@ ia64*-hpux*)
>     ;;
> 
> *-hpux*)
>     DLL_SUFFIX=".sl"
>     if test ! "$GNU_CC"; then
>     	DSO_LDOPTS='-b -Wl,+s'
>     	DSO_CFLAGS=""
>     	DSO_PIC_CFLAGS="+Z"
>-    	MKSHLIB='$(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -L$(LIBXUL_DIST)/bin -o $@'
>-    	MKCSHLIB='$(LD) -b +s -L$(LIBXUL_DIST)/bin -o $@'
>+    	MKSHLIB='$(CXX) $(CXXFLAGS) $(DSO_LDOPTS) -L$(DIST)/bin -o $@'
>+    	MKCSHLIB='$(LD) -b +s -L$(DIST)/bin -o $@'

The indentation here is a mix of spaces & tabs (leftover from the original patch - I only noticed because git complained when I applied it).

Everything else looks fine.
Attachment #8687136 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/7e91bbef386c
https://hg.mozilla.org/mozilla-central/rev/a1ded8c2c798
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
It seems this chunk:

-MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib'
+MOZ_FIX_LINK_PATHS="-Wl,-rpath-link,${DIST}/bin -Wl,-rpath-link,${prefix}/lib"

or rather this chunk ?

-    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,$(if $(X11BASE),$(X11BASE),/usr/X11R6)/lib'
+    if test -z "$X11BASE"; then
+        X11BASE=/usr/X11R6
+    fi
+    MOZ_FIX_LINK_PATHS="$MOZ_FIX_LINK_PATHS -Wl,-rpath-link,${X11BASE}/lib"

Broke OpenBSD, prefix is for some reason now unset (NONE) and i end up with (''' MOZ_FIX_LINK_PATHS ''', r''' -Wl,-rpath-link,/usr/obj/m-c/dist/bin -Wl,-rpath-link,NONE/lib -Wl,-rpath-link,/usr/X11R6/lib ''') - all libs in /usr/local/lib are not found at link time.

If i use $(prefix) in the first chunk, it is still wrong:

/usr/obj/m-c/config.status:    (''' MOZ_FIX_LINK_PATHS ''', r''' -Wl,-rpath-link,/usr/obj/m-c/dist/bin -Wl,-rpath-link,/lib -Wl,-rpath-link,/usr/X11R6/lib '''),

Previously there was no requirement to set prefix, it pointed to usr/local by default...
Flags: needinfo?(mh+mozilla)
I dont know what funky games mozbuild is doing, but if only it took the -L flags provided by pkg-config outputs to correctly resolve libs when linking, all those issues wouldnt happen...
Flags: needinfo?(mh+mozilla)
Depends on: 1225753
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.