Closed Bug 981481 Opened 9 years ago Closed 8 years ago

Non-threadsafe js shells need NSPR libraries after landing of bug 975011, but not threadsafe builds

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 975011

People

(Reporter: gkw, Assigned: sfink)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #975011 +++

After bug 975011 landed, I was making required changes to the fuzzing harness to ensure things compile properly, however I discovered something interesting on m-c rev 21f293fc8d34 (which has the patch in bug 975011):

`ls -l` output:
-rwxr-xr-x  1 skywalker  staff  24184708 Mar  9 20:29 js-dbg-64-dm-ts-darwin-21f293fc8d34

$ ./js-dbg-64-dm-ts-darwin-21f293fc8d34
js> getBuildConfiguration().threadsafe
true

So a threadsafe builds do *not* need NSPR libraries to be run anymore.

However, for non-threadsafe builds:

`ls -l` output:
-rwxr-xr-x  1 skywalker  staff  23904616 Mar  9 20:31 js-dbg-64-dm-darwin-21f293fc8d34
-rwxr-xr-x  1 skywalker  staff    338956 Mar  9 20:31 libnspr4.dylib
-rwxr-xr-x  1 skywalker  staff     30160 Mar  9 20:31 libplc4.dylib
-rwxr-xr-x  1 skywalker  staff     27080 Mar  9 20:31 libplds4.dylib

$ ./js-dbg-64-dm-darwin-21f293fc8d34
js> getBuildConfiguration().threadsafe
false

For some reason, NSPR libraries are now required. Steve, is this intended?
Flags: needinfo?(sphink)
No, I attempted to do this in a way that would not break any existing builds. I was hoping my spidermonkey builds would be similar enough to the fuzzing and other builds to be a good litmus test, but it appears I was wrong.

I will look into this now.
Flags: needinfo?(sphink)
Oh, and I misread this bug. This is a working build, but it incorrectly links in NSPR. Yes, that's very very bad, and *definitely* unintentional.
Attached patch bug-981481-no-nspr (obsolete) — Splinter Review
I don't know what try will think of this yet: https://tbpl.mozilla.org/?tree=Try&rev=c47b02448b88

This patch also displays the actual nspr or nspr-ish thing being used instead of just saying "default" if you don't request anything. Which makes it a bigger change, but it's sure handy in figuring out what it's doing.
Assignee: nobody → sphink
Attachment #8388650 - Flags: review?(mh+mozilla)
Attachment #8388650 - Flags: review?(mh+mozilla)
Fixed one problem with it, not requesting review again quite yet.
Attachment #8388650 - Attachment is obsolete: true
Attachment #8388650 - Flags: review?(mh+mozilla)
Comment on attachment 8388793 [details] [diff] [review]
standalone JS --disable-threadsafe builds should not use NSPR

Ok, try is happy. https://tbpl.mozilla.org/?tree=Try&rev=611d04a7b475
Attachment #8388793 - Flags: review?(mh+mozilla)
Comment on attachment 8388793 [details] [diff] [review]
standalone JS --disable-threadsafe builds should not use NSPR

Review of attachment 8388793 [details] [diff] [review]:
-----------------------------------------------------------------

Bug 975011 was backed out.
Attachment #8388793 - Flags: review?(mh+mozilla)
I'm rolling this fix into an updated version of bug 975011.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 975011
Attachment #8388650 - Flags: review?(mh+mozilla)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.