Opened 3 years ago

Last modified 3 years ago

#5868 new defect

FTBFS with option "--with-system-mozjs"

Reported by: Marco Amadori Owned by: s0600204
Priority: Must Have Milestone: Backlog
Component: Build & Packages Keywords: icu, libicu, spidermonkey
Cc: Patch: Phab:D3127

Description

0ad git version 82427a92c

debian amd64 unstable with libnvtt2 version 2.1.0-git20160229+dfsg-2~exp1+b4 and ccache

to reproduce:

~/src/git/0ad $ build/workspaces/clean-workspaces.sh
~/src/git/0ad $ LANG=C build/workspaces/update-workspaces.sh --with-system-nvtt --with-system-mozjs52 -j6
~/src/git/0ad $ LANG=C make verbose=1 -C build/workspaces/gcc -j6

build logs attached

Attachments (3)

update-workspaces.log.gz (160.1 KB ) - added by Marco Amadori 3 years ago.
update-workspaces.log, gzipped for space constraint
build.log (430.8 KB ) - added by Marco Amadori 3 years ago.
make verbose=1 -C build/workspaces/gcc -j6
engine.make (21.5 KB ) - added by Marco Amadori 3 years ago.
build/workspaces/gcc/engine.make

Download all attachments as: .zip

Change History (15)

by Marco Amadori, 3 years ago

Attachment: update-workspaces.log.gz added

update-workspaces.log, gzipped for space constraint

by Marco Amadori, 3 years ago

Attachment: build.log added

make verbose=1 -C build/workspaces/gcc -j6

by Marco Amadori, 3 years ago

Attachment: engine.make added

build/workspaces/gcc/engine.make

comment:1 by s0600204, 3 years ago

I can duplicate this issue... on ArchLinux.

I think the problem arises in that not only is spidermonkey 52 built against a certain version of icu (58.2 to be exact), but that the header files for that version of icu are shipped inside the mozjs-52-dev (debian) / js52 (archlinux) packages (and matching libicu*.so lib files are not).

Thus, when the --with-system-mozjs52 argument is given to update-workspaces.sh, and the include files for spidermonkey are included, the icu 58 headers are included as well and are subsequently used to compile the various components of 0AD that #include any icu headers.

(The correct headers are most likely in the includes search path, but the ones shipped with mozjs-52-dev/js52 get found first and are thus used instead.)

Later, when we get to linking, ld uses the version of icu that pkgconfig has found for it - but this doesn't match the version that was used for compilation.

Hence the error.


Debian does not appear to package mozjs-60-dev or mozjs-78-dev with the icu headers included, so this problem should be resolved - for them - when we migrate to a more recent version of spidermonkey.

Unfortunately, ArchLinux does ship js60, js68, and js78 packages with icu header files included, so users there will remain unable to use --with-system-mozjs* (unless we find a solution).

I haven't checked any other distro's packages.

comment:2 by s0600204, 3 years ago

Keywords: spidermonkey added; debian sid unstable icu::Locale::getUS removed

comment:3 by Marco Amadori, 3 years ago

Thanks for the quick and insightful reply!

comment:4 by s0600204, 3 years ago

Summary: FTBFS on debian sid with option "--with-system-mozjs52"FTBFS with option "--with-system-mozjs52"

No worries.

Of course, the question is: why are icu headers being bundled into spidermonkey packages?

An answer might lie here: https://firefox-source-docs.mozilla.org/intl/icu.html, which states:

Using system ICU will disable some Intl functionality, such as historically accurate time zone calculations, that can’t be readily supported without a precisely-controlled ICU.

Which explains why a version of icu is bundled in with the spidermonkey source, I guess. And as

Updates to ICU and related components must be synchronized [...] in lockstep

I suppose it makes sense to feed the header files to projects that embed spidermonkey so they also remain in sync.

So why have Debian stopped doing this (from sm60 up)?

[...] the main reason we were using the bundled one being the system libraries were too old. [ ref ]

So why don't other repos follow suit where they have a recent enough version of icu? (Gentoo appears to from sm68 onwards.)

*shrugs*

As a comparison, here is a list of distros and their respective spidermonkey and "system" icu versions.

Of course, the irony is that we compile our bundled spidermonkey with the --without-intl-api flag set... which disables the use of icu within spidermonkey.


Packaging note: passing either --without-intl-api or --with-system-icu to the spidermonkey build process, looks to prevent said process from copying the bundled icu header files to the include-{win32|unix}-{debug|release} directories.

comment:5 by wraitii, 3 years ago

Thanks vey much for investigating this further, I had a hunch it had to do with SM pulling in some odd dependency.

It seems from the makefile above that there are no specific include paths specified for ICU, which means we might fix this by ordering include paths in the right order; I'm not certain how doable that is, though.

---

We'll have a bit of trouble for SM anyways if we embed versions close to HEAD, as the ESR regularly get version upgrades and that means packages might no longer link the correct minor version. I'm somewhat considering dropping the "with system" option because of that.

One last question for me as I'm not very familiar with linux packaging, that maybe you can answer: can packagers still package the game with our own specific version of spidermonkey? Or would they require that we use the system one?

comment:6 by Marco Amadori, 3 years ago

About debian packaging, 0ad alpha23 was packaged without system mozjs, probably because the mozjs version 38 was not available in the debian repos. So what you suggest it could be a way if others are not available.

comment:7 by Stan, 3 years ago

Well packagers currently "can't" because of the Python2 Dep, but I assume there would be no issue if that wasn't the case. They prefer to use system libs when possible though.

comment:8 by s0600204, 3 years ago

It seems from the makefile above that there are no specific include paths specified for ICU,

That would be because the icu headers are in the "standard system directory" for headers (/usr/include), thus a specific path is not needed.

The reason why mozjs52 needs a specific path (e.g. /usr/include/mozjs-52) is because it is possible to have multiple mozjs* versions installed at the same time and thus there is a need to point to the correct one.

The "specific path" for icu would be the same as the "standard system directory", and we've encountered problems with having that listed too early before: #5157.

which means we might fix this by ordering include paths in the right order; I'm not certain how doable that is, though.

From a brief skim of the documentation for gcc, there doesn't appear to be a way to tell it explicitly to ignore a given directory whilst looking for header files.

There is, however, the -idirafter option which dictates that the given directory should be searched after the "standard system directory".

And preliminary results indicate that this works! But it does require patching premake to support it.


can packagers still package the game with our own specific version of spidermonkey? Or would they require that we use the system one?

I think packagers would prefer that we don't bundle spidermonkey, if only because its easier to track/resolve CVEs and stuff like that. Also: potentially smaller package sizes. However, if we need to build against exact minor versions of spidermonkey for... uh, to prevent OOS?... then packagers might just have to put up with it.

comment:9 by s0600204, 3 years ago

Owner: set to s0600204
Patch: Phab:D3127

comment:10 by Stan, 3 years ago

Summary: FTBFS with option "--with-system-mozjs52"FTBFS with option "--with-system-mozjs"

comment:11 by wraitii, 3 years ago

Milestone: Alpha 24Alpha 25

The issue is tricky and some bugs aren't fixed upstream, so atm this is somewhat unfixable -> pushing back.

comment:12 by wraitii, 3 years ago

Milestone: Alpha 25Backlog

Backlogging, don't expect fast progress here

Note: See TracTickets for help on using tickets.