0ad only builds fine with a specific version of mozjs

  • Done
  • quality assurance status badge
Details
3 participants
  • Guillaume Le Vaillant
  • Jesse Gibbons
  • Liliana Marie Prikler
Owner
unassigned
Submitted by
Guillaume Le Vaillant
Severity
normal
G
G
Guillaume Le Vaillant wrote on 31 Dec 2021 10:17
(address . bug-guix@gnu.org)
87y241azi0.fsf@kitej
The 0ad package checks the version of mozjs and throws an error if it is
not exactly the version it expects. This check is done in
"source/scriptinterface/ScriptTypes.h" and it currently requires version
78.6 of mozjs. As Guix has mozjs 78.15 instead, 0ad fails to build.

Patching "ScriptTypes.h" to remove the check and compile with mozjs 78.15
doesn't work, the build phase fails with:

Toggle snippet (49 lines)
../../../source/scriptinterface/ScriptContext.cpp: In member function ‘void ScriptContext::UnRegisterRealm(JS::Realm*)’:
../../../source/scriptinterface/ScriptContext.cpp:146:39: error: cannot convert ‘JS::Zone*’ to ‘JSContext*’
146 | JS::PrepareZoneForGC(js::GetRealmZone(realm));
| ~~~~~~~~~~~~~~~~^~~~~~~
| |
| JS::Zone*
In file included from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/Value.h:25,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/CallArgs.h:74,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/jsapi.h:31,
from ../../../source/scriptinterface/ScriptTypes.h:63,
from ../../../source/scriptinterface/ScriptContext.h:21,
from ../../../source/scriptinterface/ScriptContext.cpp:20:
/gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/GCAPI.h:539:55: note: initializing argument 1 of ‘void JS::PrepareZoneForGC(JSContext*, JS::Zone*)’
539 | extern JS_PUBLIC_API void PrepareZoneForGC(JSContext* cx, Zone* zone);
| ~~~~~~~~~~~^~
In file included from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/TraceKind.h:12,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/jspubtd.h:18,
from ../../../source/scriptinterface/ScriptTypes.h:62,
from ../../../source/scriptinterface/ScriptContext.h:21,
from ../../../source/scriptinterface/ScriptContext.cpp:20:
/gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/TypeDecls.h:55:21: note: class type ‘JS::Zone’ is incomplete
55 | class JS_PUBLIC_API Zone;
| ^~~~
../../../source/scriptinterface/ScriptContext.cpp: In member function ‘void ScriptContext::PrepareZonesForIncrementalGC() const’:
../../../source/scriptinterface/ScriptContext.cpp:264:40: error: cannot convert ‘JS::Zone*’ to ‘JSContext*’
264 | JS::PrepareZoneForGC(js::GetRealmZone(realm));
| ~~~~~~~~~~~~~~~~^~~~~~~
| |
| JS::Zone*
In file included from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/Value.h:25,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/CallArgs.h:74,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/jsapi.h:31,
from ../../../source/scriptinterface/ScriptTypes.h:63,
from ../../../source/scriptinterface/ScriptContext.h:21,
from ../../../source/scriptinterface/ScriptContext.cpp:20:
/gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/GCAPI.h:539:55: note: initializing argument 1 of ‘void JS::PrepareZoneForGC(JSContext*, JS::Zone*)’
539 | extern JS_PUBLIC_API void PrepareZoneForGC(JSContext* cx, Zone* zone);
| ~~~~~~~~~~~^~
In file included from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/TraceKind.h:12,
from /gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/jspubtd.h:18,
from ../../../source/scriptinterface/ScriptTypes.h:62,
from ../../../source/scriptinterface/ScriptContext.h:21,
from ../../../source/scriptinterface/ScriptContext.cpp:20:
/gnu/store/gzsa3jrlhgcr3mr6i170lhgfsxsmpcps-mozjs-78.15.0/include/mozjs-78/js/TypeDecls.h:55:21: note: class type ‘JS::Zone’ is incomplete
55 | class JS_PUBLIC_API Zone;
| ^~~~
make[1]: *** [scriptinterface.make:146: obj/scriptinterface_Release/ScriptContext.o] Error 1

What would be the best way to fix this?
- keep a mozjs-78.6 package around just for 0ad
- patch 0ad to fix the compatibility issues with mozjs 78.15
- use the mozjs version bundled in the 0ad sources

WDYT?
-----BEGIN PGP SIGNATURE-----

iIUEAREKAC0WIQTLxZxm7Ce5cXlAaz5r6CCK3yH+PwUCYc7Oxw8cZ2x2QHBvc3Rl
by5uZXQACgkQa+ggit8h/j/KBwD/QRG086d0GsQcaPtmF9XknAi63U5m3nnrocJw
9X8NaqIA+wb0gAz34Lg1aj44VJHxGo+NEADlnRoRQRH1bvIIXxB0
=qyc9
-----END PGP SIGNATURE-----

