warn newer available version of the x tool #104552

DebugSteven · 2022-11-17T20:53:13Z

rustbot · 2022-11-17T20:53:20Z

Noratrieb · 2022-11-17T22:04:50Z

jyn514

src/tools/tidy/src/main.rs

src/tools/tidy/src/lib.rs

src/tools/tidy/src/x.rs

ChrisDenton · 2022-11-18T22:08:40Z

jyn514

src/tools/tidy/src/x_version.rs

src/tools/x/src/main.rs

bors · 2022-12-31T02:25:33Z

bors · 2022-12-31T09:18:59Z

rustbot · 2022-12-31T18:07:03Z

DebugSteven · 2022-12-31T20:34:30Z

src/tools/tidy/src/x_version.rs

DebugSteven · 2023-01-02T23:40:02Z

src/tools/tidy/src/x_version.rs

+
+// Parse latest version out of `x` Cargo.toml
+fn get_x_wrapper_version() -> Option<Version> {
+    let cmd = Command::new("cargo")


Do we know this will be run from the root of the rust-lang/rust checkout?
I tested this by being in some random directory (inside the project) & it did use the root, but can we count on that or should the directory be set?

You can test by running ../../../x.py test tidy from src/tools/x, which isn't in the workspace. If that works, and rust/x.py test tidy works (from outside the source directory), this is fine :)

err sorry - actually can you use cargo_metadata, like in

rust/src/tools/tidy/src/deps.rs

Lines 347 to 354 in e9ca663

/// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path

/// to the cargo executable.

pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {

let mut cmd = cargo_metadata::MetadataCommand::new();

cmd.cargo_path(cargo)

.manifest_path(root.join("Cargo.toml"))

.features(cargo_metadata::CargoOpt::AllFeatures);

let metadata = t!(cmd.exec());

? that takes root as an argument which avoids this question altogether.

Running both of the above commands didn't end up working for me, even with the new change to use cargo_metadata.

I end up getting tidy errors for autogenerated files in the build directory... This seems like a separate issue though.

Building rustbuild Finished dev [unoptimized] target(s) in 0.08s Building stage0 tool tidy (aarch64-apple-darwin) Compiling tidy v0.1.0 (/Users/jhaigh/developer/Rust/rust/src/tools/tidy) Finished release [optimized] target(s) in 2.62s tidy check * 633 error codes * highest error code: E0791 tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/consts.rs: leading newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/consts.rs: missing trailing newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/tests.rs: leading newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/tests.rs: missing trailing newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/tests.rs: too many lines (17255) (add `// ignore-tidy-filelength` to the file to suppress this error) tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/op.rs: leading newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/bootstrap/debug/build/typenum- 949154f977098d36/out/op.rs: missing trailing newline tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/aarch64-apple-darwin/stage0/sh are/doc/cargo/README.md: too many trailing newlines (2) tidy error: /Users/jhaigh/developer/Rust/rust/src/tools/x/build/aarch64-apple-darwin/stage0/sh are/doc/rustfmt/README.md:108: trailing whitespace * 392 features some tidy checks failed Build completed unsuccessfully in 0:00:05

ah yeah that's from when I asked you to test ../../../x test tidy, sorry about that 😅 rm -rf src/tools/x/build will make the errors go away (although I've been wanting to get tidy to ignore untracked files for a while; but that's a separate PR.)

src/tools/tidy/src/x_version.rs

jyn514 · 2023-01-03T04:00:58Z

bors · 2023-01-03T04:01:00Z

rustbot assigned jyn514 Nov 17, 2022

rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2022

This comment has been minimized.

Sign in to view

jyn514 previously requested changes Nov 18, 2022

View reviewed changes

jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2022

This comment has been minimized.

Sign in to view

DebugSteven requested a review from jyn514 December 5, 2022 23:44

jyn514 reviewed Dec 7, 2022

View reviewed changes

src/tools/tidy/src/x_version.rs Show resolved Hide resolved

src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

8000 src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

src/tools/x/src/main.rs Outdated Show resolved Hide resolved

DebugSteven force-pushed the warn-newer-x branch from 15a008d to 5fda893 Compare December 31, 2022 18:07

DebugSteven and others added 8 commits December 31, 2022 11:37

warn when there's a newer version of the x tool available

c495558

remove leading comma from macro expansion

b2cd337

spawn x command and compare semvers

b9b33d9

remove commented out old code

a917308

formatting

7fe2f73

Update error message

02173f6

Co-authored-by: Joshua Nelson <github@jyn.dev>

Update error message

430ea8d

Co-authored-by: Joshua Nelson <github@jyn.dev>

rename wrapper-version argument for x to differentiate it from the bo…

f50ad6c

…otstrap version

DebugSteven force-pushed the warn-newer-x branch from 5fda893 to f50ad6c Compare December 31, 2022 18:39

DebugSteven added 2 commits January 1, 2023 13:20

handle error case where --wrapper-version argument doesn't exist

5e67ce6

fix formatting

e94354e

jyn514 reviewed Jan 1, 2023

View reviewed changes

src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

DebugSteven and others added 3 commits January 1, 2023 14:34

combine error branches

d7cac97

fix typo

e62258e

Co-authored-by: Joshua Nelson <github@jyn.dev>

improve error message

9aebb1e

Co-authored-by: Joshua Nelson <github@jyn.dev>

jyn514 reviewed Jan 1, 2023

View reviewed changes

src/tools/tidy/src/x_version.rs Outdated Show resolved Hide resolved

get latest x version from parsing cargo command

e9ca663

DebugSteven commented Jan 2, 2023

View reviewed changes

use cargo_metadata to get x version

376dd8a

jyn514 reviewed Jan 3, 2023

View reviewed changes

src/tools/tidy/src/x_version.rs Show resolved Hide resolved

no_deps

85f649f

Co-authored-by: Joshua Nelson <github@jyn.dev>

jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 3, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 3, 2023

matthiaskrgr mentioned this pull request Jan 3, 2023

Rollup of 8 pull requests #106409

Merged

bors merged commit 258a0fb into rust-lang:master Jan 3, 2023

ehuss mentioned this pull request Jan 4, 2023

tidy tries to run unrelated x binary #106469

Closed

DebugSteven mentioned this pull request Jan 11, 2023

Revert "warn newer available version of the x tool" #106733

Merged

warn newer available version of the x tool #104552

warn newer available version of the x tool #104552

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!