Fix build for win32, remove --enable-gczeal that snuck back in #71

vvuk · 2016-01-06T19:18:49Z

For some reason, gcc isn't generating public symbols for setTracingLocation if they're defined inline in the class declaration in the header. There's probably an attribute we could apply or something, but this was the simplest way to get this to work.

vvuk · 2016-01-06T19:20:34Z

Someone should test this on linux/osx with the removing of --enable-gczeal, but without the second patch -- we should see if this is a windows-specific gcc issue, or if we're doing something we shouldn't be.

jdm · 2016-01-06T19:40:54Z

I'd like @michaelwu to take a look at the second commit.

vvuk · 2016-01-07T17:52:49Z

I think the original linking problem may be due to -fno-keep-inline-dllexport that is added to the gcc flags in configure... but haven't tested without, would rather get static linking working properly.

asajeffrey · 2016-01-10T14:47:42Z

Should this be two separate PRs? One to remove --enable-gc-zeal and one to fix building on windows?

vvuk · 2016-01-10T16:35:13Z

Sure? Note that I'm working through a bunch of this stuff in

https://bugzilla.mozilla.org/show_bug.cgi?id=1238203
https://bugzilla.mozilla.org/show_bug.cgi?id=1238023
https://bugzilla.mozilla.org/show_bug.cgi?id=1237761

so that we get it all in upstream.

michaelwu · 2016-01-13T19:41:03Z

An alternative to moving setTracingLocation and other functions is to remove those from the bindings - I don't think we need those functions, and they appear to be gone from upstream SM anyway. I'm fine with either solution though, since these functions are going to disappear in the future.

larsbergstrom · 2016-01-13T20:40:00Z

Currently failing on linux with:

In file included from /home/larsberg/servo/target/debug/build/mozjs_sys-6e42204b6e19049d/out/js/src/Unified_cpp_js_src7.cpp:110:0:
/home/larsberg/mozjs/mozjs/js/src/jsapi.cpp: In function ‘bool JS_CallOnce(JSCallOnceType*, JSInitCallback)’:
/home/larsberg/mozjs/mozjs/js/src/jsapi.cpp:5830:80: error: ‘PR_CallOnceWithArg’ was not declared in this scope
     return PR_CallOnceWithArg(once, CallOnce, JS_FUNC_TO_DATA_PTR(void *, func)) == PR_SUCCESS;
                                                                                ^
make[4]: *** [Unified_cpp_js_src7.o] Error 1
make[3]: *** [js/src/target] Error 2
make[2]: *** [compile] Error 2
make[1]: *** [default] Error 2
make: *** [all] Error 2
thread '<main>' panicked at 'assertion failed: result.success()', build.rs:17

larsbergstrom · 2016-01-13T20:40:40Z

@vvuk You may already know, but if not, here's a note on the hackery so you don't need to allocate/delete critical sections - http://joeduffyblog.com/2006/11/28/windows-keyed-events-critical-sections-and-new-vista-synchronization-features/

vvuk · 2016-01-13T20:46:36Z

Arg, PR_CallOnceWithArg went away in m-c sm, but I guess not here. I'll fix shortly. Same with just removing those methods from the tracer.

As for critical sections... I had no idea about SRWLs. Since we're already depending on >= Vista, I'll switch to using them.


        Remove --enable-gczeal that snuck back in


        Use static linking; don't link with system NSPR


        Add WinNSPR.cpp implementation


        Build mozglue as a static library when building JS_STANDALONE


        Add way to skip MOZ_EXPORT stuff in mfbt


        configure regen

larsbergstrom · 2016-01-16T00:18:00Z

It looks like this is passing CI builds now. @vvuk Is this ready to be reviewed/landed?

larsbergstrom · 2016-01-19T21:56:03Z

@vvuk and @michaelwu Is there anything more that needs to be done here to land this? I think it's the last piece we need before we land the Win32 changes to Servo master.

michaelwu · 2016-01-19T23:19:27Z

I'm happy with the approach here.

larsbergstrom · 2016-01-20T00:40:43Z

@vvuk, @Jayflux and I are now getting missing setTracingLocation and friends with this and the landed rust-mozjs fixes. Is there something we're missing?

vvuk · 2016-01-20T02:11:18Z

Did rust-mozjs changes land, that had the removal of that in the bindings?

vvuk · 2016-01-20T02:11:39Z

servo/rust-mozjs#236

larsbergstrom · 2016-01-20T14:58:46Z

Great, thanks! That did it for me :-)

I'm opening the servo/servo PR, since it can land independently of these changes (which switch to static linking). That will let me start working on the CI/builder stuff.

larsbergstrom · 2016-01-21T15:11:08Z

@bors-servo r=michaelwu

(confirmed on IRC with @jdm and @Ms2ger)

bors-servo · 2016-01-21T15:11:09Z

📌 Commit d221277 has been approved by michaelwu

bors-servo · 2016-01-21T15:11:10Z

⌛ Testing commit d221277 with merge 5ac9991...


        Auto merge of #71 - vvuk:win32, r=michaelwu

Fix build for win32, remove --enable-gczeal that snuck back in For some reason, gcc isn't generating public symbols for setTracingLocation if they're defined inline in the class declaration in the header. There's probably an attribute we could apply or something, but this was the simplest way to get this to work.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/mozjs/71)

bors-servo · 2016-01-21T15:25:35Z

☀️ Test successful - travis


        Auto merge of #9385 - larsbergstrom:win32, r=frewsxcv,pcwalton,jdm,ec…

@frewsxcv

…oal95 Win32 support r? @frewsxcv for python stuff r? @pcwalton for the "remove usage of Gaol" stuff for Win32 r? anybody else for misc cargo.lock updates, etc. This replaces #7878. This works best with servo/mozjs#71, too, to enable static linking, but can be run without (via some PATH hackery). The instructions are here, and will be added to a .md file in the repo once the mozjs changes also land: https://hackpad.com/Servo-on-Windows-C1LPcI2bP25 I'd like to get these changes landed because I've been rebasing them for months, they're otherwise quite stable, and don't affect our other platforms and targets.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9385)


        servo: Merge #9385 - Win32 support (from larsbergstrom:win32); r=frew…

…sxcv,pcwalton,jdm,ecoal95 r? frewsxcv for python stuff r? pcwalton for the "remove usage of Gaol" stuff for Win32 r? anybody else for misc cargo.lock updates, etc. This replaces #7878. This works best with servo/mozjs#71, too, to enable static linking, but can be run without (via some PATH hackery). The instructions are here, and will be added to a .md file in the repo once the mozjs changes also land: https://hackpad.com/Servo-on-Windows-C1LPcI2bP25 I'd like to get these changes landed because I've been rebasing them for months, they're otherwise quite stable, and don't affect our other platforms and targets. Source-Repo: https://github.com/servo/servo Source-Revision: 525e77f64fc65ea2397b4ff3849f5b1f39386698 UltraBlame original commit: e89451f650e7a23ae482833c8f6c658e3ccc0b33


        servo: Merge #9385 - Win32 support (from larsbergstrom:win32); r=frew…

…sxcv,pcwalton,jdm,ecoal95 r? frewsxcv for python stuff r? pcwalton for the "remove usage of Gaol" stuff for Win32 r? anybody else for misc cargo.lock updates, etc. This replaces #7878. This works best with servo/mozjs#71, too, to enable static linking, but can be run without (via some PATH hackery). The instructions are here, and will be added to a .md file in the repo once the mozjs changes also land: https://hackpad.com/Servo-on-Windows-C1LPcI2bP25 I'd like to get these changes landed because I've been rebasing them for months, they're otherwise quite stable, and don't affect our other platforms and targets. Source-Repo: https://github.com/servo/servo Source-Revision: 525e77f64fc65ea2397b4ff3849f5b1f39386698 UltraBlame original commit: e89451f650e7a23ae482833c8f6c658e3ccc0b33


        servo: Merge #9385 - Win32 support (from larsbergstrom:win32); r=frew…

…sxcv,pcwalton,jdm,ecoal95 r? frewsxcv for python stuff r? pcwalton for the "remove usage of Gaol" stuff for Win32 r? anybody else for misc cargo.lock updates, etc. This replaces #7878. This works best with servo/mozjs#71, too, to enable static linking, but can be run without (via some PATH hackery). The instructions are here, and will be added to a .md file in the repo once the mozjs changes also land: https://hackpad.com/Servo-on-Windows-C1LPcI2bP25 I'd like to get these changes landed because I've been rebasing them for months, they're otherwise quite stable, and don't affect our other platforms and targets. Source-Repo: https://github.com/servo/servo Source-Revision: 525e77f64fc65ea2397b4ff3849f5b1f39386698 UltraBlame original commit: e89451f650e7a23ae482833c8f6c658e3ccc0b33

vvuk mentioned this pull request Jan 6, 2016

--enable-gczeal snuck back in, and missing JSTracer::setTracingLocation etc. on win32 #70

Closed

larsbergstrom mentioned this pull request Jan 7, 2016

Get servo working on win32 servo/servo#7878

Closed

vvuk force-pushed the vvuk:win32 branch 4 times, most recently from 177f8c3 to 1a58f44 Jan 7, 2016

vvuk force-pushed the vvuk:win32 branch from 53421c9 to fd63b28 Jan 10, 2016

vvuk added 6 commits Jan 6, 2016

Remove --enable-gczeal that snuck back in

5041b03

Use static linking; don't link with system NSPR

a3ce2b4

Add WinNSPR.cpp implementation

a69717b

Build mozglue as a static library when building JS_STANDALONE

b5088f8

Add way to skip MOZ_EXPORT stuff in mfbt

2b7d8fd

configure regen

Loading status checks…

d221277

vvuk force-pushed the vvuk:win32 branch from fd63b28 to d221277 Jan 14, 2016

larsbergstrom mentioned this pull request Jan 20, 2016

Win32 support servo/servo#9385

Merged

bors-servo merged commit d221277 into servo:master Jan 21, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

servo / mozjs

Fix build for win32, remove --enable-gczeal that snuck back in #71

Fix build for win32, remove --enable-gczeal that snuck back in #71

vvuk commented Jan 6, 2016

vvuk commented Jan 6, 2016

jdm commented Jan 6, 2016

vvuk commented Jan 7, 2016

asajeffrey commented Jan 10, 2016

vvuk commented Jan 10, 2016

michaelwu commented Jan 13, 2016

larsbergstrom commented Jan 13, 2016

larsbergstrom commented Jan 13, 2016

vvuk commented Jan 13, 2016

larsbergstrom commented Jan 16, 2016

larsbergstrom commented Jan 19, 2016

michaelwu commented Jan 19, 2016

larsbergstrom commented Jan 20, 2016

vvuk commented Jan 20, 2016

vvuk commented Jan 20, 2016

larsbergstrom commented Jan 20, 2016

larsbergstrom commented Jan 21, 2016

bors-servo commented Jan 21, 2016

bors-servo commented Jan 21, 2016

bors-servo commented Jan 21, 2016

servo / mozjs

Join GitHub today

Fix build for win32, remove --enable-gczeal that snuck back in #71

Fix build for win32, remove --enable-gczeal that snuck back in #71

Conversation

vvuk commented Jan 6, 2016

vvuk commented Jan 6, 2016

jdm commented Jan 6, 2016

vvuk commented Jan 7, 2016

asajeffrey commented Jan 10, 2016

vvuk commented Jan 10, 2016

michaelwu commented Jan 13, 2016

larsbergstrom commented Jan 13, 2016

larsbergstrom commented Jan 13, 2016

vvuk commented Jan 13, 2016

larsbergstrom commented Jan 16, 2016

larsbergstrom commented Jan 19, 2016

michaelwu commented Jan 19, 2016

larsbergstrom commented Jan 20, 2016

vvuk commented Jan 20, 2016

vvuk commented Jan 20, 2016

larsbergstrom commented Jan 20, 2016

larsbergstrom commented Jan 21, 2016

bors-servo commented Jan 21, 2016

bors-servo commented Jan 21, 2016

bors-servo commented Jan 21, 2016