Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
randomPoison opened this issue Sep 30, 2016 · 14 comments · Fixed by #67 or #79
Closed

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

randomPoison opened this issue Sep 30, 2016 · 14 comments · Fixed by #67 or #79

Comments

@randomPoison
Copy link

randomPoison commented Sep 30, 2016

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

  • Windows 10
  • rustc 1.13.0-nightly (32571c05c 2016-09-17)
  • cargo 0.13.0-nightly (9399229 2016-09-14)
@randomPoison
Copy link
Author

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
Copy link
Collaborator

alygin commented Sep 30, 2016

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
Copy link
Author

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
Copy link
Collaborator

alygin commented Sep 30, 2016

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
Copy link
Collaborator

alygin commented Sep 30, 2016

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

@randomPoison
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

alygin commented Sep 30, 2016

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
Copy link
Author

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
Copy link
Contributor

oli-obk commented Nov 3, 2016

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
Copy link
Author

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
Copy link
Contributor

oli-obk commented Nov 3, 2016

great idea!

@antage
Copy link

antage commented Jan 6, 2017

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 oli-obk reopened this Jan 6, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Jan 6, 2017

we can simply pass this flag to cargo instead of rustc

oli-obk added a commit that referenced this issue Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants