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

1.18 doesn't build with 1.17 #42543

Closed
kyrias opened this issue Jun 8, 2017 · 20 comments
Closed

1.18 doesn't build with 1.17 #42543

kyrias opened this issue Jun 8, 2017 · 20 comments
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@kyrias
Copy link
Contributor

kyrias commented Jun 8, 2017

Trying to build 1.18.0 with the Arch Linux 1.17.0 package results in the following error:

info: looks like you are running this command under `sudo`
      and so in order to preserve your $HOME this will now
      use vendored sources by default. Note that if this
      does not work you should run a normal build first
      before running a command like `sudo make install`
   Compiling rustc-serialize v0.3.23
   Compiling libc v0.2.21
   Compiling getopts v0.2.14
   Compiling gcc v0.3.45
   Compiling filetime v0.1.10
   Compiling num_cpus v0.2.13
   Compiling build_helper v0.1.0 (file:///build/rust/src/rustc-1.18.0-src/src/build_helper)
   Compiling cmake v0.1.22
   Compiling toml v0.1.30
   Compiling bootstrap v0.0.0 (file:///build/rust/src/rustc-1.18.0-src/src/bootstrap)
    Finished dev [unoptimized] target(s) in 13.49 secs


failed to execute command: "/build/rust/src/rustc-1.18.0-src/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "metadata" "--manifest-path" "/build/rust/src/rustc-1.18.0-src/src/libstd/Cargo.toml"
error: No such file or directory (os error 2)


Build completed unsuccessfully in 0:00:13

(It's built in a clean chroot.)

@sanxiyn sanxiyn added the A-build label Jun 8, 2017
@leahneukirchen
Copy link

Same on Void Linux, cargo is in $PATH and set in rustc-1.18.0-src/config.toml. This used to work fine before.

@steveklabnik steveklabnik added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jun 9, 2017
@steveklabnik
Copy link
Member

tagging with both infra and compiler because i don't know whose fault it is here.

@leahneukirchen
Copy link

src/bootstrap/bootstrap.py reads config.toml to find cargo, but src/bootstrap/lib.rs doesn't seem to... it only finds cargo in CFG_LOCAL_RUST_ROOT. Passing --enable-local-rust --local-rust-root=.../stage0 (both are needed!) adds the setting to config.mk, but then bootstrap doesn't seem to load it.

@aclemons
Copy link

Same on Slackware linux. Our build was using x.py build --verbose. Removing --verbose allows the build to succeed. Perhaps that helps to track down the root cause.

@leahneukirchen
Copy link

FWIW, we currently use this hack to make it use the cargo from $PATH, which then works fine: https://github.com/voidlinux/void-packages/blob/master/srcpkgs/rust/patches/cargo-hack.patch

@Mark-Simulacrum
Copy link
Member

I suspect at least some of the problem here is from #41779, which will hopefully be fixed soon. I plan to look into it more today, though I wasn't able to reproduce yet.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 18, 2017
…crichton

Use custom cargo/rustc paths when parsing flags.

Fixes rust-lang#41779, probably also rust-lang#42543 (I think they're duplicates).

I'm not entirely happy with the implementation, since it means we parse the configuration twice, but it's the minimal solution. I think the other choice is to move both calls to Config::parse inside Flags::parse and merge them, but I don't know if that's a good idea.

r? @alexcrichton
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 18, 2017
…crichton

Use custom cargo/rustc paths when parsing flags.

Fixes rust-lang#41779, probably also rust-lang#42543 (I think they're duplicates).

I'm not entirely happy with the implementation, since it means we parse the configuration twice, but it's the minimal solution. I think the other choice is to move both calls to Config::parse inside Flags::parse and merge them, but I don't know if that's a good idea.

r? @alexcrichton
@kyrias
Copy link
Contributor Author

kyrias commented Jun 20, 2017

1.18 doesn't build using 1.17 with #42695 applied either.

@Mark-Simulacrum
Copy link
Member

@kyrias With the same error? Are you using a custom cargo/rustc path?

@Mark-Simulacrum Mark-Simulacrum removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 20, 2017
@kyrias
Copy link
Contributor Author

kyrias commented Jun 20, 2017

Same error, yes. And building without the --verbose flag does pass let it pass that stage.

./configure \
  --prefix=/usr \
  --release-channel=stable \
  --llvm-root=/usr \
  --enable-llvm-link-shared \
  --disable-codegen-tests \
  --jemalloc-root=/usr/lib \
  --enable-local-rust
python2 ./x.py build --verbose

@infinity0
Copy link
Contributor

Same problem as @kyrias on Debian trying 1.18 using 1.17, #42695 does not fix this bug.

@infinity0
Copy link
Contributor

The problem comes from this piece of code, from metadata.rs:

fn build_krate(build: &mut Build, krate: &str) {
    // Run `cargo metadata` to figure out what crates we're testing.
    //
    // Down below we're going to call `cargo test`, but to test the right set
    // of packages we're going to have to know what `-p` arguments to pass it
    // to know what crates to test. Here we run `cargo metadata` to learn about
    // the dependency graph and what `-p` arguments there are.
    let mut cargo = Command::new(&build.cargo);
    cargo.arg("metadata")
         .arg("--manifest-path").arg(build.src.join(krate).join("Cargo.toml"));

Perhaps it is getting run before the stage0 stuff is copied.

@brson brson added P-high High priority I-nominated regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Jun 20, 2017
@Mark-Simulacrum
Copy link
Member

I know what the problem is, but I haven't figured out how to fix it yet.

@brson
Copy link
Contributor

brson commented Jun 20, 2017

Seems like our CI should be running an --enable-local-rust configuration with the prior release. A smoke test at least. Being able to bootstrap is pretty super important.

@Mark-Simulacrum
Copy link
Member

I've filed #42786 to track the general notion that bootstrapping from local rust (previous stable) should work. #42785 should fix this specific issue (and does in local tests -- my previous fix didn't handle the case where ./configure was used).

@brson
Copy link
Contributor

brson commented Jun 21, 2017

@Mark-Simulacrum does that patch backport to 1.18?

At minimum we need to provide a patch for packagers, and we should discuss whether this is worth a point release.

@Mark-Simulacrum
Copy link
Member

Yes, both #42695 and #42785 backport to beta cleanly.

@kyrias
Copy link
Contributor Author

kyrias commented Jun 21, 2017

I can confirm that backporting both does fix the build for me.

It would also be useful if we could set up a system where a few days before the actual release, what's supposed to become the release tarballs would be released to package maintainers so that we could report back if there's any fairly big problems like this, so it could either be fixed or at the very least noted clearly in the release notes. Especially since I've hit build-breaking bugs in 3 of the last 4 rust releases.

It's one of the things I rather like about the way SaltStack does releases, there's a private mailing list for package maintainers were we can test the releases first, and also make the built packages available to the SaltStack people so that they can run their tests on our built packages.

@leahneukirchen
Copy link

This does not fix the bug for me.
I have CFG_LOCAL_RUST_ROOT := /builddir/rustc-1.18.0-src/stage0, and cargo is in that directory,
yet the build tries to run /builddir/rustc-1.18.0-src/build/x86_64-unknown-linux-gnu/stage0/bin/cargo.

@kyrias
Copy link
Contributor Author

kyrias commented Jun 21, 2017

And you applied both patches?

@leahneukirchen
Copy link

Duh, missed the first one. Works fine! 👍

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 22, 2017
…ap, r=alexcrichton

Fixes bootstrapping with custom cargo/rustc.

config.mk is now always read when parsing the configuration to prevent
this from reoccurring in the future, hopefully.

Fixes rust-lang#42543.

r? @alexcrichton

cc @infinity0 @kyrias
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 22, 2017
…ap, r=alexcrichton

Fixes bootstrapping with custom cargo/rustc.

config.mk is now always read when parsing the configuration to prevent
this from reoccurring in the future, hopefully.

Fixes rust-lang#42543.

r? @alexcrichton

cc @infinity0 @kyrias
bors added a commit that referenced this issue Jun 22, 2017
…richton

Fixes bootstrapping with custom cargo/rustc.

config.mk is now always read when parsing the configuration to prevent
this from reoccurring in the future, hopefully.

Fixes #42543.

r? @alexcrichton

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

No branches or pull requests

8 participants