Support patching nixpkgs

1

In many of my projects I occasionally patch nixpkgs with some commits of some PRs that I would like to have but which aren’t merged yet. I always have to copy the same piece of nix code that performs the patching. This is cumbersome and prone to errors. What about adding support in nixpkgs for patching itself?

I added a preliminary implementation for this in:

https://github.com/NixOS/nixpkgs/pull/59990

What do you think? Please comment in the PR if possible.

2 Likes
2

I don’t want to comment in PR, as a don’t object implementation. I’ve been a long-time user of patched-nixpkgs. And what I’d like to say, there are downsides compared to channels:

  • development of patch is cumbersome. Often patches do not apply cleanly on top of your desired rev. Occasionally you have to rebase your patch or improve it. This will require a nixpkgs checkout, but if you have checkout, why use patched-approach?
  • conflicts when patch is merged. This contrasts much with module merge logic (if module is already included, it won’t cause “duplicate module” error)
  • this isn’t specific to patched approach, but GC collects all those patches, as they occur before IFD. So be ready to redownload patches after each GC. You’ll have to redownload nixpkgs checkout as well, if you don’t take care to gcroot it

But it doesn’t mean patched-nixpkgs isn’t good. Instead I think patched-nixpkgs lacks more tooling than just missing support in Nixpkgs to make it useful:

  • patched-nixpkgs is kinda “nixpkgs-flavor” defined in 1 file (instead of defining it as a repo). Which allows distributing this 1 file instead of distributing a repo. However “repo”-view is more natural and easy-to-use. What I’d want is a script that transforms 1-file into a repo view and a repo back into 1-file view.
  • I’d wish that script could trim repo history to the minimal needed one
  • using repo instead of file to bootstrap nixpkgs will remove the need for IFD
  • git lacks “apply-patch-from-url” command, unfortunately. Using 1-file view would solve this problem, but now you have to find out hash with TOFU. So better add apply-from-url command to git.

Thoughts?

3

I answer about my approach to managing patched nixpkgs (more details are in PR comments).
It is a bit different than basvandijk’s, but the evangelistic part should be the same.

nixpkgs is checked out using fetchFromGitHub and kept immutable.
So it is 6 lines of code to keep attention on, not a git-cloned directory

Rebase, yes.
Keeping the patch as a Github pull request takes care on the rebases almost automatically :slight_smile:

There must be an issue in Nix. I tried to fix it (it a way inacceptable to push upstream - by parsing nix -vv output), then it became broken again.
It would be nice to see it fixed in Nix, or at least to see a discussion/RFC about adding IFD closures to GC root.

The main reason against the repo is combinatorial explosion of number of branches if there is more than one laptop to manage.
Unlike repo-branches, patches can be combined.
Some of the patches can be applied only for particular nixops machines (for example, undo meltdown/spectre patches only on database servers).
The could be higher-order functions on patches.
The could be pull requests sent to GitHub - NixOS/nixpkgs: Nix Packages collection & NixOS each of them can have additional commits (by me or by others) and so the changes on Github are to be backported into each of relevant branches in the local nixpkgs repo.

Repo might be more natural, but patches are closer to “infrastructure as code”.

1 Like
4

Agree.

What I talk about is to rethink patched-nixpkgs. It is easy to distribute 6 lines of code, but it’s less fun when you have to amend/fit for any purposes.

So, there should be distribution format (smbd talked about flakes?), which is used to create a repo checkout. Even when someone does IFD nixpkgs, he creates repo checkout in /nix/store. Which is immutable, thus you can’t amend/inspect it. So skip this step, and instead of relying on IFD rely on script, which does “unwrap” of patched-nixpkgs spec into repo checkout.

The spec contains:

  • base nixpkgs checkout rev (is it true that most patched-nixpkgs use includes fetching nixpkgs by rev?)
  • patch spec itself (or revision revert spec)
  • patch description, either meta.description or as comment. It’s easy to get lost in unnamed patches

The spec should be autogenerated from existing repo checkout. Something like git log origin/master..HEAD | preprocess >> spec.nix

$ # patched-nixpkgs is a script that performs download of nixpkgs and
  # applies patches from spec on top of it and publish as local channel
$ nix run nixpkgs.patched-nixpkgs patched-nixpkgs --channel mynixpkgs -f nixpkgs-spec.nix
  # then you can use <mynixpkgs> - no need to IFD
$ nix run nixpkgs.patched-nixpkgs patched-nixpkgs --dir $PWD/mynixpkgs -f nixpkgs-spec.nix
  # then you can export NIX_PATH=nixpkgs=$PWD/mynixpkgs
$ nix run nixpkgs.patched-nixpkgs patched-nixpkgs --shell -f nixpkgs-spec.nix
  # now you enter a subshell, which has NIX_PATH set for you. Similar to nix-review

One extra step to deployment, but no more IFD. I think NixOps can be taught to do same for each spec.

The approach proposed by @basvandijk will force users to use this API for patched-nixpkgs, but those users will soon give-up it because maintenance of patches isn’t rewarding it’s benefits. I’m very surprised you @volth get little troubles.

Another hurdle is Github down events. By using patched-nixpkgs spec and IFD it may ruin your deploys.

5

The mentioned above issue about GC’ing IFD, just to keep it linked Nix GC collects derivations used for IFD · Issue #719 · NixOS/nix · GitHub

6

I think, bootstrapPkgs has good chances to be adopted independent on patching-support, and then, having bootstrapPkgs on board, adding patching-support would be trivial.

There are a lot of IFD cases which won’t disappear (even with adopting proposals like “ret-cont nix” which will help to avoid some IFD)

For example

nixpkgs.overlays = [
  (import (bootstrapPkgs.fetchFromGitHub { # <- `pkgs' here would result in infinite recursion
    owner   = "nixos-rocm";
    repo    = "nixos-rocm";
    rev     = "b1698fd5af692ae07b4eb78887c1d60e279e115c"; # 2019-04-23
    sha256  = "1spb21k3ri8kjv1j6cpc8pqrgdd3mhsb68ssfljx44qkn71lsgfk";
  }))
];

or

( let
    git-describe = (builtins.exec ["/bin/sh" "-c" ''
        set -e
        ( PATH=$PATH:${bootstrapPkgs.git-crypt}/bin
          cd ${lib.escapeShellArg bootstrapPkgs.path}
          ${bootstrapPkgs.coreutils}/bin/echo -n "{ x = \"$(${bootstrapPkgs.git}/bin/git describe --tags --dirty)\"; }"
        )
      '']).x;
    revision = lib.replaceStrings ["m-" "-g" "\n"] ["" "-" ""] git-describe;
    arch = if (pkgs.hostPlatform.platform?gcc.arch) then "-${pkgs.hostPlatform.platform.gcc.arch}" else "";
  in {
    system.nixos.revision      = "${revision}${arch}";
    system.nixos.versionSuffix = ".${revision}${arch}";
  }
)