Closed
Bug 975011
Opened 10 years ago
Closed 10 years ago
configure, build nsprpub for js/src standalone builds
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla33
People
(Reporter: sfink, Assigned: sfink)
References
(Depends on 1 open bug)
Details
Attachments
(3 files, 8 obsolete files)
6.07 KB,
text/plain
|
Details | |
25.65 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
28.79 KB,
patch
|
Details | Diff | Splinter Review |
I have the 1.9.0pre MozillaBuild installed. When I try to build nspr via configure; make, it fails when it runs make export from within objdir/nspr/pr/include/md. The problem is that when mozmake or pymake run the command nsinstall -m 444 .../_win95.cfg objdir/dist/include/nspr it silently (?!) fails. Here's the actual log output: nsinstall -m 444 ../../../../.././nsprpub/pr/include/md/_win95.cfg /c/Users/sfink/src/MI-GC/objdir/dist/include/nspr mv -f /c/Users/sfink/src/MI-GC/objdir/dist/include/nspr/_win95.cfg /c/Users/sfink/src/MI-GC/objdir/dist/include/nspr/prcpucfg.h mv: cannot stat `/c/Users/sfink/src/MI-GC/objdir/dist/include/nspr/_win95.cfg': No such file or directory I am suspecting something with LF vs CRLF, because if I change the Makefile - $(INSTALL) -m 444 $(srcdir)/$(MDCPUCFG_H) $(dist_includedir) + sh -c '$(INSTALL) -m 444 $(srcdir)/$(MDCPUCFG_H) $(dist_includedir)' then it works. So does - $(INSTALL) -m 444 $(srcdir)/$(MDCPUCFG_H) $(dist_includedir) + $(INSTALL) -m 444 $(srcdir)/$(MDCPUCFG_H) $(dist_includedir) # (adding a comment character at the end.) Also, the log of the full script output starts out with LF line endings, but switches to CRLF as soon as it does the make for nspr. Though that doesn't really prove anything.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
glandium recommended calling NSPR configure + build from js/src. This patch works well enough to get me a Windows build. I haven't checked it out anywhere else yet, but here's a try push: https://tbpl.mozilla.org/?tree=Try&rev=0761c986dade
Attachment #8379379 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8379379 [details] [diff] [review] imported patch nspr-config Doing too much tweaking to get this to work, so cancelling review for now.
Attachment #8379379 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 4•10 years ago
|
||
Ok, got the darn thing working. Note that there's some strange order dependence between, I think, the libffi subconfigure and the nspr configure on b2g builds. (Specifically, it seems like there's a -std=c99 that needs to be set in order for nspr to find pthread.h.) https://tbpl.mozilla.org/?tree=Try&rev=7af560a044ee
Attachment #8381820 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8379379 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Summary: nsinstall fails when run by mozmake or pymake, fine if run by msys make → configure, build nsprpub for js/src standalone builds
Comment 6•10 years ago
|
||
Comment on attachment 8381820 [details] [diff] [review] patch Review of attachment 8381820 [details] [diff] [review]: ----------------------------------------------------------------- ::: aclocal.m4 @@ +10,5 @@ > builtin(include, build/autoconf/toolchain.m4)dnl > builtin(include, build/autoconf/ccache.m4)dnl > builtin(include, build/autoconf/wrapper.m4)dnl > builtin(include, build/autoconf/nspr.m4)dnl > +builtin(include, build/autoconf/nsprpub.m4)dnl make that build-nspr.m4 or nspr-build.m4 ::: build/autoconf/nsprpub.m4 @@ +29,5 @@ > + See --with-nspr-cflags for more details.], > + NSPR_LIBS=$withval) > + > +MOZ_ARG_ENABLE_BOOL(nsprpub, > +[ --enable-nsprpub Configure and build NSPR from source tree], --enable-nspr-build? --enable-nspr? @@ +31,5 @@ > + > +MOZ_ARG_ENABLE_BOOL(nsprpub, > +[ --enable-nsprpub Configure and build NSPR from source tree], > + MOZ_USE_NSPRPUB=1, > + MOZ_USE_NSPRPUB=) I'd rather not expose this from top-level configure. Please wrap with something like: ifelse([$1],:,MOZ_ARG_ENABLE_BOOL(...)) and invoke MOZ_CONFIG_NSPR with a dummy argument in js/src/configure.in. (and double check that --enable-nsprpub doesn't appear in configure --help for top-level but does appear for js/src/configure --help. @@ +37,5 @@ > +if test -z "$BUILDING_JS" || test -n "$JS_STANDALONE"; then > + _IS_OUTER_CONFIGURE=1 > +fi > + > +if test -n "$_IS_OUTER_CONFIGURE"; then This test unfortunately doesn't do what you think it does. MOZ_ARG_WITH_BOOL is at the m4 level, and part of it expands to a completely unrelated place, unwrapped by that if. You need the m4 ifelse like above, in which case you need _IS_OUTER_CONFIGURE defined at the m4 level, which ends up being complicated. The same $1 trick as above would do it, but i'm not sure why you want --with-system-nspr in one configure and not the other. @@ +49,5 @@ > + _USE_SYSTEM_NSPR=1 ) > +fi > + > +JS_POSIX_NSPR=unset > +if test -n "$JS_STANDALONE"; then That should be dependent on BUILDING_JS, but you hit the same issue as above, so ifelse([$1],:,...) again @@ +87,5 @@ > + nspr_opts="x$nspr_opts" > + which_nspr="source-tree" > +fi > +if test "$JS_POSIX_NSPR" = unset; then > + : "can we use negation yet?" yes @@ +101,5 @@ > + > +if test -z "$nspr_opts"; then > + if test -z "$BUILDING_JS"; then > + dnl Toplevel configure defaults to using nsprpub from the source tree > + MOZ_USE_NSPRPUB=1 MOZ_BUILD_NSPR @@ +107,5 @@ > + dnl JS configure defaults to emulated NSPR if available, falling back to nsprpub > + JS_POSIX_NSPR="$JS_POSIX_NSPR_DEFAULT" > + if test -z "$JS_POSIX_NSPR"; then > + MOZ_USE_NSPRPUB=1 > + fi That's what we can remove when all automation switches to building nspr through the build system, right? @@ +130,5 @@ > +if test -n "$_USE_SYSTEM_NSPR" -o -n "$NSPR_CFLAGS" -o -n "$NSPR_LIBS"; then > + AM_PATH_NSPR($NSPR_MINVER, [MOZ_NATIVE_NSPR=1], [AC_MSG_ERROR([you do not have NSPR installed or your version is older than $NSPR_MINVER.])]) > +fi > + > +if test -n "$MOZ_NATIVE_NSPR" && test -z "$JS_POSIX_NSPR"; then You can't have both MOZ_NATIVE_NSPR and JS_POSIX_NSPR at the same time per the mutual exclusion test above. @@ +137,5 @@ > + AC_TRY_COMPILE([#include "prtypes.h"], > + [#ifndef PR_STATIC_ASSERT > + #error PR_STATIC_ASSERT not defined or requires including prtypes.h > + #endif], > + [MOZ_NATIVE_NSPR=1], I know this is how it is currently, but you might as well remove this useless assignment, since MOZ_NATIVE_NSPR is already 1 when you reach here.
Attachment #8381820 -
Flags: review?(mh+mozilla) → feedback+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6) > @@ +37,5 @@ > > +if test -z "$BUILDING_JS" || test -n "$JS_STANDALONE"; then > > + _IS_OUTER_CONFIGURE=1 > > +fi > > + > > +if test -n "$_IS_OUTER_CONFIGURE"; then > > This test unfortunately doesn't do what you think it does. MOZ_ARG_WITH_BOOL > is at the m4 level, and part of it expands to a completely unrelated place, > unwrapped by that if. You need the m4 ifelse like above, in which case you > need _IS_OUTER_CONFIGURE defined at the m4 level, which ends up being > complicated. Y'know, this bothered me when I wrote it, and I remember thinking "how could this possibly work?" but then forgot about it when my tests passed. > The same $1 trick as above would do it, but i'm not sure why > you want --with-system-nspr in one configure and not the other. Er... yeah, that doesn't make sense. It wasn't intentional. > @@ +107,5 @@ > > + dnl JS configure defaults to emulated NSPR if available, falling back to nsprpub > > + JS_POSIX_NSPR="$JS_POSIX_NSPR_DEFAULT" > > + if test -z "$JS_POSIX_NSPR"; then > > + MOZ_USE_NSPRPUB=1 > > + fi > > That's what we can remove when all automation switches to building nspr > through the build system, right? No. JS is working on removing the use of NSPR entirely. The POSIX "emulation" layer is already the default on linux and osx for standalone JS builds. A contributor has a non-POSIX replacement for NSPR on Windows that more or less works, but hasn't landed. I don't know what the plans are when building JS as part of Gecko; I think for the time being the intent is to continue using NSPR. When all automation switches to building nspr through the build system, we could maybe remove --with-nspr-libs and --with-nspr-cflags? > @@ +87,5 @@ > > + nspr_opts="x$nspr_opts" > > + which_nspr="source-tree" > > +fi > > +if test "$JS_POSIX_NSPR" = unset; then > > + : "can we use negation yet?" > > yes If negation is allowed, what about ${var+x} ? I could maybe simplify the mutual exclusion stuff with nspr_opts="${_USE_SYSTEM_NSPR+x}${MOZ_BUILD_NSPR+x}...". Though I guess the which_nspr thing is kind of handy, so I probably wouldn't anyway.
Attachment #8385709 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8381820 -
Attachment is obsolete: true
Comment 8•10 years ago
|
||
Comment on attachment 8385709 [details] [diff] [review] patch Review of attachment 8385709 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/autoconf/nspr-build.m4 @@ +1,5 @@ > +dnl This Source Code Form is subject to the terms of the Mozilla Public > +dnl License, v. 2.0. If a copy of the MPL was not distributed with this > +dnl file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +ifelse([$1],,define(CONFIGURING_JS,yes)) This is outside any AC_DEFUN, so this is not going to work properly :) @@ +59,5 @@ > + JS_POSIX_NSPR_DEFAULT=1 > + fi > + ;; > + esac > +fi this block can probably move under ifdef([CONFIGURING_JS] @@ +62,5 @@ > + esac > +fi > + > +ifdef([CONFIGURING_JS],[ > +MOZ_ARG_ENABLE_BOOL(posix-nspr-emulation, Please indent
Attachment #8385709 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #8) > Comment on attachment 8385709 [details] [diff] [review] > patch > > Review of attachment 8385709 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: build/autoconf/nspr-build.m4 > @@ +1,5 @@ > > +dnl This Source Code Form is subject to the terms of the Mozilla Public > > +dnl License, v. 2.0. If a copy of the MPL was not distributed with this > > +dnl file, You can obtain one at http://mozilla.org/MPL/2.0/. > > + > > +ifelse([$1],,define(CONFIGURING_JS,yes)) > > This is outside any AC_DEFUN, so this is not going to work properly :) Ah, but that bug is countered by me using the wrong number of commas -- that line compares $1 to the empty string, and does the define if they're equal when I meant unequal. Except *that* bug is countered by the lack of quoting around the define, so it happens unconditionally anyway. :-) Dammit, I did test this. But then I totally changed it. And didn't re-test. (I was trying to mess with the configure.in files to define some m4 macros appropriately so I could avoid the MOZ_CONFIG_NSPR(hack) thing.)
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cff27cb2845
Comment 11•10 years ago
|
||
This is excellent, worth sending a message to the internals list about this flag I think.
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7cff27cb2845
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5ca7959511
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 14•10 years ago
|
||
Backout made it to mozilla-central: https://hg.mozilla.org/mozilla-central/rev/ff5ca7959511 I've added an ignore range for this (from rev 7cff27cb2845 to rev ff5ca7959511) in the fuzzing harness, as well as restoring the original compilation scripts.
Assignee | ||
Comment 16•10 years ago
|
||
Uploading current version of patch. It's definitely not right yet -- see https://tbpl.mozilla.org/?tree=Try&rev=17adcc46d3f7 But I think it's a lot closer than it was. I will request testing when I at least get try green again.
Assignee | ||
Comment 17•10 years ago
|
||
Rebased, since soubok wants to try it.
Assignee | ||
Updated•10 years ago
|
Attachment #8404995 -
Attachment is obsolete: true
Assignee | ||
Comment 18•10 years ago
|
||
Ok, this one fixes the b2g problem. I know of no further problems, so please give this one a spin.
Attachment #8405718 -
Flags: review?(mh+mozilla)
Attachment #8405718 -
Flags: feedback?(gary)
Attachment #8405718 -
Flags: feedback?(choller)
Assignee | ||
Updated•10 years ago
|
Attachment #8405485 -
Attachment is obsolete: true
Comment 19•10 years ago
|
||
Comment on attachment 8405718 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure Compiling with --enable-threadsafe fails on Windows if the objdir is not in the repository. Log coming up. (backslashes/forward slashes in the command didn't matter)
Attachment #8405718 -
Flags: feedback?(gary) → feedback-
Comment 20•10 years ago
|
||
Steps: 1. Run `sh autoconf-2.13` in js/src. 2. Make a new objdir, then cd into it. 3. Run the configure command at the top of the log. 4. Run `mozmake -j1`. I test with the objdir in various positions, either in the repository under js/src or on ~/Desktop.
Assignee | ||
Comment 21•10 years ago
|
||
You're invoking the configure script with backslashes in the path. Does it work if you use forward slashes?
Flags: needinfo?(gary)
Comment 22•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #19) > (backslashes/forward slashes in the command didn't matter) Nope, as I mentioned in comment 19, I also tried the "other" slash and it didn't work either.
Flags: needinfo?(gary) → needinfo?(sphink)
Comment 23•10 years ago
|
||
Comment on attachment 8405718 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure I tried building with --enable-threadsafe as well as --disable-threadsafe and both seemed to be working with 32 and 64 bit builds :)
Attachment #8405718 -
Flags: feedback?(choller) → feedback+
Comment 24•10 years ago
|
||
Comment on attachment 8405718 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure Review of attachment 8405718 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/autoconf/nspr-build.m4 @@ +1,5 @@ > +dnl This Source Code Form is subject to the terms of the Mozilla Public > +dnl License, v. 2.0. If a copy of the MPL was not distributed with this > +dnl file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +ifelse([$1],,define(CONFIGURING_JS,yes)) This is still outside any AC_DEFUN. That can't work. @@ +63,5 @@ > + JS_POSIX_NSPR_DEFAULT=1 > + fi > + ;; > + esac > +fi this block can still move under ifdef([CONFIGURING_JS] @@ +66,5 @@ > + esac > +fi > + > +ifdef([CONFIGURING_JS],[ > +MOZ_ARG_ENABLE_BOOL(posix-nspr-emulation, Please indent. @@ +138,5 @@ > + fi > + AC_SUBST(JS_POSIX_NSPR) > +fi > + > +# A (sub)configure invoked by the toplevel configure will always receive either there's no "or" to that "either".
Attachment #8405718 -
Flags: review?(mh+mozilla) → feedback+
Assignee | ||
Comment 25•10 years ago
|
||
Incorporate glandium's suggestions. Haven't gone back to try to figure out gkw's problem yet.
Assignee | ||
Updated•10 years ago
|
Attachment #8405718 -
Attachment is obsolete: true
Comment 26•10 years ago
|
||
(In reply to Steve Fink [:sfink] from comment #25) > Created attachment 8440873 [details] [diff] [review] > allow js/src/configure to invoke the in-tree NSPR configure > > Incorporate glandium's suggestions. Haven't gone back to try to figure out > gkw's problem yet. works for me with the following configuration: --enable-debug --disable-optimize --enable-gczeal --enable-threadsafe --disable-gcgenerational --enable-exact-rooting --without-intl-api --enable-ctypes --with-windows-version=601 --disable-static --disable-profile-guided-optimization --disable-vista-sdk-requirements --disable-tests
Comment 27•10 years ago
|
||
Comment on attachment 8440873 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure This patch seems to work fine with my setup. \o/
Attachment #8440873 -
Flags: feedback+
Updated•10 years ago
|
Attachment #8405906 -
Attachment is obsolete: true
Assignee | ||
Comment 28•10 years ago
|
||
Comment on attachment 8440873 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure Review of attachment 8440873 [details] [diff] [review]: ----------------------------------------------------------------- Ok, this works for Gary, and works on try: https://tbpl.mozilla.org/?tree=Try&rev=80f1dfe9f706
Attachment #8440873 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(sphink)
Updated•10 years ago
|
Attachment #8440873 -
Flags: review?(mh+mozilla) → review+
Comment 29•10 years ago
|
||
Helped to land this since there was a green try run: https://hg.mozilla.org/integration/mozilla-inbound/rev/9d86b9442f0b (additionally, landing early for me to verify that this works properly on my side again)
Target Milestone: mozilla30 → mozilla33
Comment 30•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9d86b9442f0b
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 31•10 years ago
|
||
Comment on attachment 8440873 [details] [diff] [review] allow js/src/configure to invoke the in-tree NSPR configure Review of attachment 8440873 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +9140,3 @@ > if test -n "$MOZ_NO_DEBUG_RTL"; then > ac_configure_args="$ac_configure_args --disable-debug-rtl" > fi Gah, I should I seen this. This needs to move.
Comment 32•10 years ago
|
||
Backed out for causing bug 1027149. https://hg.mozilla.org/mozilla-central/rev/5a29f18ca2a0
Target Milestone: mozilla33 → ---
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Flags: needinfo?(sphink)
Resolution: FIXED → ---
Comment 33•10 years ago
|
||
Ok. This was backed out a month ago and is blocking work by Gary Kwong. Who can we get to work on this in order to get this checked in again. Needinfo? has been set for a month now.
Flags: needinfo?(jdemooij)
Comment 34•10 years ago
|
||
This is the patch that landed last time, rebased on top of what I'm about to land, and with comment 31 addressed. Please test it.
Updated•10 years ago
|
Attachment #8440873 -
Attachment is obsolete: true
Updated•10 years ago
|
Assignee: sphink → mh+mozilla
Status: REOPENED → ASSIGNED
Updated•10 years ago
|
Assignee: mh+mozilla → sphink
Comment 35•10 years ago
|
||
Comment on attachment 8457677 [details] [diff] [review] Allow js/src/configure to invoke the in-tree NSPR configure fwiw, I tested this on top of m-i rev 63c52b7ddc28 and it could still work with the fuzzing harness. feedback+
Attachment #8457677 -
Flags: feedback+
Comment 36•10 years ago
|
||
s/HAVE_64BIT_OS/HAVE_64BIT_BUILD/
Updated•10 years ago
|
Attachment #8457677 -
Attachment is obsolete: true
Assignee | ||
Comment 37•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e262e1a6f810 is a try push of the latest version. I also checked out the arewefastyet repo, since I know it broke last time we pushed this, and it looks like now it is prepared for this patch. (I still don't know why it broke in the first place, and h4writer's on PTO.) I'm paranoid, so I'll wait for the try build to complete before pushing this.
Flags: needinfo?(sphink)
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 38•10 years ago
|
||
The try build still isn't totally done, but I gave it 15 hours, so: https://hg.mozilla.org/integration/mozilla-inbound/rev/a459b02a9ca4
Comment 39•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a459b02a9ca4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•