Commit 4eb6b8f

authored

Rollup merge of #147120 - Shunpoco:issue-147105, r=Kobzol

Fix --extra-checks=spellcheck to prevent cargo install every time Fixes #147105 ## Background Current implementation of `ensure_version_of_cargo_install` uses `bin_name` to check if it exists, but it should use `<tool_root_dir>/<tool_bin_dir>/<bin_name>` instead. Otherwise the check fails every time, hence the function falls back to install the binary. ## Change Move lines which define bin_path at the top of the function, and use bin_path for the check

2 parents aa6bd55 + 5e9cab3 commit 4eb6b8fCopy full SHA for 4eb6b8f

File tree

1 file changed

-5

lines changed

src/tools/tidy/src
- lib.rs

1 file changed

-5

lines changed

`‎src/tools/tidy/src/lib.rs‎`

Lines changed: 6 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -167,12 +167,16 @@ pub fn ensure_version_or_cargo_install(`
`167`	`167`	`bin_name: &str,`
`168`	`168`	`version: &str,`
`169`	`169`	`) -> io::Result<PathBuf> {`
	`170`	`+ let tool_root_dir = build_dir.join("misc-tools");`
	`171`	`+ let tool_bin_dir = tool_root_dir.join("bin");`
	`172`	`+ let bin_path = tool_bin_dir.join(bin_name).with_extension(env::consts::EXE_EXTENSION);`
	`173`	`+`
`170`	`174`	`// ignore the process exit code here and instead just let the version number check fail.`
`171`	`175`	`// we also importantly don't return if the program wasn't installed,`
`172`	`176`	`// instead we want to continue to the fallback.`
`173`	`177`	`'ck: {`
`174`	`178`	`// FIXME: rewrite as if-let chain once this crate is 2024 edition.`
`175`		`- let Ok(output) = Command::new(bin_name).arg("--version").output() else {`
	`179`	`+ let Ok(output) = Command::new(&bin_path).arg("--version").output() else {`
`176`	`180`	`break 'ck;`
`177`	`181`	`};`
`178`	`182`	`let Ok(s) = str::from_utf8(&output.stdout) else {`
`@@ -182,12 +186,10 @@ pub fn ensure_version_or_cargo_install(`
`182`	`186`	`break 'ck;`
`183`	`187`	`};`
`184`	`188`	`if v == version {`
`185`		`- return Ok(PathBuf::from(bin_name));`
	`189`	`+ return Ok(bin_path);`
`186`	`190`	`}`
`187`	`191`	`}`
`188`	`192`
`189`		`- let tool_root_dir = build_dir.join("misc-tools");`
`190`		`- let tool_bin_dir = tool_root_dir.join("bin");`
`191`	`193`	`eprintln!("building external tool {bin_name} from package {pkg_name}@{version}");`
`192`	`194`	`// use --force to ensure that if the required version is bumped, we update it.`
`193`	`195`	`// use --target-dir to ensure we have a build cache so repeated invocations aren't slow.`
`@@ -213,7 +215,6 @@ pub fn ensure_version_or_cargo_install(`
`213`	`215`	`if !cargo_exit_code.success() {`
`214`	`216`	`return Err(io::Error::other("cargo install failed"));`
`215`	`217`	`}`
`216`		`- let bin_path = tool_bin_dir.join(bin_name).with_extension(env::consts::EXE_EXTENSION);`
`217`	`218`	`assert!(`
`218`	`219`	`matches!(bin_path.try_exists(), Ok(true)),`
`219`	`220`	`"cargo install did not produce the expected binary"`

0 commit comments

Comments

(0)

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit 4eb6b8f

File tree

1 file changed

1 file changed

`‎src/tools/tidy/src/lib.rs‎`

0 commit comments