dune icon indicating copy to clipboard operation
dune copied to clipboard

chore(nix): support build for experimental feature

Open maiste opened this issue 1 year ago • 2 comments

Following the merge of #10724, I have written this PR so we are able to build the executable using Nix Flake. I'm far from being a Nix expert, so if someone with a better expertise is able to reduce the amount of code, it would be happy to change the code :smile:

EDIT: Replace my solution with @gridbugs' one, as it is simpler and more elegant one. It consists in adding a function to the package default that could take some flags.

maiste avatar Aug 23 '24 15:08 maiste

Heads up that I pushed a commit that fixes the failing nix test.

gridbugs avatar Aug 28 '24 06:08 gridbugs

I pushed another change to remove the duplication of the experimental flags.

gridbugs avatar Aug 28 '24 06:08 gridbugs

I think it's simpler and more idiomatic to use overrideAttrs to update the configureFlags of an existing derivation, I'll try something

emillon avatar Sep 02 '24 14:09 emillon

how does it look @maiste ? I did a quick test and it seems to work but I'm not super familiar with the experimental flags so let me know if the resulting binaries work as you expect

emillon avatar Sep 02 '24 15:09 emillon

On @gridbugs and my computer, the produced binary works as expected:

  • No need to add some flags to get the compiler installed (no error about Ocaml when doing the dune pkg lock command)
  • It displays the packages installed nicely when running dune build

EDIT: I didn't realize I didn't update the branch before trying. I retried with the musl version, and it worked! :ok_hand:

maiste avatar Sep 02 '24 15:09 maiste

Nice work @emillon, I didn't know about overrideAttrs. I gave this a test on both macos-aarch64 and linux-x86_64 and both binaries have the features I expected.

gridbugs avatar Sep 03 '24 05:09 gridbugs

I have rebased and cleaned the history. If the tests pass, I think it's ok to merge it :+1:

maiste avatar Sep 03 '24 09:09 maiste

Nice work @emillon, I didn't know about overrideAttrs

The part that made it click for me is this comment - derivations are mkDerivation calls and overrideAttrs changes the input of this function.

{ stdenv, bar, baz }: # this part gets overriden by `override`
stdenv.mkDerivation { # This part gets overriden by overrideAttrs
  pname = "test";
  version = "0.0.1";
  buildInputs = [bar baz];
  phases = ["installPhase"];
  installPhase = "touch $out";
}

emillon avatar Sep 03 '24 10:09 emillon

@gridbugs @emillon, I don't have the merge right: can I gently ask one of you to do the merge? :)

maiste avatar Sep 04 '24 07:09 maiste