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

building beta with vendor=true fail due to network dependencies #42719

Closed
semarie opened this issue Jun 17, 2017 · 27 comments
Closed

building beta with vendor=true fail due to network dependencies #42719

semarie opened this issue Jun 17, 2017 · 27 comments
Labels
P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@semarie
Copy link
Contributor

semarie commented Jun 17, 2017

Trying to build beta using downloaded tarball and vendor=true (in order to test and prepare distribution for OpenBSD packages), I have the following error:

$ python2.7 /data/semarie/build-rust/build_dir/rustc/rustc-beta-src/x.py build
    Updating git repository `https://github.com/rust-lang-nursery/rustfmt`
error: failed to load source for a dependency on `rustfmt`

Caused by:
  Unable to update https://github.com/rust-lang-nursery/rustfmt?branch=libsyntax#6c1de769

Caused by:
  failed to fetch into /home/semarie/.cargo/git/db/rustfmt-5390e0ead582d971

Caused by:
  attempting to update a git repository, but --frozen was specified
failed to run: /usr/local/bin/cargo build --manifest-path /data/semarie/build-rust/build_dir/rustc/rustc-beta-src/src/bootstrap/Cargo.toml --frozen
Build completed unsuccessfully in 0:00:00

My config.toml is:

[build]
rustc = "/usr/local/bin/rustc"
cargo = "/usr/local/bin/cargo"
prefix = "/data/semarie/build-rust/install_dir/beta"
docs = false
vendor = true

[dist]
src-tarball = false

[rust]
channel = "beta"
codegen-tests = false

[target.x86_64-unknown-openbsd]
llvm-config = "/usr/local/bin/llvm-config"
$ rustc -vV
rustc 1.18.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-openbsd
release: 1.18.0
LLVM version: 4.0

$ cargo -vV
cargo 0.19.0
release: 0.19.0

The error could be about rustfmt or cargo dependency.

It seems that rls have git dependencies defined for them. From src/tools/rls/Cargo.toml:

[dependencies]
cargo = { git = "https://github.com/rust-lang/cargo" }
rustfmt = { git = "https://github.com/rust-lang-nursery/rustfmt", branch = "libsyntax" }

And even if src/Cargo.toml has a [replace] section, it is only for 0.20.0 (and not git head):

"https://github.com/rust-lang/cargo#0.20.0" = { path = "tools/cargo" }

So when cargo tries to resolv dependencies, it tries to download them from network, but fail due to --frozen.

Please note that when testing, my CARGO_HOME (~/.cargo) is empty.

@Mark-Simulacrum Mark-Simulacrum added A-build regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 18, 2017
@cuviper
Copy link
Member

cuviper commented Jun 19, 2017

I just ran into this too. :/

FWIW, on rust master rls/Cargo.toml now specifies rustfmt-nightly = "0.1", but still git cargo.

@semarie
Copy link
Contributor Author

semarie commented Jun 20, 2017

Under nightly, I have the same: only cargo error now.

...
DEBUG:cargo::core::registry: load/match    registry https://github.com/rust-lang/crates.io-index
DEBUG:cargo::core::registry: load/match    registry https://github.com/rust-lang/crates.io-index
DEBUG:cargo::core::registry: load/match    registry https://github.com/rust-lang/crates.io-index
DEBUG:cargo::core::registry: load/missing  https://github.com/rust-lang/cargo#50b1c24d
DEBUG:cargo::sources::config: loading: https://github.com/rust-lang/cargo#50b1c24d
    Updating git repository `https://github.com/rust-lang/cargo`
DEBUG:cargo: exit_with_error; err=CliError { error: Some(CargoError(Msg("failed to load source for a dependency on `cargo`"), State { next_error:
 Some(CargoError(Msg("Unable to update https://github.com/rust-lang/cargo#50b1c24d"), State { next_error: Some(CargoError(Msg("failed to fetch into /data/semarie/build-rust/install_dir/crates/git/db/cargo-e7ff1db891893a9e"), State { next_error: Some(CargoError(Msg("attempting to update a git repository, but --frozen was specified"), State { next_error: None, backtrace: None })), backtrace: None })), backtrace: None })), backtrace:
None })), unknown: false, exit_code: 101 }
error: failed to load source for a dependency on `cargo`

Caused by:
  Unable to update https://github.com/rust-lang/cargo#50b1c24d

