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

cargo build sometimes causes overrides to be built when it is already built #1215

Closed
andrewrk opened this issue Jan 24, 2015 · 37 comments
Closed
Labels
P-high Priority: High

Comments

@andrewrk
Copy link

  1. have a rust project with cargo
  2. register an override in ~/.cargo/config that the project depends on
  3. run cargo build a bunch of times. sometimes it unnecessarily rebuilds the overridden dependency.

This can be a serious issue if the compile time for the dependency is long.

@alexcrichton
Copy link
Member

Do you have an example override I could peek at? I haven't personally seen this crop up yet.

@andrewrk
Copy link
Author

my ~/.cargo/config looked like this:

paths = [ "/home/andy/dev/glium", "/home/andy/dev/glutin" ]

Projects in question:
https://github.com/tomaka/glium
https://github.com/tomaka/glutin

Root project that depended on the other ones:
https://github.com/andrewrk/genesis

andy@andy-bx:~$ rustc --version
rustc 1.0.0-nightly (4874ca36f 2015-01-23 00:18:57 +0000)
andy@andy-bx:~$ cargo --version
cargo 0.0.1-pre-nightly (3533ff1 2015-01-20 17:24:32 +0000)
andy@andy-bx:~$ uname -a
Linux andy-bx 3.16.0-25-generic #33-Ubuntu SMP Tue Nov 4 12:06:54 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
andy@andy-bx:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.10
Release:    14.10
Codename:   utopic

@alexcrichton
Copy link
Member

Can you also try running a build with RUST_LOG=cargo::ops::cargo_rustc::fingerprint and gist the output? The relevant logs would be the one where it's not supposed to rebuild but it does.

@Byron
Copy link
Member

Byron commented Feb 24, 2015

Please let me step in, as I am experiencing the same right now. It's new to me, must have sneaked in recently.

Meta

rustc --version --verbose
rustc 1.0.0-nightly (522d09dfe 2015-02-19) (built 2015-02-19)
binary: rustc
commit-hash: 522d09dfecbeca1595f25ac58c6d0178bbd21d7d
commit-date: 2015-02-19
build-date: 2015-02-19
host: x86_64-apple-darwin
release: 1.0.0-nightly
cargo --version --verbose
cargo 0.0.1-pre-nightly (43755c0 2015-02-19) (built 2015-02-19)

@alexcrichton
Copy link
Member

@Byron do you have a series of steps which reproduces this behavior? Unfortunately I'm not sure that the logs are enough to help diagnose this right now :(

@Byron
Copy link
Member

Byron commented Feb 24, 2015

@alexcrichton I will try to cook something up - you should have it in an hour or so, provided reproducing this works out for me.

@Byron
Copy link
Member

Byron commented Feb 24, 2015

mkdir tmp && cd tmp
git clone https://github.com/hyperium/hyper
git clone https://github.com/Byron/yup-oauth2
# Make sure that the full path to 'hyper' is in your ~/.cargo/config paths array
cd yup-oauth2

# build tests ... 
cargo test
# This might not rebuild
cargo test
# keep calling, it won't be long until it rebuilds hyper
...

As I used the steps above to reproduce the issue, I am confident you will manage to do the same.

@alexcrichton
Copy link
Member

Aha, I have reproduced! I am still baffled, but I'm at least a little closer!

@Byron
Copy link
Member

Byron commented Feb 25, 2015

Terrific ! Good luck hunting that thing down mercilessly !

@alexcrichton
Copy link
Member

Hm ok, this is definitely quite odd. In @bryon's example it looks like hyper was switched from rustc-serialize 0.2.15 to 0.3.0 when it was recompiled (between the second and third runs). I'm... not quite sure why!

@Byron I can't seem to reproduce today (the compiler has changed), can you still reproduce this? Some other helpful logs would be cargo::core::resolver and perhaps cargo::core::registry as well (feel free to just turn all logs on and gist everything).

@Byron
Copy link
Member

Byron commented Mar 3, 2015

Me neither (even with an updated example, as the yup repository changed) !
Cargo is now updating the index everytime I run cargo test, which also didn't happen before, I think. Maybe it's related ?
BTW: I am still on rustc 1.0.0-nightly (4db0b3246 2015-02-25) (built 2015-02-25)

@Byron
Copy link
Member

Byron commented May 11, 2015

This is happening to me again, since upgrading to version rustc 1.1.0-nightly (7bd71637c 2015-05-06) (built 2015-05-05) with the accompanying cargo.
The same issue occurs when using rustc 1.1.0-nightly (dc630d01e 2015-05-09) (built 2015-05-09) with related cargo.

It may be worth noting that the main difference is that cargo will always recompile the program's main-dependency, as well as the program itself, for example:

