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

Stack overflow on virtual project with patched dependencies which point at each other #7163

Closed
handicraftsman opened this issue Jul 23, 2019 · 7 comments · Fixed by #7174
Closed
Labels
C-bug Category: bug

Comments

@handicraftsman
Copy link

Literally this

handicraftsman@NickolayPC:~/Projects/airkit$ cargo build --verbose

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted

Steps

  1. Create a root project Cargo.toml with [workspace] section and [patch.crates-io] section
[workspace]
members = [
    "auragfx",
    "auragfx-backend-glutin"
]

[patch.crates-io]
auragfx = { path = "./auragfx" }
auragfx-backend-glutin = { path = "./auragfx-backend-glutin" }
  1. Create other two manifests
[package]
name = "auragfx"
version = "0.1.0"
authors = ["handicraftsman <nickolay02@inbox.ru>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[features]
default = ["backend-glutin"]

backend-glutin = ["auragfx-backend-glutin"]

[dependencies]
spirv_cross = "0.15.0"
shaderc = "0.6.0"

auragfx-backend-glutin = { version = "0.1.0", optional = true }
[package]
name = "auragfx-backend-glutin"
version = "0.1.0"
authors = ["handicraftsman <nickolay02@inbox.ru>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
auragfx = "0.1.0"
glutin = "0.21.0"
gl = "0.13.0"
  1. ???

Notes

Output of cargo version:

cargo 1.38.0-nightly (e3563dbdc 2019-07-16)
@handicraftsman handicraftsman added the C-bug Category: bug label Jul 23, 2019
@ehuss
Copy link
Contributor

ehuss commented Jul 23, 2019

I've narrowed this down to a test case:

#[cargo_test]
fn cycle() {
    Package::new("a", "1.0.0").publish();
    Package::new("b", "1.0.0").publish();
    let p = project()
        .file("Cargo.toml",
            r#"
            [workspace]
            members = ["a", "b"]

            [patch.crates-io]
            a = {path="a"}
            b = {path="b"}
            "#)
        .file("a/Cargo.toml",
            r#"
            [package]
            name = "a"
            version = "1.0.0"

            [dependencies]
            b = "1.0"
            "#)
        .file("a/src/lib.rs", "")
        .file("b/Cargo.toml",
            r#"
            [package]
            name = "b"
            version = "1.0.0"

            [dependencies]
            a = "1.0"
            "#)
        .file("b/src/lib.rs", "")
        .build();

    p.cargo("check").run();
}

This gets caught in a cycle in metadata_of.

@Eh2406 or @alexcrichton, I could have sworn the resolver had some cycle detection, but it doesn't seem to trigger in this case with [patch]. I don't think I know enough about the resolver to investigate more.

EDIT: The Package::new at the top isn't necessary. It can also be crate::support::registry::init();.

@Eh2406
Copy link
Contributor

Eh2406 commented Jul 23, 2019

Thanks @ehuss, I have not looked into this yet but here are some links.
The cycle detection is check_cycles.
I would guess that it is related to #7118.

@handicraftsman
Copy link
Author

Well, i understand what needs to be done to resolve this for me and what is wrong in project layout. But yeah, not giving a proper error is definitely user friendly 😸

@handicraftsman handicraftsman changed the title Stack overflow on virtual project with patched dependencies Stack overflow on virtual project with patched dependencies which point at each other Jul 23, 2019
@ehuss
Copy link
Contributor

ehuss commented Jul 23, 2019

@handicraftsman You can't have a cycle in your dependency graph. You'll need to restructure so that auragfx and auragfx-backend-glutin don't depend on one another.

@handicraftsman
Copy link
Author

handicraftsman commented Jul 23, 2019

Yeah, already noticed that as i've said above. Already done.

@ehuss
Copy link
Contributor

ehuss commented Jul 23, 2019

Yeah, already noticed that as i've said above. Already done.

Oh, haha, I misread. 😄

@alexcrichton
Copy link
Member

I believe the bug is this line which returns a false negative for matches_id when using [patch]. Instead that code should be using something like deps where it returns (&PackageId, &[Dependency]) only we'll need to update the implementation of Resolve::deps_not_replaced to return this (basically be the same-ish implementation of Resolve::deps

alexcrichton added a commit to alexcrichton/cargo that referenced this issue Jul 24, 2019
This commit fixes detection of cyclic dependencies through the use of
`[patch]` by ensuring that `matches_id` isn't used because it returns a
false negative for registry dependencies when the dependency
specifications don't match but the resolve edges are still correct.

Closes rust-lang#7163
bors added a commit that referenced this issue Jul 24, 2019
Fix detection of cyclic dependencies through `[patch]`

This commit fixes detection of cyclic dependencies through the use of
`[patch]` by ensuring that `matches_id` isn't used because it returns a
false negative for registry dependencies when the dependency
specifications don't match but the resolve edges are still correct.

Closes #7163
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants