deploy-rs icon indicating copy to clipboard operation
deploy-rs copied to clipboard

fix targets and build args from clap v4 update

Open sedlund opened this issue 5 months ago • 4 comments

Disclaimer: this was 'vibe' 🤗 patched with LLM, I know very little of rust.

this fixes usage after the clap v4 update.

it removes --targets and allows --target to be repeated as is how I understand clap v4 wants to work. (having both was confusing and redundant anyway)

to easily specify multiple targets utilize shell brace expansion:

--target=.#{host1.profile1,host2.profile3} etc...

Everything seems to work for me so far. YMMV

#325

sedlund avatar Jul 10 '25 07:07 sedlund

You should probably update the interface.json accordingly

weriomat avatar Jul 10 '25 08:07 weriomat

Thanks. I updated the test to use --target. I didn't see anything specific in interface.json that needs to change. 🧐

sedlund avatar Jul 10 '25 08:07 sedlund

Sorry you are right, there is nothing to be changed there. Regardless in my opinion this introduces a regression. I can no longer specify a target via deploy -d -s .#test but I need to deploy -d -s --target .#test. I can no longer specify multiple targets, but I need to invoke --target multiple times. Fixing your problem can be done with only removing the group = "deploy" from target and targets. Maybe we should use custom Validation or no validation at all?

weriomat avatar Jul 14 '25 17:07 weriomat

I appreciate your feedback.

I can no longer specify a target via deploy -d -s .#test

This was unintentional.

Although, having 3 ways to specify hosts to deploy is redundant in the code and mental space. It makes the tool less approachable. And in my opinion, what allowed this breakage from not being caught originally.

I suggest having one way to pass targets in the future, however that may be (while supporting brace expansion).

I can no longer specify multiple targets, but I need to invoke --target multiple times

Shell brace expansion makes this better than not using them in most cases. This was possible before.

For example, with the original --targets option:

--targets myFlake#hostAlpha.alice myFlake#hostAlpha.bob myFlake#hostBravo.alice myFlake#hostBravo.bob myFlake#hostCharlie.alice myFlake#hostCharlie.bob

becomes:

--target=myFlake#host{Alpha,Bravo,Charlie}.{alice,bob}

Fixing your problem can be done with only removing the group = "deploy" from target and targets. Maybe we should use custom Validation or no validation at all?

For clarity, the issues with deploy-rs HEAD as well as in stable nixos-25.05:

  1. Cannot specify multiple targets.
  2. Cannot pass nix build args.

This PR allows a ref to patch with to resolve these (my goal).

I don't have time to continue without concrete suggestions for improvement. If something has been merged to resolve, I will close this.

Thanks again.

sedlund avatar Jul 15 '25 05:07 sedlund