Add `xargo-check` command #267

Aaron1011 · 2019-11-14T21:31:41Z

This allows configuring Xargo to run cargo check
instead of cargo build

Aaron1011 · 2019-11-14T21:31:54Z

README.md

RalfJung · 2019-11-15T09:20:23Z

Thanks a lot! I wanted some way to make a check-only libstd build for some time but did not get around to actually work on this.

However, I am not sure what the best UI is for this. Originally I envisioned that xargo check would also do a check-only libstd build, but that is likely not going to work very well. So a toml entry seems fine to me. @jethrogb what do you think (or do you not care)?

In terms of this PR, please add a test to make sure that this actually works.

README.md

jethrogb · 2019-11-15T11:54:57Z

Originally I envisioned that xargo check would also do a check-only libstd build, but that is likely not going to work very well.

This should work, right? As long as all the metadata is there...

That would be my preferred solution.

If we want to go the way this PR is written now, since this doesn't produce a usable sysroot, I think it's ok if it's invoked differently from regular xargo build. Like xargo --check-std. I don't think it makes a lot of sense to put it as an option in Xargo.toml. Do we parse the cmdline right now?

Aaron1011 · 2019-11-16T02:55:00Z

Do we parse the cmdline right now?

Currently, I think all arguments are forwarded to Cargo. Personally, I would prefer it if we kept it that way.

RalfJung · 2019-11-16T10:17:12Z

This should work, right? As long as all the metadata is there...

I don't think so; even doing cargo check will build the build scripts and proc macros, and that needs a libstd with code, not just its metadata.

RalfJung · 2019-11-27T09:11:12Z

@jethrogb

since this doesn't produce a usable sysroot, I think it's ok if it's invoked differently from regular xargo build. Like xargo --check-std. I don't think it makes a lot of sense to put it as an option in Xargo.toml.

As an alternative option, what about using an env var to control this?

jethrogb · 2019-11-27T09:13:24Z

I don't think so; even doing cargo check will build the build scripts and proc macros, and that needs a libstd with code, not just its metadata.

No it only needs that for the host, not the target

As an alternative option, what about using an env var to control this?

That also seems somewhat weird to me. What about a second binary xargo-check?

RalfJung · 2019-11-27T09:21:26Z

No it only needs that for the host, not the target

For our use-case (Miri), those are the same.

Aaron1011 · 2019-11-28T16:48:23Z

I'd be fine with a second binary. To be clear, this would be invoked as xargo-check to avoid conflicting with xargo check, right?

Aaron1011 · 2019-11-28T19:54:09Z

@RalfJung @jethrogb: I've created a new xargo-check command, and added a test for it. The old xargo.rs was moved into lib.rs. The bin/xargo.rs and bin/xargo-check.rs files simply pass in the proper mode to the main entrypoint.

README.md

src/lib.rs

src/sysroot.rs

tests/smoke.rs

RalfJung · 2019-12-03T18:15:02Z

I've created a new xargo-check command, and added a test for it. The old xargo.rs was moved into lib.rs. The bin/xargo.rs and bin/xargo-check.rs files simply pass in the proper mode to the main entrypoint.

The approach looks good! I left some nits.

One higher-level point: is there any point to actually run the per-project cargo after the libstd check build is done? That build will anyway probably not work as expected. So it might make more sense to make xargo-check only build libstd, and nothing else?

Aaron1011 · 2019-12-03T18:43:53Z

So it might make more sense to make xargo-check only build libstd, and nothing else?

@RalfJung: You can already do this by running xargo-check with no additional arguments.

RalfJung · 2019-12-03T18:44:49Z

You can already do this by running xargo-check with no additional arguments.

Oh wow, I had no idea xargo supports this.
Could you adjust the new test to do that instead?

EDIT: Hm, but looks like that invokes cargo's help screen? (At least it does for the main xargo binary.)

Aaron1011 · 2019-12-03T18:47:07Z

EDIT: Hm, but looks like that invokes cargo's help screen? (At least it does for the main xargo binary.)

You need to be in a project directly (with a Cargo.toml) for this to work.

RalfJung · 2019-12-03T18:47:39Z

Yes that's what I did. And it makes sense, plain xargo behaves just like cargo without a subcommand.

Aaron1011 · 2019-12-03T18:51:44Z

That's strange - it works for me locally. I get the help screen when outside a project directly, but a proper built when inside the directory.

I've created tests for both xargo-check and xargo-check check, which both pass locally. Let's see if they pass on CI.

RalfJung · 2019-12-03T18:52:45Z

I get a build followed by the help screen. And given that there is no logic in xargo for not invoking cargo when there is no command, that's what I would expect.

Can you make xargo-check not call cargo for the project when there is no subcommand?

Aaron1011 · 2019-12-23T05:19:04Z

@RalfJung: I've modified xargo-check to support running in a directory with only an Xargo.toml. I added an additional test to verify that this works correctly.

tests/smoke.rs

RalfJung · 2019-12-24T12:42:05Z

I checked this locally and xargo-check now looks like this:

