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)
Change History (15)
by , 3 years ago
Attachment: | update-workspaces.log.gz added |
---|
comment:1 by , 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 , 3 years ago
Keywords: | spidermonkey added; debian sid unstable icu::Locale::getUS removed |
---|
comment:4 by , 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 , 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 , 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 , 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 , 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 , 3 years ago
Owner: | set to |
---|---|
Patch: | → Phab:D3127 |
comment:10 by , 3 years ago
Summary: | FTBFS with option "--with-system-mozjs52" → FTBFS with option "--with-system-mozjs" |
---|
comment:11 by , 3 years ago
Milestone: | Alpha 24 → Alpha 25 |
---|
The issue is tricky and some bugs aren't fixed upstream, so atm this is somewhat unfixable -> pushing back.
comment:12 by , 3 years ago
Milestone: | Alpha 25 → Backlog |
---|
Backlogging, don't expect fast progress here
update-workspaces.log, gzipped for space constraint