wrapPackage: make shell escaping optional

[krauterbaquette]

I had a problem wrapping signal-desktop where argument escaping would break the parsing by signal:
--password-store="gnome-libsecret" would correctly set the password store whereas '--password-store="gnome-libsecret"' (produces by the shell escaping) would not.

This PR therefore adds an option to skip shell escaping which fixes the signal-desktop problem for me.

[Lassulus]

hmm, the problem could actually be the type of escaping we use here, does --pasword-store='gnome-libsecret' work here? I can also try to reproduce it locally. But it would be nice if this change could be accompanied by a test

[adrian-gierakowski]

'--password-store="gnome-libsecret"' seems incorrect
it should be '--password-store=gnome-libsecret' (or --password-store='gnome-libsecret', which is equivalent in bash)

[adrian-gierakowski]

@gytic could you share the code which produced '--password-store="gnome-libsecret"' ?

[krauterbaquette]

@gytic could you share the code which produced '--password-store="gnome-libsecret"' ?

Sure, I used this code:

    inputs.wrappers.lib.wrapPackage {
      inherit pkgs;
      package = pkg.signal-desktop;
      args = [ ''--password-store="gnome-libsecret"'' ];
    };

Later on I can also try if args = [ "--password-store=gnome-libsecret" ] works correctly with the escaping

[krauterbaquette]

hmm, the problem could actually be the type of escaping we use here, does --pasword-store='gnome-libsecret' work here?

I tested it without quotes at all, so --password-store=gnome-libsecret and this works fine with signal-desktop.

But it would be nice if this change could be accompanied by a test

I also added a test for the new option

[Lassulus]

after thinking a bit about this, I would prefer to fix the escaping rather than having an option to disable it, It seems in case of signal-desktop there was a way to work around it? So I will leave this unmerged until we find a case where we cannot work around it without disabling escaping

[adrian-gierakowski]

Is escaping really broken? I think in the example provided by @gytic it was the user input that was broken due to misunderstanding of how args are processed.

Also, rather than disabling quoting for all args I would introduce unquotedArgs, with a big fat warming that they could be used to inject arbitrary code.

[krauterbaquette]

Is escaping really broken? I think in the example provided by @gytic it was the user input that was broken due to misunderstanding of how args are processed.

Yeah, this was definetly an Issue on my side. I tried to update the documentation a bit in the new commit to make it more clear, hat argumetns will be escaped.

Also, rather than disabling quoting for all args I would introduce unquotedArgs, with a big fat warming that they could be used to inject arbitrary code.

I removed the escapeArgs boolean and made a new option with an unescapedArgs list.
If you are fine with the implementation I would then suqash those commits into one, so only the final implementation stays. (Or should I open a new PR for this?)