J
J
Jesse Gibbons wrote on 1 Jan 2022 20:14
(address . 52913@debbugs.gnu.org)
5491e57c-ae88-aa24-256b-c7d6f038cba2@gmail.com
Toggle quote (1 lines)
> What would be the best way to fix this?
>  - keep a mozjs-78.6 package around just for 0ad
>  - patch 0ad to fix the compatibility issues with mozjs 78.15
>  - use the mozjs version bundled in the 0ad sources
>
> WDYT?

Keeping mozjs-78.6 just for 0ad will probably make things harder later
on because it's another package to maintain and users likely won't be
able install 0ad and icecat/icedove in the same profile. I suppose users
can always use `-P /path/to/0ad-profile` when installing or updating 0ad.

I'm thinking using the bundled mozjs is perhaps the best option, though
it isn't particularly guixy, because I expect most users would want the
0ad packaged by guix to be compatible with other 0ad builds in the wild.
However, I think it would be useful to fix compatibility issues with
mozjs 78.15 so interested contributors can tell upstream if guix's
current minor version breaks the expected deterministic behaviors
described in the error.

Another option would be to keep mozjs-78.6 for 0ad and patch it so
interested users can test updated mozjs using
`--with-input=mozjs=mozjs`. It isn't very difficult to modify a list of
packages to use a specific mozjs in a manifest or home configuration,
right? Though I guess interested contributors could always add the patch
themselves just as easily...

Anyway, that's my two cents.
L
L
Liliana Marie Prikler wrote on 1 Jan 2022 21:27
(name . Guillaume Le Vaillant)(address . glv@posteo.net)
b70e100000e1b213c4831857f229553b26075ed6.camel@gmail.com
Am Samstag, dem 01.01.2022 um 12:14 -0700 schrieb Jesse Gibbons:
Toggle quote (12 lines)
>  > What would be the best way to fix this?
>  >  - keep a mozjs-78.6 package around just for 0ad
>  >  - patch 0ad to fix the compatibility issues with mozjs 78.15
>  >  - use the mozjs version bundled in the 0ad sources
>  >
>  > WDYT?
>
> Keeping mozjs-78.6 just for 0ad will probably make things harder later
> on because it's another package to maintain and users likely won't be
> able install 0ad and icecat/icedove in the same profile. I suppose
> users can always use `-P /path/to/0ad-profile` when installing or
> updating 0ad.
I don't think that would be a problem, since mozjs is a regular input.
There ought to be no propagation conflicts between the two.

Toggle quote (4 lines)
> I'm thinking using the bundled mozjs is perhaps the best option, though
> it isn't particularly guixy, because I expect most users would want the
> 0ad packaged by guix to be compatible with other 0ad builds in the
> wild.
There is no benefit to using a bundled version over one packaged with
Guix.

Toggle quote (6 lines)
> Another option would be to keep mozjs-78.6 for 0ad and patch it so
> interested users can test updated mozjs using
> `--with-input=mozjs=mozjs`. It isn't very difficult to modify a list of
> packages to use a specific mozjs in a manifest or home configuration,
> right? Though I guess interested contributors could always add the
> patch themselves just as easily...
Contributors would probably work on top of their local checkouts
anyway, so there's no concern here (other than increased store space
for another mozjs both locally for users and in CI).

@Guillaume: From what I can gather from the build error, it appears as
though the calling convention changed to require an additional
parameter. I've tracked down the relevant commit [1] and bug [2].

Now obviously doing such a thing violates SemVer, so if rewriting 0ad
with this and other changes in mind is not an option, I think having a
hidden package for 0ad might be the lesser evil.

Cheers

[1]
G
G
Guillaume Le Vaillant wrote on 2 Jan 2022 13:56
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87h7amwarj.fsf@kitej
Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

Toggle quote (14 lines)
> @Guillaume: From what I can gather from the build error, it appears as
> though the calling convention changed to require an additional
> parameter. I've tracked down the relevant commit [1] and bug [2].
>
> Now obviously doing such a thing violates SemVer, so if rewriting 0ad
> with this and other changes in mind is not an option, I think having a
> hidden package for 0ad might be the lesser evil.
>
> Cheers
>
> [1]
> https://searchfox.org/mozilla-central/commit/a3c605929b16303e8a52ae9d99d5fe6769e8bf09
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1681268

Thanks for the pointers.
I added a phase to fix the compatibility issue with mozjs-78.15, and
pushed as fea60a2fff443b9c172ed28bd37361e34e064f13.
-----BEGIN PGP SIGNATURE-----

iIUEAREKAC0WIQTLxZxm7Ce5cXlAaz5r6CCK3yH+PwUCYdGigA8cZ2x2QHBvc3Rl
by5uZXQACgkQa+ggit8h/j+hcQD+IcTPz6fz0lGYV164v3zQdgONsnQASOuq/YUX
d9Tr2qcA/2A1ktz4tmXiDnROSeM/ri9Gm8Lv542PqUuqKCDaXYOG
=aoxs
-----END PGP SIGNATURE-----

Closed
?