Closed Bug 1512271 Opened 2 years ago Closed 2 years ago
Upgrade to rustc 1
The release is tomorrow, December 6th, however we're already in the soft freeze, so this will wait until after the merge.
Hi Simon, the rust tests builds start failing with this version with linker errors like https://github.com/rust-lang/rust/pull/44603#issuecomment-338807312 Does this workaround need an update for rustc 1.31? Is this something you could take a look at? Here's the try push: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=216291408&revision=9f4b8f6207de6a10c83a136d60caf687959de146
I really don’t know what’s going on here. I think that the work-around at https://github.com/servo/servo/pull/19003 is not relevant because Gecko (libxul) should be available, unlike when running Rust’s `src/tools/cargotest` test suite. Emilio, any idea where to look? I see from the log that `mach build -v pre-export export recurse_rusttests` is the command failing, but I don’t find the exact Cargo command that that runs.
Flags: needinfo?(simon.sapin) → needinfo?(emilio)
It's in the log it can just be tricky to spot: 01:15:48 INFO - env RUSTC_WRAPPER='z:/build/build/src/sccache2/sccache.exe' CARGO_TARGET_DIR=z:/build/build/src/obj-firefox RUSTFLAGS='-C opt-level=2 -C debuginfo=2 ' RUSTC=z:/build/build/src/rustc/bin/rustc.exe RUSTDOC=z:/build/build/src/rustc/bin/rustdoc.exe RUSTFMT=z:/build/build/src/rustc/bin/rustfmt.exe CFLAGS_i686_pc_windows_msvc="-w" MOZ_SRC=z:/build/build/src MOZ_DIST=z:/build/build/src/obj-firefox/dist LIBCLANG_PATH="z:/build/build/src/clang/bin" CLANG_PATH="z:/build/build/src/clang/bin/clang.exe" PKG_CONFIG_ALLOW_CROSS=1 RUST_BACKTRACE=full MOZ_TOPOBJDIR=z:/build/build/src/obj-firefox CARGO_INCREMENTAL=0 z:/build/build/src/rustc/bin/cargo.exe test --target=i686-pc-windows-msvc --no-fail-fast -p selectors -p servo_arc -p stylo_tests --features "servo bindgen gecko_debug quantum_render simd-accel moz_memory spidermonkey_rust gecko_profiler" --frozen --manifest-path z:/build/build/src/toolkit/library/rust/Cargo.toml -vv
Not off-hand, but taking a closer look: 01:26:42 INFO - warning: lint `private_no_mangle_fns` has been removed: `no longer an warning, #[no_mangle] functions always exported` 01:26:42 INFO - --> servo\ports\geckolib\tests\servo_function_signatures.rs:19:40 01:26:42 INFO - | 01:26:42 INFO - 19 | #[allow(non_snake_case, unused_unsafe, private_no_mangle_fns)] 01:26:42 INFO - | ^^^^^^^^^^^^^^^^^^^^^ 01:26:42 INFO - | 01:26:42 INFO - = note: #[warn(renamed_and_removed_lints)] on by default Which comes from https://github.com/rust-lang/rust/pull/54451, which is in the range. So it seems we are relying on Rust not exporting private #[no_mangle] functions so that link.exe could strip them out (since they're not called).
So I suspect something like this should do the trick if I'm right, though I have no windows machine to test. Chris, does that fix the issue? Longer term we should just avoid relying on this test that uses the bindgen-generated signatures and switch to cbindgen for this job, presumably.
Attachment #9030444 - Flags: feedback? → feedback?(cmanchester)
Comment on attachment 9030444 [details] [diff] [review] Allow link.exe to keep linking the stylo tests after rust-lang/rust#54451. Review of attachment 9030444 [details] [diff] [review]: ----------------------------------------------------------------- Ok, the theory that this is due to https://github.com/rust-lang/rust/pull/54451 sounds very plausible. This looks like an appropriate fix. r+ assuming a successful try run. I chatted with Emilio on IRC to understand how this all works: we have `extern fn` Rust functions in servo/ports/geckolib/glue.rs, and hand-written C++ declarations in layout/style/ServoBindings.h so that they can be called form C++. There’s a test that uses Bindgen to convert those C++ headers back to Rust declarations (in this generated glue.rs file), and compares function pointer types to make sure the signatures are identical.
Attachment #9030444 - Flags: review?(simon.sapin) → review+
Comment on attachment 9030444 [details] [diff] [review] Allow link.exe to keep linking the stylo tests after rust-lang/rust#54451. Review of attachment 9030444 [details] [diff] [review]: ----------------------------------------------------------------- Try looks good, thank you very much for the patch!
Attachment #9030444 - Flags: feedback?(cmanchester) → feedback+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/37f24bab080b Allow link.exe to keep linking the stylo tests after rust-lang/rust#54451. r=simonsapin https://hg.mozilla.org/integration/mozilla-inbound/rev/677bcec279cd Upgrade builders to rustc 1.31 r=ted
2 years ago
You need to log in before you can comment on or make changes to this bug.