➜  google-apis-rs git:(master) ✗ make discovery1-cli-cargo ARGS=build
cd gen/discovery1-cli && cargo build
   Compiling google-discovery1 v0.1.7+00000000 (file:///Users/byron/Documents/dev/rust/google-apis-rs/gen/discovery1-cli)
   Compiling google-discovery1-cli v0.2.0+00000000 (file:///Users/byron/Documents/dev/rust/google-apis-rs/gen/discovery1-cli)

# Now running it again will perform the same build, even though nothing changed
➜  google-apis-rs git:(master) ✗ make discovery1-cli-cargo ARGS=build
cd gen/discovery1-cli && cargo build
   Compiling google-discovery1 v0.1.7+00000000 (file:///Users/byron/Documents/dev/rust/google-apis-rs/gen/discovery1-cli)
   Compiling google-discovery1-cli v0.2.0+00000000 (file:///Users/byron/Documents/dev/rust/google-apis-rs/gen/discovery1-cli)

Thus, it is not happening for overrides, but for the entire program. In my particular case this causes enormous build overheads, as I deal with 78 programs that now want to recompile all the time.

@Byron
Copy link
Member

Byron commented May 12, 2015

I think the key difference to other projects is that, in this case, google-discovery1 is setup as dependent project of google-discovery1-cli using the path=../discovery1 . Note the relative path, which could have something to do with it.

As this might be unrelated, I created a new issue.

@alexcrichton alexcrichton added the P-high Priority: High label May 18, 2015
@ArtemGr
Copy link

ArtemGr commented Jun 1, 2015

I noticed that dependencies fetched from crates.io or github are cached, but any local dependencies aren't, regardless of whether the path to a local dependency is relative or not.

Now, what's interesting is that for remore dependencies the fingerprint file seems okay (http://pastebin.com/dFEUvQVs) while for the local dependencies it contains some gibberish on every line (http://pastebin.com/p0rnm0KG)!

cargo 0.3.0-nightly (06dbe65 2015-05-29) (built 2015-05-31)

@alexcrichton
Copy link
Member

@ArtemGr hm that may actually be a different issue, a dependency on something like <quote\ expansion typically means that a syntax extension is mistakenly adding the dependency on some weird file. Do you have any syntax extensions in play?

@ArtemGr
Copy link

ArtemGr commented Jun 1, 2015

Only #![plugin(serde_macros)].
Might be a different issue but I'm definitely seeing those superfluous recompilations happening.

@alexcrichton
Copy link
Member

Do you have a project I could take a peek at?

@ArtemGr
Copy link

ArtemGr commented Jun 2, 2015

Yeah, I can upload it but I'm not sure there's a point. The yup-oauth2 mentioned above have a similar issue and you can observe a similar fingerprint gibberish:

root@boot2docker(Docker):/tmp/yup-oauth2/target/debug/.fingerprint/yup-oauth2-f648bac97266aa1c$ cat dep-lib-yup-oauth2
/tmp/yup-oauth2/tmp/yup-oauth2/target/debug/libyup_oauth2.rlib: src/lib.rs src/device.rs src/refresh.rs src/common.rs src/helper.rs <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion

/tmp/yup-oauth2/target/debug/yup_oauth2.d: src/lib.rs src/device.rs src/refresh.rs src/common.rs src/helper.rs <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion <quote\ expansion

If yup-oauth2 is fixed but I still has a problem then I'll make a separate test case. That all right with you?

Note that I'm using an older rustc nightly (rustc 1.2.0-nightly (2e7d7bc05 2015-05-18) (built 2015-05-19)) because there's a problem using serde_macros from the latest one (serde-deprecated/aster#12).

@alexcrichton
Copy link
Member

Hm that looks like a bug in quasi as that string appears to be leaking into the source map.

@alexcrichton
Copy link
Member

@ArtemGr could you also gist the logs of RUST_LOG=cargo::ops::cargo_rustc::fingerprint?

@alexcrichton
Copy link
Member

Aha, then it definitely looks like your bug is a dupe of serde-rs/serde#85.

It looks like @Byron's logs don't indicate that bug, however, so I'm still at a loss for what this is.

@ArtemGr
Copy link

ArtemGr commented Jun 2, 2015

Thanks! Hopefully fixing the Serde's bug will make it easier to triage the Byron's issue.

@ArtemGr
Copy link

ArtemGr commented Jun 14, 2015

If I were to patch a Cargo fork with a temporary workaround in order to filter the bogus dependencies, where in the Cargo should I look to do that?

@alexcrichton
Copy link
Member

Most of the logic is in this file, but you'd specifically want to take a look at this loop

@ArtemGr
Copy link

ArtemGr commented Jun 14, 2015

Much appreciated!

@gustavla
Copy link

I'm experiencing this problem as well, so I thought I would add a sample point. I have two crates A and B and B depends on A. If I start an empty project and add either A or B as the only dependency - no problem. However, when I add both, the behavior of random rebuilds starts happening.

I am experiencing this specifically when A is image (0.3.11) and B is piston2d-opengl_graphics (0.1.0).

@alexcrichton
Copy link
Member

@gustavla could you try out a Cargo build with #1714? That may actually fix this bug here, but it's very difficult to tell.

Also, is the "small test case" for you just a blank project with those two dependencies?

@gustavla
Copy link

Yup, the small test case was completely blank with these lines added to Cargo.toml:

[dependencies]
image = "0.3.11"
piston2d-opengl_graphics = "0.1.0"

However, as per your suggestion, I changed this to:

[dependencies]
piston2d-opengl_graphics = "0.1.0"

[dependencies.image]
version = "0.3.11"
default-features = false

This fixes the problem. The fix holds up in my actual project too, not just the distilled test case. Thanks!

@gustavla
Copy link

@alexcrichton Actually, my original test case seems to be resolved simply with more recent versions of Cargo (a bisect pointed to 40d1c10 and #1567). Sorry about that, updating Cargo should have been the first thing I did.

@alexcrichton
Copy link
Member

Ah no worries, glad it's fixed!

@ArtemGr
Copy link

ArtemGr commented Jun 19, 2015

So here's - https://github.com/ArtemGr/cargo/commit/f56369511c2737c5c18a63e382f5bbbae88f8de4 - a patch that fixes it for me. Proves that in my case serde-deprecated/quasi#7 is the culprit.

I wonder if we should ignore a dependency if it does not exists?
What is the point of accounting for non-existing dependencies?
In makefiles I sometimes use files as build completion flags, but was anybody doing that with cargo?

@alexcrichton
Copy link
Member

I wonder if we should ignore a dependency if it does not exists?

This means that a source file may have been deleted, in which case we need to re-run the compiler to really make sure that it is in fact not needed.

Byron referenced this issue Jun 21, 2015
This commit is an architectural change inside of Cargo itself in the way that it
handles the output format of builds. Previously when a build start, all existing
directories and files would be renamed to `old-foo` folders. The build would
then `rename` all files back into the right location as they were seen as fresh
and needed for the build.

The benefit of a system such as this is a rock-solid guarantee that the build
tree contains exactly what it would if we were to start the build from a totally
clean directory each time. There are some downsides, however:

* In #800, it was discovered that this method has an unfortunate interaction
  with Docker. Docker apparently will mount many filesystems which `rename` will
  not work across.

* I have seen countless flaky failures on windows due to an attempt to remove a
  file that was still in use somehow. I've never been able to truly track down
  why these failures are happening, however.

The new system for managing output files is to build up a list of all known
files at the start of a build, whitelist any necessary files when the build is
being prepared, and then wipe out all unknown files right before the build
begins. This is not quite as close to the guarantee as the benefits reaped
before because on the second build all build files will still be in their final
output locations, they may just get updated as part of the build as well. This
seems like an acceptable compromise, however.

Closes #800
@Byron
Copy link
Member

Byron commented Jun 21, 2015

In my particular situation I am building multiple projects in parallel into the same target-dir. This works fine, until cargo thinks it has to rebuild a perfectly up-to-date dependency, and deletes it, which in turn causes the other build processes to fail. The last time this happened, it looked like this.

Cargo really wants to rebuild items it has just build, including clap-rs which is the cause of the failure in the referenced gist. Whenever cargo needs a rebuild, it will for good reason remove the existing artifacts.
It's worth noting that serde is involved, even though it is just used as build script this time, generating new code which is included into the actual library file, as described here.

Unfortunately, the issue faded away before I could have a look at the contents of the fingerprints file.

How to reproduce

Even though the following statements would get you close to my setup, it differs as it will also use rustc. In my local setup, I use multirust with the stable toolchain, but manually symlinked cargo from nightly into the stable toolchain.

Also when trying these statements myself, I was unable to reproduce the issue, which might possibly hint at rustc being involved in some way.

$ git clone https://github.com/Byron/google-apis-rs
$ cd google-apis-rs
# need nightly to get latest cargo - rustc stable would be the default way to compile google.rs
$ multirust override nightly
# build the first library without any make-controlled parallelism to get all dependencies
$ make discovery1-cli-cargo ARGS=build
# now build APIs and CLIs in parallel. It takes about 30m to show the issue presented in the gist.
# I wonder why something is deleted all of the sudden ... .
$ time make cargo-cli ARGS=build -j8

@alexcrichton
Copy link
Member

@Byron there are many reasons why running cargo concurrently on the same system will cause problems, so that's definitely not expected to work (see #354)

@Byron
Copy link
Member

Byron commented Jun 22, 2015

Thanks for the heads-up - I will post some thoughts in the referenced issue.

@alexcrichton
Copy link
Member

I have a feeling that this is probably a dupe of #2041, so I'm going to close in favor of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high Priority: High
Projects
None yet
Development

No branches or pull requests

5 participants