1694318 - Remove the mozjs_sys and js crates

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task)

Description

[Mike Hommey [:glandium]]

cmake is used to build exactly one source file, which could just as well be done with the cc crate instead.

Comment 1

[Mike Hommey [:glandium]]

Fun facts:

• It looks like the current setup only builds by chance (https://github.com/rust-lang/cargo/issues/9200)
• On top of that, the current sequence of builds only works by chance (the second command works if it runs after the first one because some things are not actually recompiled that should be, but starting from scratch, it doesn't work)
• The dependency on cmake was actually removed upstream close to 3 years ago! (https://github.com/servo/rust-mozjs/commit/52554a69a1db9ea1b684108411ebec7cd3b2fce7)
• Tons of changes have happened to the crate upstream that are not in m-c. The same applies to the mozjs crate (from js/src/Cargo.toml)

At this point, I'm wondering if we shouldn't remove the crate, and the corresponding build tasks.

Comment 2

[Jan de Mooij [:jandem]]

Hey Josh, how do you feel about comment 1?

It sounds like upstream Servo has diverged from the code in mozilla-central and this crate is making things difficult for glandium, so if Servo isn't using it anyway it's worth reconsidering.

Comment 3

[Mike Hommey [:glandium]]

Filed https://github.com/servo/mozjs/issues/270 after some more investigation.

Comment 4

[Josh Matthews [:jdm]]

Yep, Servo doesn't use the crate in mozilla-central. You're free to remove it.

Comment 5

[Mike Hommey [:glandium]]

Thanks Josh.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1694318 - Remove the js rust crate.

The sm-rust-bindings task that builds and runs tests for it actually only
doesn't fail by three(!) distinct happenstances of chance:

• It somehow doesn't hit a race condition that can happen, but that
  attempting to change the build process for jsglue.cpp did,
• The second call to cargo test without --feature debugmozjs only
  works because nothing was cleaned up from the first and the second
  build uses artifacts from the first that are otherwise not there if
  only running the second.
• The addition of rust code to spidermonkey (the jsrust crate in
  js/src/rust) somehow didsn't cause problems in the current
  configuration, but in other configurations (which fixing the second
  thing above unveils) the build fails because of the multiple
  definition of rust_eh_personality between the jsrust staticlib and the
  rust stdlib that rustc links to test programs.

On top of all the above, the content of the crate in mozilla-central
has not received updates from upstream servo in years, and doesn't
reflect what servo actually uses.

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1694318 - Remove the mozjs_sys rust crate.

The mozjs_sys crate is actually responsible for all the problems
encountered with the js crate we just removed, but the sm-mozjs-crate
task that builds and runs tests for it doesn't fail because... there is
actually no test to build and run, so all it builds is a static library,
which doesn't expose all the problems that exist.

The content of this crate in mozilla-central is also outdated compared
to servo upstream.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1694318 - Remove the debian8-mozjs-rust-build docker image.

The task that was using it was removed. While here, we remove the now
unused cmake backport for Debian jessie.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ceacb906b5e
Remove the js rust crate. r=jandem,sheehan
https://hg.mozilla.org/integration/autoland/rev/4d19c36b6b32
Remove the mozjs_sys rust crate. r=jandem
https://hg.mozilla.org/integration/autoland/rev/3d54ca5931f0
Remove the debian8-mozjs-rust-build docker image. r=firefox-build-system-reviewers,andi,mhentges

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6ceacb906b5e
https://hg.mozilla.org/mozilla-central/rev/4d19c36b6b32
https://hg.mozilla.org/mozilla-central/rev/3d54ca5931f0