`build.rs` doesn't spot wrong rustc version

JohanLorenzo commented 8 years ago

STR

Be at a revision lower than the current master. Current master: 2016-05-30. Revision tested: 2016-05-22
git pull upstream master
cargo build
Results

Cargo understands there's a new Cargo.lock, fetches the new dependencies and compiles them. For example:

    Updating git repository `https://github.com/dhylands/get_if_addrs`
[More git repo updates...] 
 Downloading serde v0.7.6
[More downloads...] 
 Downloading c_linked_list v1.1.0
   Compiling quasi v0.11.0
[More dependencies compiled...] 
   Compiling c_linked_list v1.1.0
Build failed, waiting for other jobs to finish...
/home/jlorenzo/.multirust/toolchains/nightly-2016-05-22/cargo/registry/src/github.com-1ecc6299db9ec823/aster-0.17.0/src/expr.rs:1396:65: 1396:75 error: mismatched types [E0308]
/home/jlorenzo/.multirust/toolchains/nightly-2016-05-22/cargo/registry/src/github.com-1ecc6299db9ec823/aster-0.17.0/src/expr.rs:1396         self.builder.build_expr_kind(ast::ExprKind::Loop(block, self.label))
                                                                                                                                                                                                     ^~~~~~~~~~

This shows build.rs is called after dependencies are build (so right before foxbox is compiled). cargo clean && cargo build returns the same error.

I added a panic!() in build.rs, this is what is happening:

$ cargo build -j1
   Compiling regex-syntax v0.3.3
[More compiling...]
   Compiling openssl-sys v0.7.13
   Compiling foxbox v0.1.0 (file:///home/jlorenzo/git/fxbox/foxbox)
build.rs:16:5: 16:46 warning: unreachable statement, #[warn(unreachable_code)] on by default
build.rs:16     let info = rustc_version::version_meta();
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: failed to run custom build command for `foxbox v0.1.0 (file:///home/jlorenzo/git/fxbox/foxbox)`
Process didn't exit successfully: `/home/jlorenzo/git/fxbox/foxbox/target/debug/build/foxbox-d218e9c349bd8f7e/build-script-build` (exit code: 101)
--- stderr
thread '<main>' panicked at 'BUILD.RS IS NOW CALLED', build.rs:15
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Impact

Rust ups are usually broken by dependencies, this makes the check done in build.rs nearly useless. Moreover, this might give a false sense of a graceful failure.

The cargo documentation is a little bit ambiguous about the order of build.

The Rust file designated by the build command (relative to the package root) will be compiled and invoked before anything else is compiled in the package

Therefore, I don't know if we should raise a bug in rust-lang/cargo or remove check_rustc_version() in build.rs. What's your take on this, @fabricedesre ?

fabricedesre commented 8 years ago

I added that not so much to help with people doing a rustup, but more to warn people that are not up to date anymore. I think the current still provides that (unless we don't have to rebuild the foxbox crate which should not happen often in a rustup).

Indeed it's annoying in general that build.rs is not processed first. I don't know what the cargo people would be willing to take, but if we could add a "rustc-version = ..." check in the main Cargo.toml that would be the simplest thing for us.

fabricedesre commented 8 years ago

I guess the other option is, like Servo, to not call cargo directly but to use a wrapper script that does these kind of checks (Servo uses mach that is also used in gecko).

JohanLorenzo commented 8 years ago

Linking to #449 where mach was also proposed

I agree, mach would be a good actionable alternative.

fxbox / foxbox

`build.rs` doesn't spot wrong rustc version #516

STR

Results

Impact