Closed
Bug 1354401
Opened 8 years ago
Closed 8 years ago
Add mach check-rust command
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1378440
People
(Reporter: rillian, Assigned: rillian)
References
Details
Attachments
(2 files)
For quickly verifying in-tree rust code, and compatibility with a particular toolchain, it would be nice to have a way to invoke the `cargo check` command on all the top-level crates (library and program) in the tree. This parses all the source from each crate and its dependencies, without invoking actual code generation, so it's a quick way to check if something compiles. It also writes out a metadata file useful for the Rust Language Service lookup and completion engine.
I propose hooking this up to `mach check-rust`, `mach cargo check`, or something similar to handle distributing the invocation and invoking cargo with the correct flags for the configured build.
Assignee | ||
Comment 1•8 years ago
|
||
This can also be used in a ci task to check compatibility with minimum-supported and upcoming versions of rust, per bug 1321847.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → giles
Comment 2•8 years ago
|
||
How about a generic `mach cargo` command that invokes cargo with the given parameters in all the relevant directories?
Comment 3•8 years ago
|
||
I’ve also used `cargo check` a lot during development, to see compilation errors more quickly during a refactoring for example.
We do have `mach cargo` in Servo (so one can use `./mach cargo check`), though it only runs Cargo once. (I’ve just added `mach cargo-geckolib` in Servo, with matching differences as `mach build-geckolib` with `mach build`.)
Comment 4•8 years ago
|
||
mystor was talking about adding some Rust sources in GENERATED_FILES just the other day, so we'll want to make sure that whatever we do here works in that scenario.
Comment 5•8 years ago
|
||
This is getting off topic, but how does GENERATED_FILES compare to Rust’s pattern of having "build scripts", writing files to Cargo’s $OUT_DIR, then using `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` ? What makes one preferable over the other?
Comment 6•8 years ago
|
||
(In reply to Simon Sapin (:SimonSapin) from comment #5)
> This is getting off topic, but how does GENERATED_FILES compare to Rust’s
> pattern of having "build scripts", writing files to Cargo’s $OUT_DIR, then
> using `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` ? What makes one
> preferable over the other?
GENERATED_FILES is the tool which we use to generate .cpp files and .h files in tree. Sometimes we want to use the same logic to generate a C++ and rust file in tandem (for example if we're generating code which we want to bind to in rust-land). GENERATED_FILES means we can use the same code paths, and keep the logic together, while with build.rs we're using a completely different code path for rust code.
I could have used build.rs for the nserror bindings (bug 1320179) but I wanted the code to be generated by the same logic which is being used to generate the C++ code, so I chose to use GENERATED_FILES.
The easiest way to make sure that this works well is probably to simply make sure that we run export before we run cargo check when running mach check-rust, as .rs files are built during the export phase like .h files.
Comment 7•8 years ago
|
||
If this is simple linting and not a dispatcher around arbitrary `cargo` invocations, `mach lint` should be used, as that is where we're trying to consolidate our "do static analysis" commands.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•8 years ago
|
||
Just some patches I was playing around with.
(In reply to Mike Hommey [:glandium] from comment #2)
> How about a generic `mach cargo` command that invokes cargo with the given
> parameters in all the relevant directories?
I'm not sure how to go about that. Is there a mozbuild api I can use to query all the directories with rust code in them?
Comment 11•8 years ago
|
||
(In reply to Ralph Giles (:rillian) from comment #10)
> (In reply to Mike Hommey [:glandium] from comment #2)
>
> > How about a generic `mach cargo` command that invokes cargo with the given
> > parameters in all the relevant directories?
>
> I'm not sure how to go about that. Is there a mozbuild api I can use to
> query all the directories with rust code in them?
I don’t think running cargo multiple times is desirable if it’s with arbitrary arguments. Instead, I think it should be run once in toolkit/library/rust, and --manifest-path can be used if some other directory is desired for a given command.
Assignee | ||
Comment 12•8 years ago
|
||
Nathan did this in bug 1378440.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•