#28260 closed defect (fixed)

Tor nightly builds are broken on Linux with Rust enabled (since 10/31)

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, TorBrowserTeam201811R
Cc: boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Landing 74c1e44746a7d9c810f095177af88b7dbaafd3e3 in tor breaks our nightly builds, presumably because our Rust compiler (1.26.1) is too old for that:

   Compiling tor_allocate v0.0.1 (file:///var/tmp/build/tor-aa1ae1343a2e/src/rust/tor_allocate)
   Compiling smartlist v0.0.1 (file:///var/tmp/build/tor-aa1ae1343a2e/src/rust/smartlist)
  CC     src/lib/crypt_ops/src_lib_libtor_crypt_ops_a-crypto_curve25519.o
error[E0432]: unresolved import `std::alloc::System`
  --> tor_allocate/lib.rs:14:5

I think we have three options to fix that:

1) We build the nightlies with the Rust version tor needs.
2) We build only tor in the nightlies with the Rust version tor needs.
3) We disable compiling tor with Rust on nightlies

I think I prefer 2) here.

Child Tickets

Change History (17)

comment:1 Changed 13 months ago by boklm

Option 2) sounds good to me. Unless changes to support two rust versions are too complex, but we can try 2) first and try the the others if it doesn't work.

comment:2 Changed 13 months ago by boklm

std::alloc::System was introduced in version 1.28.0:
https://doc.rust-lang.org/std/alloc/struct.System.html

comment:3 Changed 13 months ago by boklm

