Result of nixpkgs-review pr 107892 run on x86_64-linux 1

Result of nixpkgs-review pr 107892 run on x86_64-linux 1

Result of nixpkgs-review pr 107892 run on x86_64-darwin 1

Done, and squashed.

It looks like "PREFIX=$(out)" is the preferred way of making makefiles install to the nix store, so I've changed it to that, from having it set both PREFIX and DESTDIR.

Result of nixpkgs-review pr 107892 1

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/317

@wishfort36 can you add support to patches?

@AndersonTorres: Consider it done.

I'm looking at st's default.nix and am thinking. Could it be a good idea to have an extraLibs as well? Some patches will probably require more build dependencies. This one does, for example. I could get it building anyways by just overriding the buildInputs:

{
  packageOverrides = pkgs: rec {
    herbe = pkgs.herbe.overrideAttrs (attrs: {
      patches = attrs.patches ++ [ /home/wishfort36/21.diff ];
      buildInputs = attrs.buildInputs ++ [ pkgs.xorg.libXrandr ];
    });
  };
}

and rebuilding, but it doesn't exactly feel "correct" per se. I guess the same would go for overriding the prePatch or postPatch to get your own config file handled by Nix (for example to set a systemwide colourscheme in Nix), but having a patches feels like it would go hand in hand with an extraLibs.

IMO, if the common way to extend this program is using patches, I think adding an extraLibs is also fine. Anything more than this would probably be better done using overrideAttrs, since we can consider them "strange user cases".

It's a good idea to me. This is a "standard" approach to softwares configured by patches, indeed.
Can you implement it?

I've been under the impression that this:

{ stdenv, lib, fetchFromGitHub, libX11, libXft, freetype, patches ? [ ],
  extraLibs ? [ ] }:

stdenv.mkDerivation rec {
  # (...)

  buildInputs = [ libX11 libXft freetype ] ++ extraLibs;

  # (...)

}

would be sufficient, but if I do that and modify my ~/.config/nixpkgs/nix to this:

{
  packageOverrides = pkgs: rec {
    herbe = pkgs.herbe.overrideAttrs (attrs: {
      patches = attrs.patches ++ [ /home/wishfort36/21.diff ];
      extraLibs = with pkgs; [ xorg.libXrandr ];
#      buildInputs = attrs.buildInputs ++ [ pkgs.xorg.libXrandr ];
    });
  };
}

then compilation fails, complaining that it can't "find" the xrandr header file, as if libXrandr isn't present. It compiles fine if i comment out the extraLibs line and uncomment the buildInputs line. Might you know why? I'm at a loss.

@wishfort36 If you're using { stdenv, lib, fetchFromGitHub, libX11, libXft, freetype, patches ? [ ], extraLibs ? [ ] }: you need to use override and not overrideAttrs. overrideAttrs is for those cases when you want to modify the derivation itself, not the input parameters.

In this case you want to (sorry if this is wrong, I am doing it by head, I may forgetting something):

{
  packageOverrides = pkgs: rec {
    herbe = pkgs.herbe.override ({
      patches = [ /home/wishfort36/21.diff ];
      extraLibs = with pkgs; [ xorg.libXrandr ];
    });
  };
}

Thanks! That worked. I've also pushed the extraLibs change.

Result of nixpkgs-review pr 107892 1