Caused by:
  failed to fetch into /data/semarie/build-rust/install_dir/crates/git/db/cargo-e7ff1db891893a9e

Caused by:
  attempting to update a git repository, but --frozen was specified
Traceback (most recent call last):
  File "/data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/bootstrap.py", line 694, in <module>
    main()
  File "/data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/bootstrap.py", line 678, in main
    bootstrap()
  File "/data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/bootstrap.py", line 660, in bootstrap
    rb.build_bootstrap()
  File "/data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/bootstrap.py", line 413, in build_bootstrap
    run(args, env=env, verbose=self.verbose)
  File "/data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/bootstrap.py", line 142, in run
    raise RuntimeError(err)
RuntimeError: failed to run: /data/semarie/build-rust/install_dir/beta/bin/cargo build --manifest-path /data/semarie/build-rust/build_dir/rustc/rustc-nightly-src/src/bootstrap/Cargo.toml --verbose --frozen

@est31
Copy link
Member

est31 commented Jul 3, 2017

Either we fix alexcrichton/cargo-vendor#30 or we stop depending on git deps somehow.

@est31
Copy link
Member

est31 commented Jul 3, 2017

hmm digging a bit more, it seems the git dependency in Cargo.lock is due to rls requiring it in its Cargo.toml. It is being replaced in the src/Cargo.toml of this repo, but apparently Cargo still needs the git repo for some reason. So that may be a Cargo bug?

@brson
Copy link
Contributor

brson commented Jul 7, 2017

cc @alexcrichton

This would be a pretty unfortunate thing to hit stable.

Anybody have ideas about how we can automatically test this on CI?

@brson brson added the P-high High priority label Jul 7, 2017
@est31
Copy link
Member

est31 commented Jul 7, 2017

Anybody have ideas about how we can automatically test this on CI?

We could create a source tarball and download the stage0, remove network by doing sudo ifdown <interface name> or something similar, and then do a bootstrap of rustc.

@est31
Copy link
Member

est31 commented Jul 7, 2017

A possible emergency mitigation of this issue: we could point beta to a branch/fork of rls that points to cargo as a relative path dependency instead of a git dependency. Alternatively we simply remove rls but not sure how much changes of rustbuild that entails.

