bug#52913: 0ad only builds fine with a specific version of mozjs

bug#52913: 0ad only builds fine with a specific version of mozjs

[Guillaume Le Vaillant]

[-- Attachment #1: Type: text/plain, Size: 4903 bytes --]

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:

--8<---------------cut here---------------start------------->8---
../../../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
--8<---------------cut here---------------end--------------->8---

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

bug#52913: 0ad only builds fine with a specific version of mozjs

[Jesse Gibbons]

 > 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.

bug#52913: 0ad only builds fine with a specific version of mozjs

[Liliana Marie Prikler]

Am Samstag, dem 01.01.2022 um 12:14 -0700 schrieb Jesse Gibbons:
>  > 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.

> 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.

> 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]
https://searchfox.org/mozilla-central/commit/a3c605929b16303e8a52ae9d99d5fe6769e8bf09
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1681268

bug#52913: 0ad only builds fine with a specific version of mozjs

[Guillaume Le Vaillant]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> @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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]