When to use substituteInPlace functions vs a patch?

Sometimes I need to change a source file and I’m not sure how I should do it. What is a good guideline for when to use each of these?

  • a patch file
  • the bash stdenv substituteInPlace function
  • the nix substituteInPlace function
3 Likes

A patch file usually has the advantage that it fails loudly when it no longer applies. This is important
because we have automatic update mechanism like nixpkgs-update and might miss failing substituteInPlace invocations.
It should be used if you are make logical changes to the code.
substituteInPlace in works well if you change strings i.e. paths to program/libraries with files in the nix store - especially if those occurrences happens in many files.
The nix function substituteAll works well if you have a patch that contains path store references. Use this if you actually want to use a patch but still have some references to the nix store.

4 Likes

In some cases such as python packages which frequently need to have dependency versions changed, failing silently is sometime advantageous because the patches are only necessary to make the build succeed, but not critical for package validity. This allows for nixpkgs-update to continue with updating packages even though a “patch” couldn’t be applied. Obviously if you were doing something like --replace 'run("protoc")' 'run("${protobuf}/bin/protoc")' you would want it to fail loudly, as the patch is critical for it’s runtime behavior.

In general I agree with your other points though :+1:

3 Likes

nixpkgs-review-checks now also checks for stale substitutes. An example can be found here.

I am surprised that substitute and friends will fail to replace anything and continue as if everything’s okay, only printing a warning. I feel like in most cases, outside of exceptional cases like @jonringer mentions, a failed substitution should be inspected. It might be necessary to avoid breaking some aspect of a program. It’s also an opportunity to clean up unneeded substitutions. I see that the underlying replace program can’t indicate via exit code whether any substitutions failed though, unfortunately. I wonder if we could add that functionality, or at least document this limitation of substitute*, and maybe have better guidelines around when to use them vs. sed vs. patches. Failing patches abort the build, build scripts run with set -e… Whatever we recommend for modifying sources deserves the same treatment.

It would also be nice to catch problems earlier on, at build time. @Sandro, I don’t suppose you have any numbers on how many unused substitutions there are across all of Nixpkgs?

2 Likes

I don’t but it could be easily calculated by ripgrepping the logs of all builds on hydra.

3 Likes

Alternately, we could get the numbers from hammering: tests: add integration test to check for crashes against nixpkgs by rmcgibbo · Pull Request #97 · jtojnar/nixpkgs-hammering · GitHub

Right, of course all the build logs are there, good thinking.

Also I need to correct what I said, substitute does not use that replace program; the paragraph I got that from in the stable Nixpkgs manual has been removed in unstable. substituteStream in stdenv is the primitive.