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

Bootstrap panics if a crate is overloaded to one outside the local tree #78617

Closed
IsaacWoods opened this issue Nov 1, 2020 · 8 comments · Fixed by #78709
Closed

Bootstrap panics if a crate is overloaded to one outside the local tree #78617

IsaacWoods opened this issue Nov 1, 2020 · 8 comments · Fixed by #78709
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@IsaacWoods
Copy link
Contributor

This was triggered by overloading a crate in std's Cargo.toml:

libc = { path = "/hdd/libc", default-features = false, features = ['rustc-dep-of-std'] }
# libc = { version = "0.2.79", default-features = false, features = ['rustc-dep-of-std'] }

Causing the following panic:

$ ./x.py build --stage 1 library/std
Updating only changed submodules
Submodules updated in 0.05 seconds
    Updating crates.io index
    Finished dev [unoptimized + debuginfo] target(s) in 1.78s
thread 'main' panicked at 'no entry found for key', src/bootstrap/lib.rs:1114:26
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /hdd/rust/build/bootstrap/debug/bootstrap build --stage 1 library/std
Build completed unsuccessfully in 0:00:02
@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Nov 1, 2020
@bjorn3
Copy link
Member

bjorn3 commented Nov 1, 2020

This happens in builder.all_krates. It thinks that the crate is a local crate part of the source tree and then panics when it isn't part of the workspace while trying to get the the path to it.

@ehuss
Copy link
Contributor

ehuss commented Nov 3, 2020

I'm unable to reproduce this, can you provide more details on exactly what changes were made? Which version of libc were you pointing to? Which commit of the rust repo are you on?

FWIW, modifying a single Cargo.toml to point to a local path dependency can cause problems and is not the best way to go about using a local copy. Usually you want to specify a patch in the root Cargo.toml.

@bjorn3
Copy link
Member

bjorn3 commented Nov 3, 2020

@ehuss
Copy link
Contributor

ehuss commented Nov 3, 2020

@bjorn3 I can't see those links for some reason. GitHub is saying "There isn’t anything to compare. We couldn’t figure out how to compare these references, do they point to valid commits?"

For example, the following works for me on latest master (8e8939b):

diff --git a/Cargo.toml b/Cargo.toml
index c27e5c469cf..f1430e2b1c9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -89,6 +89,8 @@ rustfmt-nightly = { path = "src/tools/rustfmt" }
 # here
 rustc-workspace-hack = { path = 'src/tools/rustc-workspace-hack' }

+libc = {path="libc-0.2.79"}
+
 # See comments in `library/rustc-std-workspace-core/README.md` for what's going on
 # here
 rustc-std-workspace-core = { path = 'library/rustc-std-workspace-core' }

@bjorn3
Copy link
Member

bjorn3 commented Nov 3, 2020

Github somehow removed the # from the urls. The correct urls are:

https://github.com/softdevteam/ykrustc/compare/a8d6da3...c28cab6#diff-b46049dee2a9f04cac5ccc2e6820ecf7ef17d996c6f7258c85759cd48937b1f9R153
https://github.com/softdevteam/ykrustc/compare/a8d6da3...c28cab6#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542R110

Try moving libc-0.2.79 to the parent dir. Otherwise it is part of the rust workspace.

@ehuss
Copy link
Contributor

ehuss commented Nov 3, 2020

I see, I forgot that "path deps are workspace members" does not work if it is outside of the workspace root.

Regardless, it is still probably best in this particular case of libc to use a [patch].

BTW, those links still don't seem to work. I just see " This comparison is big! We’re only showing the most recent 250 commits", and a long list of commits.

I posted with a fix at #78709.

@bjorn3
Copy link
Member

bjorn3 commented Nov 3, 2020

Seems like github doesn't switch to the "files changed" tab automatically. Anyway here is a copy:

# When developing Yorick, you may find that you need to override ykpack. You
# can do it once here, instead of all over the place, by uncommenting the
# following two lines.
#[patch."https://github.com/softdevteam/yk"]
#ykpack = { path = "../yk/ykpack" }
@@ -150,6 +150,10 @@ pub mod util;
 #[cfg(windows)]
 mod job;

+/// During development of Yorick, we override dependencies to local paths, and this confuses the
+/// bootstrapper into believing that our code is in-tree. It's not!
+const NOT_IN_TREE: [&str; 1] = ["ykpack"];
 #[cfg(all(unix, not(target_os = "haiku")))]
 mod job {
     pub unsafe fn setup(build: &mut crate::Build) {
@@ -1105,6 +1109,9 @@ impl Build {
         let mut list = vec![INTERNER.intern_str(root)];
         let mut visited = HashSet::new();
         while let Some(krate) = list.pop() {
+            if NOT_IN_TREE.contains(&&*krate) {
+                continue;
+            }
             let krate = &self.crates[&krate];
             ret.push(krate);
             for dep in &krate.deps {

@IsaacWoods
Copy link
Contributor Author

IsaacWoods commented Nov 3, 2020

Thanks @ehuss and @bjorn3 for tracking this down. This was just with the latest masters of both repos at the time, specifically rust 2e8a54af60df63034e41359acfc923e5c5769a91 and libc 4c0a7e5b7ca949088a0ba17784fb043a22e4537e. Maybe I didn't match the versions carefully enough or something, but building the patched libstd out-of-tree with -Zbuild-std does work correctly.

FWIW, I also tried with a [patch.crates-io], which gave the same error. I can give more details if this changes things.

@bors bors closed this as completed in a65507b Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants