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

Regression in cargo 1.26.0-nightly 2018-04-04: [target.'cfg(FLAG)'.dependencies] does not work anymore #5313

Closed
PaulGrandperrin opened this issue Apr 7, 2018 · 10 comments · Fixed by #5389

Comments

@PaulGrandperrin
Copy link

The new revision of cargo included in rust nightly has a regression.

cargo 1.26.0-nightly (b70ab13b3 2018-04-04)
release: 1.26.0
commit-hash: b70ab13b31628e91b05961d55c07abf20ad49de6
commit-date: 2018-04-04

My crate honggfuzz-rs is tested on travis daily and today the compilation failed even though there wasn't any commit:
https://travis-ci.org/rust-fuzz/honggfuzz-rs/builds/363265565

I suspect the regression comes from this line in my cargo.toml:

[target.'cfg(fuzzing_debug)'.dependencies]
memmap = "0.6"

Code including memmap is only compiled when building with --cfg fuzzing_debug and until now, this crate was accordingly also only included when building with this flag.

It seems that this dependency is not understood anymore on the latest nightly.

I guess very few people/crates will be affected by this change/regression. For the record, I just want to state that I really don't care if it is decided that this functionality is not supported anymore.

@alexcrichton
Copy link
Member

Thanks for the report! Would it be possible to minimize this a bit to remove some of the intermediate dependencies. A simple test locally shows that this didn't trivially regress at least, but I may be missing something!

@PaulGrandperrin
Copy link
Author

Yes of course! I thought it was as simple as it looked but if not, I'll look into it. I'll report back with my findings.

@lzybkr
Copy link

lzybkr commented Apr 18, 2018

@alexcrichton - I think I'm seeing the same thing, simple repro on Windows here, just move .cargo_config to .cargo/config and src_main.rs to src/main.rs.

Build with: cargo build --features natvis
Expected - linker errors complaining that the natvis file is missing.
Actual - no linker errors.

(My scenario is the opposite - the file isn't missing, but it isn't getting included in the pdb.)

Change .cargo/config to use [target.x86_64-pc-windows-msvc] instead of [target.'cfg(feature = "natvis")'] and you should get the error assuming you're using that target of course.

I think I see this in 1.26.0-nightly (e5277c145 2018-03-28) and 1.25.0 (84203cac6 2018-03-25)

@matklad
Copy link
Member

matklad commented Apr 18, 2018

Will look into this today!

@matklad
Copy link
Member

matklad commented Apr 18, 2018

@lzybkr are you sure it is a regression? It looks like features are not supported in .cargo/config even on stable?

@matklad
Copy link
Member

matklad commented Apr 18, 2018

Reproduced

λ ls --tree
drwxr-xr-x    - matklad 18 Apr 16:50 .
.rw-r--r-- 1.9k matklad 18 Apr 16:49 ├── Cargo.lock
.rw-r--r--  172 matklad 18 Apr 16:49 ├── Cargo.toml
drwxr-xr-x    - matklad 18 Apr 16:49 └── src
.rw-r--r--   89 matklad 18 Apr 16:49    └── main.rs

~/projects/honggfuzz-rs/foo master* ⇣
λ cat Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = ["Aleksey Kladov <aleksey.kladov@gmail.com>"]

[dependencies]

[target.'cfg(fuzzing_debug)'.dependencies]
memmap = "0.6"

~/projects/honggfuzz-rs/foo master* ⇣
λ cat src/main.rs
#[cfg(fuzzing_debug)]
extern crate memmap;

fn main() {
    println!("Hello, world!");
}

~/projects/honggfuzz-rs/foo master* ⇣
λ RUSTFLAGS="--cfg fuzzing_debug" cargo build +nightly --target x86_64-unknown-linux-gnu
   Compiling foo v0.1.0 (file:///home/matklad/projects/honggfuzz-rs/foo)
error[E0463]: can't find crate for `memmap`
 --> src/main.rs:2:1
  |
2 | extern crate memmap;
  | ^^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `foo`.

To learn more, run the command again with --verbose.

~/projects/honggfuzz-rs/foo master* ⇣
λ RUSTFLAGS="--cfg fuzzing_debug" cargo +nightly build

   Compiling libc v0.2.40
   Compiling memmap v0.6.2
   Compiling foo v0.1.0 (file:///home/matklad/projects/honggfuzz-rs/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 1.47 secs

~/projects/honggfuzz-rs/foo master* ⇣
λ RUSTFLAGS="--cfg fuzzing_debug" cargo +beta build --target x86_64-unknown-linux-gnu
   Compiling libc v0.2.40
   Compiling memmap v0.6.2
   Compiling foo v0.1.0 (file:///home/matklad/projects/honggfuzz-rs/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 1.29 secs

@matklad
Copy link
Member

matklad commented Apr 18, 2018

Ok, I believe this is caused by: #5249. Even if host == target, they differ in RUSTFLAGS (because only target gets them), so that optimization is incorrect. We can safely revert it though, because we now cache rustc anyway.

@lzybkr
Copy link

lzybkr commented Apr 18, 2018

@matklad - I was doing something new, it didn't work, and I found this issue when looking to see if the issue was known, so it might not be a regression.

I was just looking for any way to pass a linker flag (not a path or lib) to a specific invocation of the compiler. RUSTFLAGS or specifying the target triple aren't working for me when compiling the dependencies in my crate.

@alexcrichton
Copy link
Member

Oops sorry about that! I agree with @matklad though that it should be safe to revert now. @matklad would you like to do that or should I?

bors added a commit that referenced this issue Apr 18, 2018
One hard cs problem

Closes #5313

r? @alexcrichton

Note that, due to the first commit, this still gives us all the benefits of #5249: if RUSTFLAGS is empty, we will run only a single rustc process, even if we can't cache it across different cargo invocations.
@PaulGrandperrin
Copy link
Author

I can confirm that it's indeed fixed.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants