Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flake.nix: adds runHook preInstall/postInstall to installPhase so hooks function #2224

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

ddellacosta
Copy link
Contributor

@ddellacosta ddellacosta commented Jul 14, 2023

Per https://nixos.org/manual/nixpkgs/stable/#sec-stdenv-phases:

When overriding a phase, for example installPhase, it is important to start with runHook preInstall and end it with runHook postInstall, otherwise preInstall and postInstall will not be run. Even if you don’t use them directly, it is good practice to do so anyways for downstream users who would want to add a postInstall by overriding your derivation.

To give an explicit example to motivate this better I hope, this lets me add my own installPhase overrides to llama.cpp when using it as an input in another flake:

llama-cpp-with-includes =
  llama-cpp.packages.${system}.default.overrideAttrs (oldAttrs: {
    postInstall = (oldAttrs.postInstall or "") + ''
      mkdir -p $out/lib
      mkdir -p $out/include
      cp *.a $out/lib/
      cp $src/*.h $out/include/
    '';
  });

This should hopefully allow for more customization for nix users with fewer PRs, and I think this may obviate the need for #1811 as well?

@ddellacosta ddellacosta changed the title adds runHook preInstall/postInstall to installPhase so hooks function flake.nix: adds runHook preInstall/postInstall to installPhase so hooks function Jul 14, 2023
ddellacosta added a commit to ddellacosta/llama.cpp-bindings that referenced this pull request Jul 14, 2023
ggerganov/llama.cpp#2224

...also adds MIT LICENSE, just making it consistent with llama.cpp.
@ggerganov ggerganov merged commit a6803ca into ggerganov:master Jul 14, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants