Closed
Bug 1478499
Opened 6 years ago
Closed 6 years ago
Linux64 Tup build of the Spidermonkey shell
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: mgaudet, Assigned: chmanchester)
References
Details
Attachments
(6 files)
59 bytes,
text/x-review-board-request
|
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
Details | |
59 bytes,
text/x-review-board-request
|
Details | |
59 bytes,
text/x-review-board-request
|
mshal
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mshal
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mshal
:
review+
|
Details |
I managed to build the spidermonkey shell with tup recently and encountered three problems while doing it: Versions and Methods: Linux 64 (Ubuntu 17.04), grabbed tup with cd ~/.mozbuild && mach artifact toolchain --from-build linux64-tup (https://docs.google.com/presentation/d/1SHb9Vp0aWPbHz1Eo53h9Xbh4-6qmukSknFQXQc2BVqA/edit\#slide=id.g3c628ff6b6\_3\_0) Configure and build like this: mkdir js/src/build_tup_DBG.OBJ cd js/src/build_tup_DBG.OBJ ../configure --enable-warnings-as-errors --enable-tests --disable-optimize --enable-debug --enable-build-backends=Tup Failure 1: ---------- [ ETA~=<1s Remaining=19 ] 52%tup error: Unable to insert duplicate tupid 355243 - [355243] js/src/build_tup_DBG.OBJ/dist/system_wrappers/pixman.h * 22) [0.010s] js/src/build_tup_DBG.OBJ/config tup error: The output file '../dist/system_wrappers/pixman.h' is listed multiple times in a command. tup error: Error parsing Tupfile line 7 Line was: ': $(MOZ_OBJ_ROOT)/<early-generated-files> |> ^o python /home/mgaudet/mozilla-unified-clean/config/make-system-wrappers.py:ge...' [ ETA~=<1s Remaining=18 ] 55% *** tup: 1 job failed. Resolution: Edit config/Tupfile, and remove one instance of ../dist/system\_wrappers/pixman.h Failure 2: ---------- * 39) [0.000s] js/src/build_tup_DBG.OBJ/js/src/shell tup error: Explicitly named file 'js' in subdir 'js/src/build_tup_DBG.OBJ/dist/bin' is scheduled to be deleted (possibly the command that created it has been removed). tup error: Error parsing Tupfile line 9 Line was: ': /home/mgaudet/mozilla-unified-clean/js/src/build_tup_DBG.OBJ/dist/bin/js |> !tup_ln |> $(MOZ_OBJ_ROOT)/js/src/js $(MOZ_OBJ...' [ ETA~=<1s Remaining=1 ] 97% *** tup: 1 job failed. mgaudet@ubuntu:~/mozilla-un Resolution: There's a rule in js/src/shell/Tupfile : /home/mgaudet/mozilla-unified-clean/js/src/build_tup_DBG.OBJ/dist/bin/js |\> !tup\_ln |\> $(MOZ_OBJ_ROOT)/js/src/js $(MOZ_OBJ_ROOT)/\<default\> Delete it. Failure 3: ---------- 1314) [0.073s] js/src/build_tup_DBG.OBJ/modules/fdlibm/src: CXX /home/mgaudet/mozilla-unified-clean/modules/fdlibm/src/e_acos.cpp * 1315) js/src/build_tup_DBG.OBJ/js/src/build: LINK libmozjs-63a1.so /usr/bin/ld.gold: error: cannot open symverscript: No such file or directory /usr/bin/ld.gold: fatal error: unable to parse version script file symverscript clang-5.0: error: linker command failed with exit code 1 (use -v to see invocation) *** tup errors *** *** Command ID=331290 failed with return value 1 tup error: Expected to write to file 'libmozjs-63a1.so' from cmd 331290 but didn't 1316) [4.340s] js/src/build_tup_DBG.OBJ/js/src/build: AR libjs_static.a [ Resolution: Edit js/src/build/Tupfile, and remove the version script argument With the above three failures fixed, it appears we build a working js shell (and jsapi-tests)
Assignee | ||
Comment 1•6 years ago
|
||
To summarize some comments on irc: Failure 1 seems to be due to processing pixman.h twice in system-headers.mozbuild. This should be trivial to fix. Failure 2 appears to be due to tup's distaste for rules depending on an output that is generated later in that file. We may be able to work around this until it's fixed in tup upstream. Failure 3 appears to be a generated file (symverscript) at https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/js/src/build/Makefile.in#13 that needs to be converted to moz.build. I did something similar for the "symverscript" next to libxul in bug 1423815.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → cmanchester
Assignee | ||
Comment 2•6 years ago
|
||
I have a set of patches for this, but unfortunately Failure 2 is a little more complicated than I thought. We're attempting to install the js shell executable to $objdir/js/src/js, but many compile invocations will attempt to find headers in a potential directory $objdir/js/src/js (js-confdefs.h, in $objdir/js/src, contains #include "js/RequiredDefines.h"), so tup thinks all of these compilations should depend on the file $objdir/js/src/js. As far as I can tell the additional symlink to $objdir/js/src/js is a matter of convention, although I don't know how useful this convention is to people in practice, so maybe we can skip this in the tup backend for now.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Attachment #8996144 -
Flags: review?(core-build-config-reviews) → review?(nfroyd)
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8996144 [details] Bug 1478499 - Do not list pixman.h twice in system-headers.h https://reviewboard.mozilla.org/r/260370/#review267442
Attachment #8996144 -
Flags: review?(nfroyd) → review+
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8996145 [details] Bug 1478499 - Skip additional js shell installation via ObjdirFiles in the tup build. https://reviewboard.mozilla.org/r/260372/#review267638
Attachment #8996145 -
Flags: review+
Updated•6 years ago
|
Attachment #8996145 -
Flags: review?(core-build-config-reviews)
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8996146 [details] Bug 1478499 - Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. https://reviewboard.mozilla.org/r/260374/#review267640 ::: js/src/build/moz.build:34 (Diff revision 1) > + if CONFIG['OS_TARGET'] == 'Linux': > + GENERATED_FILES += ['symverscript'] > + GENERATED_FILES['symverscript'].script = '/toolkit/library/gen_symverscript.py' > + GENERATED_FILES['symverscript'].inputs = ['symverscript.in'] > + GENERATED_FILES['symverscript'].flags = [ > + CONFIG['JS_LIBRARY_NAME'].replace('-', '_'), For me locally, the Makefile version produced a symverscript with -DVERSION='js', but this ends up with something like 'mozjs_63a1'. Is that intentional? Or does it not matter? ::: js/src/build/moz.build (Diff revision 1) > DIST_INSTALL = True > > -# Ensure symbol versions of shared library on Linux do not conflict > -# with those in libxul. > -if CONFIG['OS_TARGET'] == 'Linux': > - LDFLAGS += ['-Wl,-version-script,symverscript'] When using clang locally, this originally added the -Wl,-version-script,symverscript to the command-line for linking libmozjs-63a1.so, but after this patch I no longer see it on the command-line (in either make or tup). I think for tup we currently don't handle SYMBOLS_FILE at all, and for rules.mk it looks like no flag is added on Linux unless GCC_USE_GNU_LD is set.
Updated•6 years ago
|
Attachment #8996146 -
Flags: review?(core-build-config-reviews)
Assignee | ||
Comment 9•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8996146 [details] Bug 1478499 - Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. https://reviewboard.mozilla.org/r/260374/#review267640 > For me locally, the Makefile version produced a symverscript with -DVERSION='js', but this ends up with something like 'mozjs_63a1'. Is that intentional? Or does it not matter? Huh, yeah, it looks like it's expecting to see the `mozjs_$version` form since it replaces dashes with underscores in the make version, but it does always see "js". > When using clang locally, this originally added the -Wl,-version-script,symverscript to the command-line for linking libmozjs-63a1.so, but after this patch I no longer see it on the command-line (in either make or tup). > > I think for tup we currently don't handle SYMBOLS_FILE at all, and for rules.mk it looks like no flag is added on Linux unless GCC_USE_GNU_LD is set. Hmm, I wonder if we can just always pass SYMBOLS_FILE to lld, it doesn't complain. We handle SYMBOLS_FILE in the tup backend here: https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/python/mozbuild/mozbuild/backend/tup.py#380 I see it added when I try this locally, but you're right that we're not consistent with the make backend about being picky about gnu ld.
Assignee | ||
Comment 10•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8996146 [details] Bug 1478499 - Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. https://reviewboard.mozilla.org/r/260374/#review267640 > Huh, yeah, it looks like it's expecting to see the `mozjs_$version` form since it replaces dashes with underscores in the make version, but it does always see "js". According to the bug that added the symbols files (bug 809430) we were intending to have a version number here, but either this never quite worked as expected or it regressed at some point and nobody noticed.
Assignee | ||
Comment 11•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8996146 [details] Bug 1478499 - Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. https://reviewboard.mozilla.org/r/260374/#review267640 > Hmm, I wonder if we can just always pass SYMBOLS_FILE to lld, it doesn't complain. We handle SYMBOLS_FILE in the tup backend here: https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/python/mozbuild/mozbuild/backend/tup.py#380 I see it added when I try this locally, but you're right that we're not consistent with the make backend about being picky about gnu ld. Oh, I didn't notice this before, but for some reason GCC_USE_GNU_LD is set when we use lld, just not under js/src, because we don't AC_SUBST it in js/src/old-configure.in. I'll add this to a "pre" patch.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8996146 [details] Bug 1478499 - Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. https://reviewboard.mozilla.org/r/260374/#review267812
Attachment #8996146 -
Flags: review+
Updated•6 years ago
|
Attachment #8996146 -
Flags: review?(core-build-config-reviews)
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8996483 [details] Bug 1478499 - Move symbol version script generation for js shared library to moz.build. https://reviewboard.mozilla.org/r/260554/#review267814
Attachment #8996483 -
Flags: review?(mshal) → review+
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8996484 [details] Bug 1478499 - Specify the symbol version script for js with SYMBOLS_FILE. https://reviewboard.mozilla.org/r/260556/#review267816
Attachment #8996484 -
Flags: review?(mshal) → review+
Comment 21•6 years ago
|
||
mozreview-review |
Comment on attachment 8996485 [details] Bug 1478499 - Consider GCC_USE_GNU_LD when passing a symbols file in the Tup build for consistency with the Make build. https://reviewboard.mozilla.org/r/260558/#review267818
Attachment #8996485 -
Flags: review?(mshal) → review+
Comment 22•6 years ago
|
||
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3f72358d338a Do not list pixman.h twice in system-headers.h r=froydnj https://hg.mozilla.org/integration/autoland/rev/237dc169f486 Skip additional js shell installation via ObjdirFiles in the tup build. r=mshal https://hg.mozilla.org/integration/autoland/rev/03f68a47b89e Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. r=mshal https://hg.mozilla.org/integration/autoland/rev/6f6e562e2013 Move symbol version script generation for js shared library to moz.build. r=mshal https://hg.mozilla.org/integration/autoland/rev/3982c14a834e Specify the symbol version script for js with SYMBOLS_FILE. r=mshal https://hg.mozilla.org/integration/autoland/rev/9dee6a08a2dd Consider GCC_USE_GNU_LD when passing a symbols file in the Tup build for consistency with the Make build. r=mshal
Comment 23•6 years ago
|
||
Backed out for failing spidermonkey pkg Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9dee6a08a2ddd5156d5b25cd7a8732deaaf76aed Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=191415100&repo=autoland&lineNumber=58668 Backout: https://hg.mozilla.org/integration/autoland/rev/5de91845f8e775926d8488ae76c38cee5043fa2b
Flags: needinfo?(cmanchester)
Assignee | ||
Comment 24•6 years ago
|
||
Moving gen_symverscript.py under build/ should fix this, but I'll test on try.
Flags: needinfo?(cmanchester)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 31•6 years ago
|
||
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b798a2902a46 Do not list pixman.h twice in system-headers.h r=froydnj https://hg.mozilla.org/integration/autoland/rev/97e20fbd0365 Skip additional js shell installation via ObjdirFiles in the tup build. r=mshal https://hg.mozilla.org/integration/autoland/rev/844b006cc3e9 Use JS_LIBRARY_NAME rather than LIBRARY_NAME when versioning js symbols. r=mshal https://hg.mozilla.org/integration/autoland/rev/02c279671108 Move symbol version script generation for js shared library to moz.build. r=mshal https://hg.mozilla.org/integration/autoland/rev/e815b69d6941 Specify the symbol version script for js with SYMBOLS_FILE. r=mshal https://hg.mozilla.org/integration/autoland/rev/6d4cbc671448 Consider GCC_USE_GNU_LD when passing a symbols file in the Tup build for consistency with the Make build. r=mshal
Comment 32•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b798a2902a46 https://hg.mozilla.org/mozilla-central/rev/97e20fbd0365 https://hg.mozilla.org/mozilla-central/rev/844b006cc3e9 https://hg.mozilla.org/mozilla-central/rev/02c279671108 https://hg.mozilla.org/mozilla-central/rev/e815b69d6941 https://hg.mozilla.org/mozilla-central/rev/6d4cbc671448
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•