When trying to build rust 1.28.0, using rust 1.27.2, I'm getting the following error:

   Compiling error-chain v0.11.0
   Compiling url v1.7.0
   Compiling semver v0.9.0
   Compiling toml v0.4.6
   Compiling cargo_metadata v0.5.4
   Compiling rustc_version v0.2.2
   Compiling clippy v0.0.202 (file:///var/tmp/build/rustc-1.28.0-src/src/tools/clippy)
error: failed to run custom build command for `clippy v0.0.202 (file:///var/tmp/build/rustc-1.28.0-src/src/tools/clippy)`
process didn't exit successfully: `/var/tmp/build/rustc-1.28.0-src/build/build/x86_64-unknown-linux-gnu/stage2-tools/release/build/clippy-24df587c23c51264/build-script-b
uild` (exit code: 101)
--- stderr
thread 'main' panicked at 'current rustc version information does not contain a rustc commit date', libcore/option.rs:960:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

warning: build failed, waiting for other jobs to finish...
error: failed to run custom build command for `clippy v0.0.202 (file:///var/tmp/build/rustc-1.28.0-src/src/tools/clippy)`
process didn't exit successfully: `/var/tmp/build/rustc-1.28.0-src/build/build/x86_64-unknown-linux-gnu/stage2-tools/release/build/clippy-24df587c23c51264/build-script-b
uild` (exit code: 101)
--- stderr
thread 'main' panicked at 'current rustc version information does not contain a rustc commit date', libcore/option.rs:960:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

warning: build failed, waiting for other jobs to finish...
error: build failed
command did not execute successfully: "/var/tmp/dist/rust-rust-old/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "8" "--release" "--frozen" "--manifest-p
ath" "/var/tmp/build/rustc-1.28.0-src/src/tools/clippy/Cargo.toml" "--features" "" "--message-format" "json"
expected success, got: exit code: 101

comment:4 Changed 13 months ago by gk

It's okay using the latest stable one if that helps I think.

comment:5 Changed 13 months ago by alexcrichton

Oh dear, sorry for the breakage here! This can also be fixed locally with a patch that looks like this -- https://gist.github.com/alexcrichton/108f3c3bb70d8e5d250a63f0ca986d58 -- where the feature here which requires 1.28.0 isn't used unless 1.28.0 is compiled in. It means that the sanitizer tests won't work on pre-1.28.0 compilers, but that's already true for other reasons!

If something like that would work I don't mind sending a PR for it!

comment:6 in reply to:  5 Changed 13 months ago by boklm

Replying to alexcrichton:

If something like that would work I don't mind sending a PR for it!

I confirm this patch is fixing the build issue. Sending a PR for it sounds good to me.

comment:7 Changed 13 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201810 removed

Moving our tickets to November.

comment:8 Changed 13 months ago by gk

Just to share the results of building 1.30.0 (the current stable): it breaks during installation[sic!]:

Build completed successfully in 0:06:09
    Finished dev [unoptimized] target(s) in 0.34s
thread 'main' panicked at 'could not canonicalize /var/tmp/dist/rust', bootstrap/install.rs:78:48

comment:9 Changed 13 months ago by boklm

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: newneeds_review

<+GeKo> boklm: in other news i think the broken rust compliation is due to us creating $distdir only for macOS by |mkdir -p $distdir/helper|
<+GeKo> starting with the canonicalization rust got picky about exising dirs for the prefix it seems
<+GeKo> (i was wondering why i did not see the problem during my bisect thing for #26475)
<+GeKo> boklm: so, doing a |mkdir -p $distdir| right at the beginning (and just a |mkdir $distdir/helper| for macOS later on)
<+GeKo> should be all we need
<+GeKo> and i think we should go with 1.30.0 while we are at it
<+GeKo> feel free to test that and add it to your branch in case it helps

Adding the mkdir -p $distdir/helper is indeed fixing the issue issue building 1.30.0.

In branch bug_28260_v3 there is a patch updating Rust to 1.30.0, only for Tor:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_28260_v3&id=8fc28dd994888b620d8e2ae334e2466e043dead1

comment:10 Changed 13 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201811R removed
Status: needs_reviewneeds_revision

Just one nit: In the macOS block it is enough to have mkdir $distdir/helper as we are now creating $distdir for all platforms at the beginning.

comment:11 in reply to:  5 Changed 13 months ago by gk

Replying to alexcrichton:

Oh dear, sorry for the breakage here! This can also be fixed locally with a patch that looks like this -- https://gist.github.com/alexcrichton/108f3c3bb70d8e5d250a63f0ca986d58 -- where the feature here which requires 1.28.0 isn't used unless 1.28.0 is compiled in. It means that the sanitizer tests won't work on pre-1.28.0 compilers, but that's already true for other reasons!

If something like that would work I don't mind sending a PR for it!

Thanks for the patch, really appreciated! I am a bit wary to either ship it just in our Tor Browser build environment (as we would not test tor master anymore that way) or trying to convince the tor folks to take it for our sake despite their policy to essentially go with Rust stable releases.

This won't be the last time that we have to figure out how to deal with different Rust minimum version requirements (i.e. our compiler requirements for all platforms and all parts of the bundle vs. those tor has). So far I think using two different ones for our nightly builds is okay-ish for us. But it might be smart to think harder to be prepared for future issues.

comment:12 in reply to:  10 Changed 13 months ago by boklm

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: needs_revisionneeds_review

Replying to gk:

Just one nit: In the macOS block it is enough to have mkdir $distdir/helper as we are now creating $distdir for all platforms at the beginning.

I removed the -p in mkdir $distdir/helper in branch bug_28260_v4:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_28260_v4&id=d750a50014818bcc4b0646864d2451fa93b9556f

comment:13 Changed 13 months ago by gk

Status: needs_reviewneeds_information

boklm: Did you get a 1.30 for i686 compiled as well with this patch? I am just getting LLVM_ERROR: out of memory. If you get the same and 1.28 would work around that (yes, we should do #26323 at some point :) ), then using that one is fine by me.

comment:14 in reply to:  13 Changed 13 months ago by boklm

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201811R removed
Status: needs_informationneeds_revision

Replying to gk:

boklm: Did you get a 1.30 for i686 compiled as well with this patch? I am just getting LLVM_ERROR: out of memory. If you get the same and 1.28 would work around that (yes, we should do #26323 at some point :) ), then using that one is fine by me.

Hm, I forgot to test i686, and I have the same error.

comment:15 Changed 13 months ago by gk

1.28.0 works for me.

comment:16 in reply to:  15 Changed 13 months ago by boklm

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: needs_revisionneeds_review

Replying to gk:

1.28.0 works for me.

It works for me too.

I made a new revision of the patch in branch bug_28260_v5:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_28260_v5&id=0a633e6289822f316b838b32fe0135d5b7c9967d

comment:17 Changed 13 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good, thanks! Merge to master (commit 0a633e6289822f316b838b32fe0135d5b7c9967d).

Note: See TracTickets for help on using tickets.