Long term one can think about either giving cargo-vendor support to git deps or fixing cargo itself to worh with the current situation (not syncing git repos if they get [replace]'d), but I doubt such a fix will make it into beta in time.

@brson
Copy link
Contributor

brson commented Jul 7, 2017

Are there any hacks we can do to automatically mangle the rls deps during vendoring? e.g. x.py vendor calls cargo vendor while doing hacks on all the resulting tomls.

@alexcrichton
Copy link
Member

There are no hacks I know of to get this to work. If we want this fixed I believe we need to remove the rls from the repository on the beta branch.

@brson
Copy link
Contributor

brson commented Jul 8, 2017

There's also a 1.19 milestone issue that I'd forgotten about to disable RLS packaging: #42357

@brson
Copy link
Contributor

brson commented Jul 8, 2017

cc @rillian seems like this might impact you.

@est31
Copy link
Member

est31 commented Jul 9, 2017

If dropping the network doesn't work on travis because e.g. it outputs the log via the network, we might have to use docker, but we already use it for android and stuff, so it should be no big deal.

@semarie
Copy link
Contributor Author

semarie commented Jul 10, 2017

the current error is present even with network access: cargo catch it needs network for git whereas it should also respect --frozen.

But I agree that be able to drop the network would catch more errors. Another possibility is to setup firewall rules to forbid outgoing connection.

@cuviper
Copy link
Member

cuviper commented Jul 10, 2017

FWIW, I usually try to do a test build of beta on all Fedora arches by this time in the release cycle, but this bug is blocking me.

@est31
Copy link
Member

est31 commented Jul 10, 2017

@cuviper it should be simple to patch out:

  1. Replace the cargo = { git = stuff in src/tools/rls/Cargo.toml with cargo = { path = "../cargo" in src/tools/Cargo.toml
  2. Remove the [replace] section in src/Cargo.toml
  3. Update Cargo.lock somehow without doing a proper "cargo update". Locally this can be done by issuing ./x.py build, it should happen right at the start, so you can just abort it and put the Cargo.lock change into your patch.

Not tried it however.

Note that I'm only suggesting this to help you with testing. It would be pretty disastrous if we attempted to release 1.20 with this bug present.

@cuviper
Copy link
Member

cuviper commented Jul 10, 2017

Fair enough, I can patch it. There's also rls->rustfmt to deal with, but I think I can just comment out these dependencies for now and leave rls broken. IIRC it's not part of a normal build yet...

@aturon
Copy link
Member

aturon commented Jul 12, 2017

@cuviper Would you be willing to get a patch up to remove rls from the beta branch? We'd need it by the end of the week.

@cuviper
Copy link
Member

cuviper commented Jul 12, 2017

Yeah, I can - if not today then tomorrow.

@nrc
Copy link
Member

nrc commented Jul 12, 2017

In general we shouldn't depend on git deps at all. Cargo should be replaced by the local version and rustfmt should be a crates.io dependency. It was temporarily pointed at a branch to land a libsyntax change. We just need to update the rls submodule to go back to a crates.io dep (I have a PR doing this, but it also turns on tests and so is having trouble landing).

cuviper added a commit to cuviper/rust that referenced this issue Jul 13, 2017
Its git dependencies don't work when building with vendored crates,
so for now it will just be removed from the workspace and disabled in
the rustbuild rules.

cc rust-lang#42719
@cuviper
Copy link
Member

cuviper commented Jul 13, 2017

See #43199.

alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Jul 13, 2017
Its git dependencies don't work when building with vendored crates,
so for now it will just be removed from the workspace and disabled in
the rustbuild rules.

cc rust-lang#42719
alexcrichton pushed a commit to brson/rust that referenced this issue Jul 13, 2017
Its git dependencies don't work when building with vendored crates,
so for now it will just be removed from the workspace and disabled in
the rustbuild rules.

cc rust-lang#42719
@brson
Copy link
Contributor

brson commented Jul 14, 2017

Should be fixed on current beta.4. Can anyone confirm?

@cuviper
Copy link
Member

cuviper commented Jul 14, 2017

Seems good so far: https://koji.fedoraproject.org/koji/taskinfo?taskID=20529669
(I just started that build, but before it would have failed right away.)

@semarie
Copy link
Contributor Author

semarie commented Jul 15, 2017

I could confirm 1.19.0-beta.4 is fixed.

@semarie semarie closed this as completed Jul 15, 2017
@est31
Copy link
Member

est31 commented Jul 15, 2017

There is #43218 to track the issue for the 1.20 release.

@est31
Copy link
Member

est31 commented Jul 16, 2017

oh, TIL about alexcrichton/cargo-vendor#42 and rust-lang/cargo#3992 . Those are probably the long term fixes for this issue...

@semarie
Copy link
Contributor Author

semarie commented Aug 6, 2017

same the problem again with nightly. should I open a new issue, or should I reopen this one (but tags would be wrong: it is a nightly problem currently) ?

@est31
Copy link
Member

est31 commented Aug 6, 2017

@semarie I think its handled on a per-release basis (see #43218), but you can open an issue for 1.21 already if you wish.

bors added a commit that referenced this issue Jul 15, 2018
tidy: add a new test for external dependencies

ensure all packages in Cargo.lock will be vendored, and fail if the
source packages isn't whitelisted.

the purpose is to avoid such kind of issues:
- #52029 Rustfmt isn't vendored correctly
- #42719 building beta with vendor=true fail due to network dependencies

as Rust comes with several external dependencies (clippy, miri, rustfmt, rls), it is important to have a way to catch some errors in the update of this submodules.

The new check in tidy quickly reads `Cargo.lock` to search for the `source` of all packages. This attribute is present when the package comes from external source (like `crates.io-index` or some `git` repository). Some sources are whitelisted (like `crates.io-index`) as the crates are vendored.

`Cargo.lock` extract with several cases (git, crates.io, and local).
```
[[package]]
name = "rustfmt-nightly"
version = "0.8.2"
source = "git+https://github.com/rust-lang-nursery/rustfmt?rev=5e5992517d3591e2708d4ca6b155dfcbdf3344b9#5e5992517d3591e2708d4ca6b155dfcbdf3344b9"
dependencies = [
...
]

[[package]]
name = "same-file"
version = "1.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
...
]

[[package]]
name = "rustdoc-themes"
version = "0.1.0"
```

r? @alexcrichton
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-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

8 participants