Bug#1029167: mozjs78: Fails to build on armhf and armel

[Simon McVittie]

On Mon, 13 Mar 2023 at 15:27:19 +0100, Emanuele Rocca wrote:
> On 2023-01-27 12:03, Adrian Bunk wrote:
> > WITH_SYSTEM_ICU = yes fixes this error on armel.
> 
> And on armhf too.
> 
> > The build fails later due to 146 TEST-UNEXPECTED-FAIL,
> > which is not a problem for 0ad who aren't running the mozjs testsuite.
> 
> Of those 146 test failures, many seem to be discrepancies such as:
> 
>   /build/mozjs78-78.15.0/js/src/tests/non262/Date/shell.js:160:21 Error: Assertion failed: got "CEST", expected "Central European Summer Time"
> 
>   /build/mozjs78-78.15.0/js/src/tests/non262/Intl/DisplayNames/dateTimeField.js:141:15 Error: Assertion failed: got "yr", expected "yr."
> 
>   /build/mozjs78-78.15.0/js/src/tests/non262/Intl/NumberFormat/shell.js:42:21 Error: Assertion failed: got "US$123.00", expected "$123.00": locale=en-CA, options={"style":"currency","currency":"USD","currencyDisplay":"narrowSymbol"}, value=123
> 
> There are a bunch of patches under debian/patches/system-ICU that seem
> to deal with those differences such as [1]. Even applying those patches
> though, other tests (140+) fail in a similar way.
> 
> The presence of those patches under debian/patches/system-ICU seems to
> suggest that we don't really care much about the discrepancies. If
> that's the case, should we just skip the tests? Any other suggestion for
> how to properly move from the embedded ICU to libicu-dev?

For context, the one remaining source package with a direct dependency
on mozjs78 is Cinnamon's cjs. The other former rdep is GNOME's gjs (of
which cjs is a fork), which has moved to the newer mozjs branch mozjs102.

The root cause of many of these test failures is that mozjs' upstream
(Mozilla) doesn't really seem to treat a non-vendored ICU as fully
supported, and many of the tests make assertions that are really facts
about the behaviour of the version of ICU in use, more than being facts
about the mozjs codebase; and because they have only tested with the
vendored version of ICU and not Debian's different version, the test
results reflect that.

If people who are interested in mozjs78/cjs/Cinnamon want to move it to
a non-vendored ICU, then someone who wants to do that work should go
through the failing tests and assess whether they are "real failures"
that would indicate a user-visible bug in the mozjs78/cjs/Cinnamon stack,
or whether they are ignorable implementation details like using slightly
different wording for a localized string.

The patches in debian/patches/system-ICU are the result of someone in
the GNOME team (in practice likely several people) going through that
process in the past, before mozjs78 switched to using its vendored ICU
(which was originally done to avoid mozjs78 being tightly coupled to an
ICU transition). They were correct for the then-current version of ICU,
but our system copy of ICU has changed since then, so some of the patches
will probably still be needed, but some probably need to be dropped or
modified, and others probably need to be added.

I don't think it would be a good solution to skip mozjs' test suite
altogether on any architecture, because it's highly architecture-specific
(it's a JIT interpreter which generates architecture-specific machine code
and makes use of architecture-specific facts about things like atomic
operations and memory layout), and we have had incidents in the past
where it was broken on release architectures. Nobody is routinely running
Cinnamon on many (perhaps most) of our architectures, so the build-time
test suite is the only evidence we have that mozjs is functional on
the remaining architectures. Some test failures represent a RC bug, but
others don't, and unfortunately this needs to be assessed case-by-case.

This is particularly true now that gjs has stopped using mozjs78, because
gjs has build-time test and autopkgtest coverage which would detect severe
brokenness in mozjs (and genuinely did detect that in the past), but cjs'
build-time tests are disabled and it doesn't seem to have any non-trivial
autopkgtests either (only a build smoke-test and an ABI check).

The other possible route for fixing mozjs78's build on armhf and armel
is to locate whatever fixes in ICU made it link successfully on armhf
and armel, and backport those to the version of ICU that is vendored by
mozjs78. I would expect this to be the easier route, and almost certainly
a smaller diff to present to the release team, with a correspondingly
smaller regression risk for mozjs78/cjs/Cinnamon.

    smcv