$ rm ~/.xargo/ -rf && xargo-check
   Compiling compiler_builtins v0.1.22
   Compiling cc v1.0.47
    Checking core v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore)
   Compiling build_helper v0.1.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/build_helper)
   Compiling libc v0.2.64
   Compiling autocfg v0.1.6
   Compiling std v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd)
   Compiling cmake v0.1.38
   Compiling hashbrown v0.6.2
   Compiling unwind v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libunwind)
   Compiling backtrace-sys v0.1.32
   Compiling rustc_lsan v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/librustc_lsan)
   Compiling rustc_asan v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/librustc_asan)
   Compiling rustc_tsan v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/librustc_tsan)
   Compiling rustc_msan v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/librustc_msan)
    Checking rustc-std-workspace-core v1.99.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/tools/rustc-std-workspace-core)
    Checking alloc v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/liballoc)
    Checking cfg-if v0.1.8
    Checking rustc-demangle v0.1.16
    Checking panic_abort v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libpanic_abort)
    Checking backtrace v0.3.40
    Checking rustc-std-workspace-alloc v1.99.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/tools/rustc-std-workspace-alloc)
    Checking panic_unwind v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libpanic_unwind)
    Finished release [optimized] target(s) in 17.09s
    Checking rustc-std-workspace-std v1.99.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/tools/rustc-std-workspace-std)
    Checking term v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libterm)
    Checking proc_macro v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libproc_macro)
    Checking unicode-width v0.1.6
    Checking getopts v0.2.21
    Checking test v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libtest)
    Finished release [optimized] target(s) in 2.14s
Rust's package manager

USAGE:
    cargo [OPTIONS] [SUBCOMMAND]

OPTIONS:
    -V, --version           Print version info and exit
        --list              List installed commands
        --explain <CODE>    Run `rustc --explain CODE`
    -v, --verbose           Use verbose output (-vv very verbose/build.rs output)
    -q, --quiet             No output printed to stdout
        --color <WHEN>      Coloring: auto, always, never
        --frozen            Require Cargo.lock and cache are up to date
        --locked            Require Cargo.lock is up to date
        --offline           Run without accessing the network
    -Z <FLAG>...            Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details
    -h, --help              Prints help information

Some common cargo commands are (see all commands with --list):
    build       Compile the current package
    check       Analyze the current package and report errors, but don't build object files
    clean       Remove the target directory
    doc         Build this package's and its dependencies' documentation
    new         Create a new cargo package
    init        Create a new cargo package in an existing directory
    run         Run a binary or example of the local package
    test        Run the tests
    bench       Run the benchmarks
    update      Update dependencies listed in Cargo.lock
    search      Search registry for crates
    publish     Package and upload this package to the registry
    install     Install a Rust binary. Default location is $HOME/.cargo/bin
    uninstall   Uninstall a Rust binary

See 'cargo help <command>' for more information on a specific command.

Given taht building just libstd is now an intended usecase, I don't think it is a good idea to show the cargo help screen there. xargo should just not invoke local cargo at all when there is no Cargo.toml.

Aaron1011 · 2019-12-28T05:15:10Z

@RalfJung: Updated

src/sysroot.rs

RalfJung · 2019-12-28T12:43:27Z

@Aaron1011 thanks, that looks good! There are still two open comments from before though.

Aaron1011 · 2019-12-29T00:05:21Z

@RalfJung: I've addressed your comments

RalfJung · 2019-12-29T09:24:56Z

Thanks!

bors r+

267: Add `xargo-check` command r=RalfJung a=Aaron1011 This allows configuring Xargo to run `cargo check` instead of `cargo build` Needed to support rust-lang/miri#1048 Co-authored-by: Aaron Hill <aa1ronham@gmail.com>

bors · 2019-12-29T09:39:12Z

Build failed

continuous-integration/travis-ci/push

RalfJung · 2019-12-29T10:15:15Z

Dang, CI fails because default-run was not stable yet in nightly-2018-12-01 (which we test for backwards compatibility).

CI itself doesn't need that feature, right? So we could sed-patch Cargo.toml to make it compatible with the old nightly on CI? That's quite hacky, but I cannot think of anything more reasonable right now.

Aaron1011 · 2019-12-29T20:56:42Z

@RalfJung: I've added the hack

RalfJung · 2019-12-30T01:07:13Z

bors r+

267: Add `xargo-check` command r=RalfJung a=Aaron1011 This allows configuring Xargo to run `cargo check` instead of `cargo build` Needed to support rust-lang/miri#1048 Co-authored-by: Aaron Hill <aa1ronham@gmail.com>

bors · 2019-12-30T01:21:51Z

Build failed

continuous-integration/travis-ci/push

RalfJung · 2019-12-30T11:17:22Z

Looks like that sed script doesn't work on macOS :(

jethrogb · 2019-12-30T11:49:53Z

https://stackoverflow.com/questions/4247068/sed-command-with-i-option-failing-on-mac-but-works-on-linux

Aaron1011 · 2019-12-31T04:06:34Z

@RalfJung: I've now just using a plain sed invocation, along with mv

jethrogb · 2019-12-31T09:58:24Z

bors r=RalfJung

267: Add `xargo-check` command r=RalfJung a=Aaron1011 This allows configuring Xargo to run `cargo check` instead of `cargo build` Needed to support rust-lang/miri#1048 Co-authored-by: Aaron Hill <aa1ronham@gmail.com>

bors · 2019-12-31T10:13:36Z

Build succeeded

And happy new year from bors! 🎉

continuous-integration/travis-ci/push

Aaron1011 mentioned this pull request Nov 14, 2019

Rustup for panic changes rust-lang/miri#1048

Closed

RalfJung reviewed Nov 15, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

RalfJung reviewed Nov 15, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

RalfJung mentioned this pull request Nov 17, 2019

Build dependencies without codegen ("cargo check") where possible rust-lang/miri#1057

Closed

Aaron1011 force-pushed the feature/cargo-mode branch from 1bc689f to 31e02f1 Compare November 28, 2019 19:52

Aaron1011 changed the title ~~Add cargo_mode option to Xargo.toml~~ Add xargo-check command Nov 28, 2019