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.

Sign up for GitHub

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

Jump to bottom

Rustfmt is not vendored correctly #52029

Closed
bugabinga opened this issue Jul 3, 2018 · 6 comments
Closed

Rustfmt is not vendored correctly #52029

bugabinga opened this issue Jul 3, 2018 · 6 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Milestone
1.28

Comments

@bugabinga
Copy link

rust/src/Cargo.toml

Line 67 in 860d169

[patch.crates-io]

I think this line should be [patch."https://github.com/rust-lang-nursery/rustfmt"].
RLS switched its depenence on rustfmt to a git dependency some time ago.

This came up while looking into semarie/build-rust#9.
@Mark-Simulacrum
Copy link
Member

cc @alexcrichton

@semarie
Copy link
Contributor

semarie commented Jul 4, 2018

@Mark-Simulacrum it should come from this update of RLS #51407 which reached beta branch.

The problem is similar to #42719 (same problem but for a different rust release).
It is a stable-from-beta regression.

@semarie
Copy link
Contributor

semarie commented Jul 4, 2018

Full log with a build with Rust 1.27.0 :

Mon Jul  2 19:47:22 CEST 2018: info: building: 1.28.0-beta.6 (915ac6602 2018-06-30)
Mon Jul  2 19:47:22 CEST 2018: info: required stage0:
	date: 2018-06-21
	rustc: 1.27.0
	cargo: 0.28.0
Mon Jul  2 19:47:22 CEST 2018: info: rustc -vV
	rustc 1.27.0
	binary: rustc
	commit-hash: unknown
	commit-date: unknown
	host: x86_64-unknown-openbsd
	release: 1.27.0
	LLVM version: 6.0
Mon Jul  2 19:47:22 CEST 2018: info: cargo -vV
	cargo 1.27.0
	release: 1.27.0
Mon Jul  2 19:47:22 CEST 2018: starting rustbuild dist --jobs=4
running: /usr/local/bin/cargo build --manifest-path /data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/Cargo.toml --frozen
    Updating git repository `https://github.com/rust-lang-nursery/rustfmt`
error: failed to load source for a dependency on `rustfmt-nightly`

Caused by:
  Unable to update https://github.com/rust-lang-nursery/rustfmt?rev=f3906267#f3906267

Caused by:
  failed to fetch into /data/semarie/build-rust/install_dir/crates/git/db/rustfmt-5390e0ead582d971

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-beta-src/x.py", line 20, in <module>
    bootstrap.main()
  File "/data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/bootstrap.py", line 827, in main
    bootstrap(help_triggered)
  File "/data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/bootstrap.py", line 806, in bootstrap
    build.build_bootstrap()
  File "/data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/bootstrap.py", line 633, in build_bootstrap
    run(args, env=env, verbose=self.verbose)
  File "/data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/bootstrap.py", line 148, in run
    raise RuntimeError(err)
RuntimeError: failed to run: /usr/local/bin/cargo build --manifest-path /data/semarie/build-rust/build_dir/rustc-beta-src/src/bootstrap/Cargo.toml --frozen

@Mark-Simulacrum Mark-Simulacrum added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Jul 4, 2018
@Mark-Simulacrum Mark-Simulacrum added this to the 1.28 milestone Jul 4, 2018
@Mark-Simulacrum Mark-Simulacrum added I-nominated T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 4, 2018
@Mark-Simulacrum
Copy link
Member

If someone wants to prepare a patch for this that would be great; otherwise we'll probably discuss this during next week's infra meeting and confirm that the suggested fix is the right one.

@Mark-Simulacrum
Copy link
Member

We discussed this in the infrastructure meeting and the general conclusion was that @alexcrichton is probably not going to fix this themselves but that a fix is likely to be reviewed. @semarie or @bugabinga either of you interested in writing up a PR?

@semarie semarie mentioned this issue Jul 15, 2018
Merged
bors added a commit that referenced this issue Jul 15, 2018
@bors
Auto merge of #52401 - semarie:tidy-extdeps, r=alexcrichton
100fb12 
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
@semarie semarie mentioned this issue Jul 16, 2018
Merged
semarie added a commit to semarie/rust that referenced this issue Jul 16, 2018
@semarie
use vendored rustfmt-nightly (src/tools/rustfmt)
69ee44d 
Fixes: rust-lang#52029
bors added a commit that referenced this issue Jul 16, 2018
@bors
Auto merge of #52419 - semarie:beta-rustfmt, r=Mark-Simulacrum
a7e85eb 
use vendored rustfmt-nightly (src/tools/rustfmt)

Fixes: #52029
@pietroalbini
Copy link
Member

Fixed and backported by #52419.

@bugabinga bugabinga mentioned this issue Jul 24, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.28
Development

No branches or pull requests
4 participants
@bugabinga @pietroalbini @Mark-Simulacrum @semarie