Support conditionally setting arguments to a command

[huonw]

In pantsbuild/scie-pants#249, we encounter a circumstance where a command has an arg that needs to be conditionally set: a binding computes an env var that is sometimes set to --python-repos-find-links=... and sometime empty. If the env var is empty, it'd be better for the command to not receive an argument: invoking pants "" ... causes issues with pants specifically, and in general passing a spurious empty string is likely to cause problems for any program.

It'd be handy to be able to (optionally) make an argument conditional, so that these sort of "maybe set an argument" uses cases are easier to handle.

AFAICT, this isn't currently possible with scie-jump: there's a reified argument for every argument in the manifest, and string like "{scie.bindings.configure:PANTS_SHA_FIND_LINKS}" just becomes an empty string.

jump/jump/src/context.rs

Lines 270 to 274 in 1403e1f

	for arg in &cmd.args {
	let (reified_arg, needs_manifest) = self.reify_string(&env, arg)?;
	needs_lift_manifest |= needs_manifest;
	args.push(reified_arg.into());
	}

jump/jump/src/context.rs

Lines 576 to 581 in 1403e1f

	let value = binding_env
	.get(&parsed_env.name)
	.map(String::to_owned)
	.or(parsed_env.default)
	.unwrap_or_default();
	reified.push_str(&value)

Suppose there's a manifest like the following, we'd like to be able to annotate the ARGUMENT arg to disappear completely in some cases (particularly when ARGUMENT is empty, but a more general mechanism would be okay too):

...
"commands": {
  "": {
    "exe": ".../executable"
    "args": ["{scie.bindings.some_binding:ARGUMENT}"]
  }
}
...

In shell terms:

ARGUMENT=""

.../executable "$ARGUMENT" # current behaviour: passing a "" argument

.../executable $ARGUMENT # requested behaviour: no argument at all

Workaround that we'll likely use in pantsbuild/scie-pants#250: instead of setting the env var to nothing, set it to a "no-op" argument, so that we're never passing an empty arg to the executable.

(This may apply to arguments in bindings in addition to commands?)

[jsirois]

@huonw as I noted here: pantsbuild/scie-pants#249 (comment), scie-jump doesn't support dynamic commands, the lift manifest is static save for interpolation. None the less, commands can be selected dynamically by using recursive dispatch. This is what scie-pants already does to handle the debug case. I'd really like to keep scie-jump simple as per the general thrust outlined here: https://github.com/a-scie/jump/blob/main/CONTRIBUTING.md#guiding-principles

Does the recursive dynamic dispatch of commands that scie-pants already does suffice for you @huonw?

[huonw]

Yep, happy to call that sufficient for this. I assume you aware of the combinatorial explosion (if one has 2 args that might need to disappear independently, I think one will need 4 commmands for the different combinations of present vs not; 3 args => 8 commands, etc?), but, as I said, happy to call this one answered.

[jsirois]

Yeah, I'm aware Pants would need to go from 2 to 4 commands the way it's currently doing things. My initial thought was to not do things this way (not have the existing configuration / install handle PEX installs), but to have Pants release an unpacked scie (PEX + manifest) where each release of Pants could have arbitrarily complex bespoke to the version complexity (see: #10).

All that said, isn't scie-pants / Pants engaged in hoisting and petards here? IIUC one can say "--python-repos-find-links=[]" to indicate to Pants a base set of 0 find links repos (or via env var), or "--python-repos-find-links=foo to append one, etc? I.E. I think it's the case that scie-pants could have solved this totally in house in a clean way.

[jsirois]

I noted what seems to me to be the root bug here: pantsbuild/scie-pants#226 (comment)

[huonw]

Yeah, in this case, there's an "obvious" no-op value, so that's the fix we went for in pantsbuild/scie-pants#250