makeWrapper: --add-flag and --append-flag arguments

[BirdeeHub]

Please see this other PR for context. #397604

This is --add-flag and --append-flag mentioned there.

This is to address a major shortcoming in makeBinaryWrapper

The inability to pass a string with a space in it as a single argument to makeBinaryWrapper's implementation of --add-flags argument.

Adds new wrapper arguments --add-flag and --append-flag to both makeWrapper implementations with the same behavior for each.

# --add-flag    ARG      : prepend ARG to the invocation of the executable, escaped as a single argument
# --append-flag ARG      : append ARG to the invocation of the executable, escaped as a single argument

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[drupol]

Does this PR set makeWrapper and makeBinaryWrapper at the same level and get rid of the makeBinaryWrapper limitations ?

[BirdeeHub]

Not quite.

I attempted to fix --add-flags in the other PR (and found a solution) but it wasnt meant to be.

But this does help with that!

This does make makeBinaryWrapper more useful. It does fix a major issue, that issue being it wasnt possible to pass an argument that contained a quoted string with a space in it using --add-flags

It does this by allowing you to pass single arguments, which are guaranteed to be grouped as single arguments.

It doesnt fix all the issues, nor make them interchangeable, and they arent necessarily meant to be, although I do think consistency should be something to think about regardless, but it does fix at least some of the issues!

See the other PR for why it isnt just fixing --add-flags to work correctly. I'm still of the opinion that it should be fixed, but the problem space was quite thoroughly explored and the required solution was 100 lines of bash tokenizer unless someone finds a better way, so I understand why others are of the opinion that it should be left as is to avoid breaking changes.

[BirdeeHub]

Added documentation updates, and merged in the makeShellWrapper changes into this one as well as per request.

[BirdeeHub]

Question:

Should the arguments be added before the --add-flags and --append-flags arguments, or after?

Because the new ones are more reliably escaped, I put them before, but there could be an argument made for either way.

For example, maybe for -x style flags, --add-flags is slightly nicer because you can add a bunch of them, and they often need to go first, so having --add-flags be first instead might be better?

Interspersed is not the way to go due to not being possible in the makeBinaryWrapper implementation without reworking it extensively (or adopting my previous PR)

But a good argument could be made about whether they should be before or after their plural counterparts, and I would appreciate if that argument were to occur before this is merged rather than after!

Edit:

It should definitely be after, both for the reason I give above, and to avoid any possible breaking changes at all.

If we wish to do interspersed, I need to parse the arguments for --add-flags in makeBinaryWrapper like in the other PR.

Another option. we allow their implementations to drift further. Intersperse them in the shell wrapper, and add them after in makeBinaryWrapper.

Edit2: I suppose the last option would be only adding the extra option to makeBinaryWrapper because makeShellWrapper doesnt really need it? Edit3: chose this option. makeShellWrapper has no need for an --add-flag

[BirdeeHub]

I do still feel that this PR is ok though. Not being able to pass a string with a space at all needs to be solved, and adding a workaround flag like --add-flag seems to be the easiest and most uncontroversial way to do so, and makeShellWrapper has no need for such a flag due to it having an --add-flags that actually works as expected.

[ncfavier]

The arguments should of course be interspersed. --add-flag a --add-flags 'b c' --add-flag d should result in a b c d. local -n works just fine with arrays. Don't use eval.

[BirdeeHub]

Ok. So, both implementations have the new flag, variables dont expand at runtime in either of them, interspersed with the old --add-flags which works as before. And suggested changes have been implemented :) That is how it should be yes?

Trying to get the flags from the new argument to be separate and still expand at runtime in makeShellWrapper proved difficult, so I figured consistent behavior for the flag between both wrappers made more sense. The old ones in makeShellWrapper still expand as normal obviously.

If you think the variables should expand at runtime in makeShellWrapper like the --add-flags arguments do, let me know and I can attempt to do that, and if I don't succeed, just remove them makeShellWrapper and submit only the changes to makeBinaryWrapper. But I think Im satisfied with the fact that the new flags currently have the same behavior in both wrappers, and users who want runtime expansion can use --add-flags in makeShellWrapper still.

[ncfavier]

If you think the variables should expand at runtime in makeShellWrapper like the --add-flags arguments do

No, definitely not. --add-flag should be the default interface for adding verbatim arguments to the command line and should not be subject to any kind of mangling.

[BirdeeHub]

If you think the variables should expand at runtime in makeShellWrapper like the --add-flags arguments do

No, definitely not. --add-flag should be the default interface for adding verbatim arguments to the command line and should not be subject to any kind of mangling.

ok.

Nice.

So the current behavior is correct then, it just needs cleanup.

I will work on the cleanups you suggest soon!

Thank you for taking the time to help me with this by the way!

[ncfavier]

Small whitespace issues, otherwise looks good now