Closed Bug 622989 Opened 14 years ago Closed 8 years ago

Shell doesn't build on Windows with NSPR

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 975011

People

(Reporter: paul.biggar, Unassigned)

References

Details

Or maybe it does, and we just haven't documented the correct incantation.
I tried this, and I eventually got it to work, but it was fairly crappy--there's got to be a better way. Here are the problems I ran into and what I did:

1. Setting the --with-nspr options. This isn't terribly well documented. We should give an example, or a list of different ways to do it, rather than merely an explanation of each individual option.

2. nspr-config doesn't work on windows. I figured out that I am supposed to be able to use:

   --with-nspr-cflags=`nspr-config --cflags` \
   --with-nspr-libs=`nspr-config --libs`

The cflags part works, but the libs part returns '-LC:/path_to_libs -lnspr4 -lplc4 -lplds4'. None of these options is valid with MSVC 'link', so the link step fails.

3. Making the obvious fix to the libs option also doesn't work. From the previous step, I tried fixing the options to what 'link' accepts, which means:

  --with-nspr-libs='-LIBPATH:/path_to_libs libnspr4.lib libplc4.lib libplds4.lib'

This doesn't work because the lib files get added as deps to mozjs.dll (which can also be seen with make -p), so 'make' says it can't find a rule to build those needed deps and stops.

I found that this works:

  --with-nspr-libs='path_to_libs/libnspr4.lib path_to_libs/libplc4.lib path_to_libs/libplds4.lib'

That seems like a lot of junk to have to pass to 'configure', though. But at this point I can build the JS shell.

4. The resulting shell doesn't run because the DLLs aren't on the path. This is easy enough to fix but we at least need docs here.
Summarizing the previous, I think what we want is:

