Repository navigation
tidy tries to run unrelated x binary #106469
Description
Activity
Hmm, that's unfortunate, I didn't realize this was a name that was already being used :(
cc @DebugSteven, we might have to revert that PR :/
@ehuss would it be ok if we only ran your
xcommand and silently hid the error, so nothing ends up happening? Or does even that cause issues?- changed the title
[-]tidy fails with unexpected character[/-][+]tidy tried to run unrelated `x` binary[/+]on Jan 4, 2023 - changed the title
[-]tidy tried to run unrelated `x` binary[/-][+]tidy tries to run unrelated `x` binary[/+]on Jan 4, 2023 We can probably check that the binary is in ~/.cargo/bin or so before running it, at least. We can probably also add a custom section to it or other content that we can find without running it (e.g., by reading the contents and looking for a well known version).
I seem to recall cargo tracking installed versions too, maybe we can lean on that?
Reacted by jynI seem to recall cargo tracking installed versions too, maybe we can lean on that?
Oh, that's an excellent idea! yes,
cargo install --listwill output something likex v0.1.0 (/home/gh-jyn514/rust3/src/tools/x): xand we can only run
xif we seesrc/tools/xin the output.A debug section seems slightly overkill but I'm not opposed if you think
cargo install --listwill be unreliable for whatever reason; I supposePATHcould be in the wrong order or something like that.I think my point is that if we're diligent about bumping the cargo.toml version we don't ever need to run it, just look at Cargo output.
Reacted by jynI think for me it might be OK to ignore any errors running the command or any parsing errors on the output.
cargo install --listisn't intended to be machine-readable, but is also an option that could work.Opening the executable and searching for a unique sequence of bytes that precedes the version number is also an option, but possibly a bit of a hassle. (Or it could look for some unique string that identifies it as the x.py wrapper, and then execute it.)
Any option is fine, and there's no urgency here unless it affects other people.
I really don't like the idea of just running a arbitrary binary, even if it appears in
cargo install --list. On my system, the commandX(note that it is a capital but this is too close for comfort) runs X11.Perhaps the check can be moved into bootstrap (or anywhere out of tidy) so that it can know its version without running binaries? IMO, tidy is more about linting code; it shouldn't emit an error if a user has an old version of an optional and unessential tool.
Starting with #104552,
./x.py test tidyfails on my system:I have an executable installed on my system called
xthat is unrelated to rust. I would prefer if tidy didn't execute it at all.