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

Rust 1.18 regression, isbfc 0.0.1, igo 0.0.2 #42365

Closed
brson opened this issue Jun 1, 2017 · 9 comments
Closed

Rust 1.18 regression, isbfc 0.0.1, igo 0.0.2 #42365

brson opened this issue Jun 1, 2017 · 9 comments
Labels
P-low Low priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jun 1, 2017

A similar widespread bug was fixed previously (#41797), but I still see a regression in these two crates:

brian@ip-10-145-43-250:~/dev/isbfc⟫ cargo +beta test
   Compiling libc v0.2.20
   Compiling strsim v0.6.0
   Compiling unicode-width v0.1.4
   Compiling bitflags v0.7.0
   Compiling unicode-segmentation v1.1.0
   Compiling ansi_term v0.9.0
   Compiling vec_map v0.6.0
   Compiling term_size v0.2.3
   Compiling clap v2.20.5
   Compiling isbfc v0.0.1 (file:///mnt2/dev/isbfc)
error: couldn't read "src/bin/isbfc.rs": No such file or directory (os error 2)

error: Could not compile `isbfc`.
Build failed, waiting for other jobs to finish...
error: build failed
brian@ip-10-145-43-250:/mnt2/dev/isbfc⟫ rustc +beta -Vv
rustc 1.18.0-beta.4 (0308c9865 2017-05-27)
binary: rustc
commit-hash: 0308c986510a541a9c02bff7f4e239054fa15086
commit-date: 2017-05-27
host: x86_64-unknown-linux-gnu
release: 1.18.0-beta.4
LLVM version: 3.9
@brson brson added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Jun 1, 2017
@brson
Copy link
Contributor Author

brson commented Jun 1, 2017

cc @alexcrichton

@brson brson added the T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. label Jun 1, 2017
@brson
Copy link
Contributor Author

brson commented Jun 1, 2017

cc @ids1024

@ids1024
Copy link
Contributor

ids1024 commented Jun 1, 2017

Here is a minimal test case, with the caveat that I probably really shouldn't have organized that crate the way I did:

Cargo.toml

[package]
name = "bugtest"
version = "0.1.0"

[[bin]]
name = "bugtest"

[[bin]]
name = "bugtest2"

src/lib.rs

fn test() {
}

src/main.rs

fn main() {
}

src/bin/bugtest2.rs

fn main() {
}

Edit: This arguably shouldn't have worked in the first place.

@ids1024
Copy link
Contributor

ids1024 commented Jun 1, 2017

Anyway, the cause of the issue (and I've verified that igo is the same) is that the crate contains a library and binary with the name of the crate, as well as another binary, but also an explicit [[bin]] section for the main binary. This combination makes it not check src/main.rs but only src/bin/<cratename>.rs.

@alexcrichton
Copy link
Member

Thanks for the investigation @ids1024! This is close enough to the release that it's likely to leak into 1.18 though :(

@brson brson added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. P-low Low priority E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-needs-decision Issue: In need of a decision. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels Jun 15, 2017
@alexcrichton
Copy link
Member

cc @rust-lang/cargo, would anyone be willing to help out investigating this?

@matklad
Copy link
Member

matklad commented Jun 16, 2017

I've looked into it and it seems ... complicated. It's another fallout from rust-lang/cargo#3609 I guess, so cc @jmatraszek. Here's the test for Cargo:

#[test]
fn explicit_bins_without_paths() {
    let p = project("foo")
        .file("Cargo.toml", r#"
            [package]
            name = "foo"
            version = "0.1.0"
            authors = []

            [[bin]]
            name = "foo"

            [[bin]]
            name = "bar"
        "#)
        .file("src/lib.rs", "")
        .file("src/main.rs", "fn main() {}")
        .file("src/bin/bar.rs", "fn main() {}");

    assert_that(p.cargo_process("build"), execs().with_status(0));
}

The problem is that the logic for inferring a binary path is quite fiddly and fallbacks to a default instead of erroring, so I am not sure how to fix this particular case without breaking some other.

bors added a commit to rust-lang/cargo that referenced this issue Jun 17, 2017
Simplify inferred binary names

Fix for rust-lang/rust#42365, which probably breaks other stuff :(
@brson
Copy link
Contributor Author

brson commented Jul 13, 2017

Looks like this is fixed in cargo. We probably need to confirm this is fixed on a particular branch, and get it into the relnotes.

@Mark-Simulacrum Mark-Simulacrum removed the I-needs-decision Issue: In need of a decision. label Jul 13, 2017
@Mark-Simulacrum
Copy link
Member

Closing. I think this was fixed, we can reconsider if necessary and reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants