Lint to forbid std::process::Command

[omertuc]

I use RUST_LOG=debug and for crate::task::Task it works great, e.g.:

DEBUG exec: "lsblk" "--pairs" "--paths" "--inverse" "--output" "NAME,TYPE" "composefs"

but for std::process::Command (e.g.):

bootc/lib/src/mount.rs

Lines 32 to 41 in d7c3f9f

	let o: Findmnt = Command::new("findmnt")
	.args([
	"-J",
	"-v",
	// If you change this you probably also want to change the Filesystem struct above
	"--output=SOURCE,TARGET,MAJ:MIN,FSTYPE,OPTIONS,UUID",
	])
	.args(args)
	.arg(path)
	.run_and_parse_json()?;

there's no log, which is unfortunate.

Would it be reasonable to get rid of Command altogether?

Some obvious concerns is that Task doesn't necessarily provide all the flexibility Command does, but I'm sure in most cases that flexibility is unnecessary

[cgwalters]

I keep going back and forth on Task honestly. Lately I've been leaning a bit more against - to solve the logging thing what we could do is add an e.g. .log_debug() extension method? I went ahead and did that #794

[omertuc]

I keep going back and forth on Task honestly. Lately I've been leaning a bit more against - to solve the logging thing what we could do is add an e.g. .log_debug() extension method? I went ahead and did that #794

That seems great yeah, even though it's opt-in so it's not exactly covering what I was hoping for

[cgwalters]

Yeah...what I feel like is Task ended up being a bit too high level, so I started circling back to just "helpers for Command" with clear semantics. Adding a log message by default is starting to get a little opinionated again...in some cases the caller may have already done logging in a more comprehensible way than just dumping the args?

I guess arguably we could log at trace level unconditionally?

[omertuc]

Yeah...what I feel like is Task ended up being a bit too high level

I'm also generally against wrapping standard library command runners (in any language), but it seemed commonplace in bootc so I suggested to just align everything to it. But "helpers for Command" which I only saw later does seem like a much better idea.

I guess arguably we could log at trace level unconditionally?

That's definitely not that opinionated, I support this

[omertuc]

(EDIT: comment posted before the above)

Or maybe could we just add a log to:

bootc/utils/src/command.rs

Lines 68 to 72 in d7c3f9f

	fn run(&mut self) -> Result<()> {
	let stderr = tempfile::tempfile()?;
	self.stderr(stderr.try_clone()?);
	self.status()?.check_status(stderr)
	}

(but perhaps logging is not the only benefit of having everything go through Task so maybe it's not an excuse to give up on this discussion)

[omertuc]

Closing for now due to #794