Precision strike

[lheckemann]

Motivation for this change

#21852 It would be nice to allow removing specific references rather than all references besides the ones specifically listed.

Also includes a sample usage for syncthing. I think this should be used in more places (e.g. buildGoPackage, perhaps some places that currently use nuke-refs), but was hoping to get some feedback on the idea, the implementation and the name (chosen by analogy to “nuke”).

Things done

• Tested using sandboxing
  (nix.useSandbox on NixOS,
  or option build-use-sandbox in nix.conf
  on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • Linux
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

[mention-bot]

@lheckemann, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peterhoeg, @pshendry and @jokogr to be potential reviewers.

[joachifm]

Here $storeId is not escaped, is that intentional?

[lheckemann]

Good call, fixed.

[abbradar]

No problems at first glance, thanks! I like the name :D

I'll merge in several days if noone beats me to it.

[lheckemann]

Turns out it's actually rather broken because apparently I don't test this kind of stuff properly. Hold on the merge...

[lheckemann]

Fixed, now happy to have it merged.

[joachifm]

I'd like to merge but can't help but ask if we could do away with the perl dep and just use sed for the substitution, or is there some special sauce in the perl invocation not easily replicable with sed?

[lheckemann]

Yes, that should be fine. IIRC it's used in nuke-refs (which I copied from) because that requires lookahead, but this doesn't so I'll replace it with sed.

[lheckemann]

@joachifm done. This assumes it's GNU sed, I hope that's okay. This is because of inconsistency across sed implementations in terms of handling the -i flag — I remember encountering issues with using -i on OSX sed in the past.

[joachifm]

From what I can tell, all the stdenvs use the GNU tools so it should be okay (sed -i is used a lot already). It's an unfortunate inconsistency though ...

[polendri]

Just my two cents: I don't think tools like these should have novelty names. Something like pruneReferences would more accurately describe what it does, particularly in contrast to the similar nukeReferences.

[lheckemann]

I do agree that the novelty name feels inappropriate. However, I think in this case it describes the functionality a bit better than something like pruneReferences which to me doesn't convey how it's targetting very specific references very particularly. Definitely open to a better name though.

[bjornfor]

killReferences?

[polendri]

excludeReferences?

[lheckemann]

killReferences?

Also doesn't really convey that it's killing specific references rather than all of them.

Precision Ref Strike. [sorry…]

[bjornfor]

removeReferences?

[joachifm]

removeReferencesTo

[lheckemann]

Changed name to removeReferencesTo, rebased on master, and switched to using writeScriptBin (a change that might make sense for nuke-refs as well).

[joachifm]

This LGTM. An idea for the future: what if there was a stdenv attribute called removeReferencesTo and a standard setup hook that'd process all outputs and strip out those references automatically, obviating the need to pass removeReferencesTo explicitly & add that boiler plate to postFixup?

[lheckemann]

Yes, I like that! That also allows expressing the outputs as a nix list rather than manual calls to removeReferencesTo. The more declarative, the better!

[joachifm]

Seems to me to work as advertised. Thank you

[joachifm]

@lheckemann this is an awful hackup of what it could look like (sans stdenv integration, so the hook still needs to be added to inputs manually): https://github.com/joachifm/nixpkgs/tree/remove-reference-to-setup-hook An actual implementation I think would end up unifying the two and add some checks for robustness and whatnot, but the demo seems to work.

Not sure if it'll be used enough to warrant this sort of integration, though it'd save quite a bit of boilerplate.

[Profpatsch]

This copies the builder of nuke-references nearly verbatim. Is there no way to factor that out? @lheckemann

[lheckemann]

The only code that's almost copied verbatim is the option-getting part, and the code implementing the actual functionality is very different. I don't think it would be very beneficial to make the option-getting code common to the two.