AtomBuild / atom-build-cargo

Cargo (Rust) integration for Atom
MIT License
18 stars 6 forks source link

Building triggers all dependencies to be rebuilt when building from the terminal #65

Closed randomPoison closed 7 years ago

randomPoison commented 8 years ago

When I use atom-build-cargo to build my project within Atom it rebuilds all dependencies the first time, regardless of whether I've previous built from the terminal. After building from Atom, the next time I build from the terminal all dependencies get rebuilt. When I then go back to Atom and build all dependencies are rebuilt again. If I build multiple times in a row from just the terminal or just from Atom dependencies are only rebuilt the first time and then never again (unless I touch one of the files in a dependency's crate).

It would appear that building in Atom invalidates the build artifacts from terminal builds, and vice versa.

Expected Behavior

Builds from within Atom and builds from the terminal work with the same asset cache, so building dependencies from one builds for the other. Or, at the very least, building from one does not cause all dependencies to be rebuilt for the other.

Platform Notes

randomPoison commented 8 years ago

After some investigation, I'm pretty sure this is caused by the inclusion of -Z unstable-options in the RUSTFLAGS environment variable when building. To test this I did the following:

  1. Ran cargo build from the terminal with no RUSTFLAGS environment variable set. The build compiled all dependencies and completed as normal.
  2. Ran cargo build again. The build was a no-op, and cargo didn't report rebuilding any dependencies.
  3. Ran set RUSTFLAGS=-Z unstable-options.
  4. Ran cargo build. Cargo rebuilt all dependencies even though no code had changed, otherwise the build completed as normal.
  5. Ran cargo build again. The build was a no-op, and cargo didn't report rebuilding any dependencies.
  6. Ran set RUSTFLAGS= to clear the RUSTFLAGS variable.
  7. Ran cargo build. All dependencies were recompiled even though no code had changed.

To mitigate this atom-build-cargo should exclude -Z unstable-options in versions where --error-format=json is stable, i.e. rustc 1.12 and later.

alygin commented 8 years ago

It's strange, I don't use JSON output in cargo-build, but I see the recompilation when switching between Atom and a terminal. I tried switching setting and unsetting various environment variables, but to no avail. So I think there's something else that causes such behaviour.

According to your description, you only used a terminal. What happens if you switch off JSON output in build-cargo and try switching between Atom and the terminal while recompiling your project?

randomPoison commented 8 years ago

It looks like it's not just -Z unstable-options, changing --error-format=json also does it. So if I disable JSON output then switching between Atom and the terminal doesn't trigger a recompile. I'm guessing that any change to RUSTFLAGS is causing cargo (or rustc) to treat previous builds as out of date. If this is a desired behavior in cargo/rustc then I don't know how atom-cargo-build can work around it.

alygin commented 8 years ago

Yes, cargo triggers recompilation on any RUSTFLAGS change which seems reasonable. But personally, I encounter the recompilation behaviour on mac even when not using JSON output. So there must be something else that triggers it.

But anyway, it's the cargo behaviour and I don't think we can (and should) do much about it in cargo-build.

alygin commented 8 years ago

And you're right, after --error-format=json becomes a stable option, we can omit -Z unstable-options when using JSON output.

randomPoison commented 8 years ago

An alternative would be to do cargo rustc -- error-format=json. This still forces the root crate to recompile, but none of the dependencies. The downside is that if any dependencies need to be updated they won't use the JSON error formatting.

randomPoison commented 8 years ago

Fun fact, AtomLinter/linter-rust#77 is seeing this same issue, and that issue point out that solving this is blocked by rust-lang/cargo#2982.

alygin commented 8 years ago

It's not clear to me from the Cargo issue discussion that fixing it will prevent calling rustc when the --error-format option is changed between calls. Though, based on how Cargo handles its options now, it's possible that it will.

Regarding to calling cargo rustc with custom arguments, I think we could add such command. But even without this option, Atom Build allows you to add custom commands. You need to create .atom-build.yml file and describe your command there. Here's the documentation on how to do that. For instance:

cmd: cargo
name: Cargo rustc
args:
  - rustc
shell: false

will add Cargo rustc command to your project.

randomPoison commented 8 years ago

In saying that this is blocked on rust-lang/cargo#2982 I figured that once cargo directly supported JSON output it could taught to treat --error-format=json as a special case compiler flag that doesn't require recompilation of all dependencies. I've opened rust-lang/cargo#3141 to see if there's any possibility of handling the error format compiler flag before it's made first-class in cargo.

In the meantime, I'll give the custom build command a shot. If I can get that working it'll at least cover my use-case for the time being.

oli-obk commented 7 years ago

it's in the latest nightly now. The only issue left is to figure out whether to simply break all old nightlies and stables or to add a dropdown menu to choose the variant to use (RUSTFLAGS, --error-format=json, or normal errors).

randomPoison commented 7 years ago

There's already some code to check the version number of rustc to see if JSON output is supported, could the same be done to see if the cargo version supports --error-format=json?

oli-obk commented 7 years ago

great idea!

antage commented 7 years ago

I have the same problem due to buildCfg.env.RUSTFLAGS = '--color=always' in build-cargo 1.2.0. cargo build from terminal doesn't use --color=always by default so I get full rebuilding.

oli-obk commented 7 years ago

we can simply pass this flag to cargo instead of rustc