- docs on how to build NSPR on Windows and install it in a way that makes the rest of JS shell building easy, and get the DLLs on the path.
- set up configure to be able to use said install with just one configure option and build all the way through. It might even be nice to have NSPR installed in the source tree and then let configure pick that up by default.
- it would also be nice to be able to conveniently use a debug NSPR build with debug JS builds and opt with opt.
What about providing something-like nano-NSPR on Windows that would implement the functionality JS uses?
(In reply to comment #3)
> What about providing something-like nano-NSPR on Windows that would implement
> the functionality JS uses?

Or just put NSPR into the tree like we do with readline, nanojit, etc?
I suggest building NSPR from source code on Windows.
You can either add NSPR to your source tree, or require
that the NSPR source tree be checked out to a fixed
location relative to your source tree.
(In reply to comment #2)
> 4. The resulting shell doesn't run because the DLLs aren't on the path. This is
> easy enough to fix but we at least need docs here.

What's the fix to this?
(In reply to comment #6)
> (In reply to comment #2)
> > 4. The resulting shell doesn't run because the DLLs aren't on the path. This is
> > easy enough to fix but we at least need docs here.
> 
> What's the fix to this?

Well, it depends if we are just using one copy of the DLLs for everything, or separate debug and opt DLLs. If one, then they can just go on the path, say the mozilla-build /local/bin. Otherwise, they probably need to get copied to the dir where js.exe lives. We might want the build to do that.
Can we link statically to NSPR, for the shell?  That would make a lot of people happier, I think.
I've updated a bunch of documentation pages to make this process easier.

On https://developer.mozilla.org/En/SpiderMonkey/Build_Documentation#section_6 I've documented how to build on Windows with NSPR. I've also removed some old information and rearranged things so that it's clearer how to build with NSPR, and to explain what the --with-nspr-cflags and --with-nspr-libs options do.

On https://developer.mozilla.org/en/NSPR#section_1, I've included a list of how to install NSPR on various platforms.

On https://developer.mozilla.org/en/NSPR_build_instructions I've updated the build documentation to use M-C as the repo, not the old CVS repository.

On https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_THREADSAFE, I've removed the build information (moved to SpiderMonkey/Build_Documentation), and made it clear that JS_THREADSAFE is no longer optional.


I believe we now have an easy-to-follow guide to installing SpiderMonkey with --enable-threadsafe on. If you have any comments, grammar nits, etc, please feel free to fix them, or raise them here and I'll fix them.
Those docs look pretty good. I'd like to test the directions before we go ahead with this.

The other question is whether we want to land a change that causes some developer disruption this close to Fx4. Thoughts?
(In reply to comment #10)
> The other question is whether we want to land a change that causes some
> developer disruption this close to Fx4. Thoughts?

Personally, I think it's very important that we _do_ land this change. It seems that many of our developers develop without JS_THREADSAFE, which affects their perception of their changes to FF4. This was OK over the last few months: small changes to performance, memory usage or behaviour, could be fixed in the long run to FF4. But with so little time left, it's imperative that we're fixing bugs in the right places and measuring the right things.

Perhaps a happy medium is to give them a heads-up in advance. Let them check that it works at their leisure, rather than forcing a context switch while they're working on something else.
(In reply to comment #9)
> On https://developer.mozilla.org/en/NSPR_build_instructions I've updated the
> build documentation to use M-C as the repo, not the old CVS repository.

Paul, thank you for improving NSPR documentation!

Unfortunately I had to revert your changes to this page.
Your changes had two problems:
1. Some existing sections, such as "Configure options",
were duplicated.  (You noticed and deleted the duplicate
"Prerequistes" section.)
2. More important, the change to use M-C as the repo
is wrong.  The master NSPR source repository is still
cvs.mozilla.org.

Changing 'gmake' to 'make' is fine, but it requires a
note for Unix platforms where 'make' is the original
Unix make.
OS: Mac OS X → All
OS: All → Mac OS X
(In reply to comment #12)
> > On https://developer.mozilla.org/en/NSPR_build_instructions I've updated the
> > build documentation to use M-C as the repo, not the old CVS repository.
> 
> Unfortunately I had to revert your changes to this page.

Thanks for backing it out. I didn't know NSPR was still developed in CVS, my bad; I thought that page had just bit-rotted. In that case, I don't have any changes to make to that page.
dmandelin: ping?
(In reply to comment #14)
> dmandelin: ping?

Well, in my opinion, this is not a blocker, and has nonzero risk of disrupting and delaying blocker work, so it shouldn't land now. It seems like the beginning of a cycle is the perfect time to land a thing like this.
dmandelin: post-FF4 ping?
(In reply to comment #16)
> dmandelin: post-FF4 ping?

What are your current plans? Are we going to get nspr-config working on Windows first?
I was waiting for bug 625895 - which would have made all this trivial - to land, but it's been rejected.

jimb is working on fixing nspr-config (bug 636615), though I'm not sure they'll accept a patch which changes the output based on the compiler with which nspr is compiled.

However, even with this fix Windows users will still need to specify a PATH, so they'll still need to read the NSPR on Windows docs, so it doesn't seem like we gain much from waiting.
I got back to trying this. It doesn't seem to quite work right now:

1. The MDC instructions [1] use the wrong shell quoting for the \s in Windows paths, I think. (The lib names might be wrong too, but I haven't gotten that far yet.)

2. I get compile errors:

jsanalyze.cpp
c:\sources\t2\js\src\jscntxt.h(2685) : error C2664: '_InterlockedIncrement' : cannot convert parameter 1 from 'PRInt32 *' to 'volatile long *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
c:\sources\t2\js\src\jscntxt.h(2687) : error C2664: '_InterlockedDecrement' : cannot convert parameter 1 from 'PRInt32 *' to 'volatile long *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
c:\sources\t2\js\src\jscntxt.h(3110) : error C2664: '_InterlockedExchange' : cannot convert parameter 1 from 'PRInt32 *' to 'volatile long *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
c:\sources\t2\js\src\jscntxt.h(3115) : error C2664: '_InterlockedIncrement' : cannot convert parameter 1 from 'PRInt32 *' to 'volatile long *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
make[1]: *** [jsanalyze.obj] Error 2

I also note that the configure line for Windows this way is yuckily long. I understand that the bugs that will fix that either were nixed or are still being worked on. So that's OK. Copy-pasting the config line from the MDC docs, so that's pretty good. Maybe we should provide a reference shell script that does a 1-step build, maybe even with options for debug or opt builds.

[1] https://developer.mozilla.org/En/SpiderMonkey/Build_Documentation#NSPR_on_Windows
Assignee: general → nobody
This works now, and we have it in continuous integration (eg the SM(p) build) so it should keep working.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.