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