Allow and skip empty elements in Command construction

[LecrisUT]

Technically a bug because if the first element is "" it will become " actual_command" which can have some tricky consequences

Pull Request Checklist

• implement the feature

[happz]

I can see the issue with Command('ls', ''), but how would None get it? Is the relaxation of this type needed? What will be easier if we could let None travel through our code unchecked until dropped in Command? That seems risky.

[LecrisUT]

Well without this whenever we want to use this feature we would have to use an empty "" explicitly. None seems more explicit in that case as an element that will not be expanded.

[happz]

Hmm, isn't None or "" appearing as command element the actual issue, i.e. something feeding Command with things that cannot function as command elements? Dropping it would make things work, but that really seems like silencing errors. If you ran into something like that, we should probably fix the callsite, and maybe raise a ValueError to catch future violations. Empty strings are sneaky bastards, we should root them out as soon as we spot them, if they travel through our calls, we are probably missing checks somewhere.

[LecrisUT]

Well this happens in constructs such as

tmt/tmt/package_managers/dnf.py

Lines 41 to 42 in 5ba7562

	if self.guest.facts.sudo_prefix:
	command = Command(self.guest.facts.sudo_prefix) + self._base_command

which I think would be perfectly valid

[LecrisUT]

I mean prefectly valid to avoid the prior if check and use the whole thing

tmt/tmt/package_managers/bootc.py

Lines 95 to 98 in 5ba7562

	command = Command('bootc')
	if self.guest.facts.sudo_prefix:
	command = Command(self.guest.facts.sudo_prefix, 'bootc')

Becoming just

command = Command(self.guest.facts.sudo_prefix, 'bootc')

[happz]

Interesting. So dropping the test, and just constructing the command, relying on Command filtering out all invalid items?

I get the use case, but it really seems risky to me, because once this "negative" kind of items is allowed, sooner or later an unwanted and unexpected one would sneak in, without us noticing :/

If I had my way, I would root out empty strings completely, including the sudo_prefix, because the emptiness is often misused to track "undefined", "unset", or "unknown" kind of info. Maybe we could turn sudo_prefix into something friendlier to other kids... Not sure what though, I'd like to think about this first.

[happz]

I mean prefectly valid to avoid the prior if check and use the whole thing

tmt/tmt/package_managers/bootc.py

Lines 95 to 98 in 5ba7562

	command = Command('bootc')
	if self.guest.facts.sudo_prefix:
	command = Command(self.guest.facts.sudo_prefix, 'bootc')

Becoming just

command = Command(self.guest.facts.sudo_prefix, 'bootc')

Got it.

It might seem I'm needlessly paranoid... But I really dislike empty strings being silently dropped; with the history of people (mis)using empty strings, it's practically guaranteed the silence will harm us one day. So I'd like to find something we could verify. Maybe a type for strings that can be empty, and it does not bear any boolean-like meaning, who knows.

[gemini-code-assist]

Code Review

This pull request addresses an issue in Command construction where an initial empty string element would create a command with a leading space. This is resolved by filtering out any falsy elements, such as empty strings and None, during command initialization. The type hint for RawCommandElement has been updated to include None to align with this change.

_{Note: Security Review is unavailable for this PR.}