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

Nix flake #6650

Closed
wants to merge 2 commits into from
Closed

Nix flake #6650

wants to merge 2 commits into from

Conversation

nothingmuch
Copy link

@nothingmuch nothingmuch commented Sep 7, 2023

per @chrisguida's request, cc @jurraca, opening a draft PR for a new flake.nix

disabling doCheck = true is enough to build from source using nix build ".?submodules=1", and in nix develop the python interpreter is set up with the poetry dependencies, but that's as far as I got, this does not yet work.

TODO:

  • reuse upstream clightning package
  • poetry2nix based python environment
    • figure out why callPackages python override from poetry2nix built environment is not doing the right thing, currently there's an ugly workaround (prepending to nativeBuildInputs)
    • ensure pyproject.toml & poetry.lock files are adequately imported, and poetry2nix is used correctly WRT multiple subdirectories in contrib (do they need separate derivations?)
    • use nix supplied python environment in make check
      • right now i just deleted PYTHONPATH bits from the Makefile to get the wrapped interpreter in the build environment, this works for some deps but not pyln-testing
  • get doCheck in derivation working
  • devshell with additional tools
    • nixpkgs.poetry (poetry itself is not supposed to be a dep of pyproject.toml)
    • bitcoin core (required for testing)
    • valgrind, etc - other tools used for --enable-developer
  • crane based rust derivations
  • nix flake checks
    • various extended developer tests could be set up with source filtering for efficient caching, this would make git rebase --exec 'nix flake check' pretty reasonable

defines a default package:
- pname is now "cln" because why not
- version uses flake input metadata, doesn't match git describe format

building requires submodules:

    nix build ".?submodules=1"

the devshell does not, project can be built from checkout with submodules in
this devshell.
description = "A very basic flake";

inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/23.05";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use the unstable branch, FYI this currently causes an infinite cascade of derivations due to a breaking change in the nixpkgs python stuff that poetry2nix uses, you would need to pin nixpkgs to just before the merging PR mentioned in this thread:

https://discourse.nixos.org/t/psa-poetry2nix-is-currently-broken-with-nixpkgs-unstable/32281

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably shouldn't use unstable anyway since it would reevaluate packages every time, not a great UX. Pinning it to the latest stable release seems ok.
We could also figure out which packages we want bleeding edge for and cherry pick those from nixpkgs-unstable, like nix-bitcoin does.

@jurraca
Copy link

jurraca commented Sep 8, 2023

thanks for kicking this off!
I think you may be overcomplicating it by reusing the nixpkgs clightning package. Besides, the point of a flake is to be (mostly) self-contained to the repo. I would just reuse the nixpkgs expression in a local default.nix file or something.
Just thinking out loud about goals here:

  • you can nix build for major systems (x86-64-linux, aarch64-linux, x86_64-darwin, aarch64-darwin)
  • you can nix develop and have a set up dev environment
  • you can run tests across the CLN test suite
  • should handle setting up a local network in regtest, like startup-regtest.sh does.

@cdecker cdecker changed the title Wip flake Nix flake Sep 11, 2023
@chrisguida
Copy link
Contributor

chrisguida commented Sep 11, 2023

Does anyone know of a good repo to look at for an example of what to do here? @jurraca @nothingmuch

The goal is to be able to type nix develop and have a dev env ready to run make.

Even something without python would work, I'm just trying to get my bearings right now.

Will add startup_regtest.sh once I have that working.

Then I'll add the ability to run this from downstream repos, ie plugins that also need the startup_regtest.sh environment.

@nothingmuch
Copy link
Author

nothingmuch commented Sep 16, 2023

thanks for kicking this off! I think you may be overcomplicating it by reusing the nixpkgs clightning package.

yep, that's a hack, only intended (i should have clarified this) to delay making some decisions until later but should not be merged as such since it spreads out knowledge about what is appropriate to do to build the project in a very inconvenient way, introducing a dep on nixpkgs at the wrong level of abstraction (edit: that is, in addition to the appropriate dep on nixpkgs, namely relying on it for (native) build inputs)

  • you can nix build for major systems (x86-64-linux, aarch64-linux, x86_64-darwin, aarch64-darwin)
  • you can nix develop and have a set up dev environment
  • you can run tests across the CLN test suite
  • should handle setting up a local network in regtest, like startup-regtest.sh does.

i would only add to this that it's not just CLN proper but plugin development too that should be facilitated, regardless of whether the plugin lives in this repo or outside of it

@nothingmuch
Copy link
Author

Does anyone know of a good repo to look at for an example of what to do here? @jurraca @nothingmuch

not off the top of my head

The goal is to be able to type nix develop and have a dev env ready to run make.

that's what i was going for in the first commit but it's somewhat incomplete

Even something without python would work, I'm just trying to get my bearings right now.

python is important IMO because of make check

@cdecker
Copy link
Member

cdecker commented Feb 16, 2024

FWIW I was able to side-step the requirement for poetry2nix altogether by adding a shell hook that'd just invoke poetry install --with=dev --no-root in it. While not as nicely integrated into nix I think it could get us unstuck trying to get the first iteration working, and we can later start looking for better integrated solutions, or remove/replace dependencies that are hard to nixify.

What do you think @nothingmuch ?

@chrisguida
Copy link
Contributor

@cdecker do you have a link to your flake handy? would love to take a look!

@cdecker
Copy link
Member

cdecker commented Feb 16, 2024

I'm afraid I have it committed in some random branch that I am having trouble finding again 😊 But I will share as soon as I find it.

@JosephGoulden
Copy link
Contributor

I just tried this out. Flake loads once I update it to 24.05, but then I get a stack overflow when building. I can't really help with the C/Make/python stuff but I can write crane based rust derivations.

@chrisguida
Copy link
Contributor

@JosephGoulden can you post the error? Would love to resurrect this and get it working... I can jump on a call to collaborate!

@JosephGoulden
Copy link
Contributor

Lots of logs about instantiating python stuff then a stack overflow.

instantiated 'python3-3.11.9' -> '/nix/store/123855506lv9kabjla4andyqy4an0sih-python3-3.11.9.drv'
performing daemon worker op: 7
instantiated 'wrap-python-hook' -> '/nix/store/18scsq0ah0xdlcp1yq2kdp3y4kn93m27-wrap-python-hook.drv'
performing daemon worker op: 7
instantiated 'python-remove-tests-dir-hook' -> '/nix/store/xp7kphjxcy8p87jjsklr1vyj1k9kqd2l-python-remove-tests-dir-hook.drv'
performing daemon worker op: 7
instantiated 'python-remove-bin-bytecode-hook' -> '/nix/store/8qn2wd6wqgyak7s84rcw8xsg0jw0mz3x-python-remove-bin-bytecode-hook.drv'
evaluating derivation 'git+file:///lightning#packages.x86_64-linux.default'error: stack overflow (possible infinite recursion)

@chrisguida
Copy link
Contributor

Hmm I'm not even getting past nix develop on this branch now...

$ nix develop
error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'cln'
         whose name attribute is located at /nix/store/vhq11h949l5zycaw07acphv53ifq4p2c-source/pkgs/stdenv/generic/make-derivation.nix:303:7

       … while evaluating attribute 'makeFlags' of derivation 'cln'

         at /nix/store/z8ap2lygq7h2hibjfwm5vl610glx4idk-source/flake.nix:37:11:

           36|           version = "${self.lastModifiedDate}-flake-${self.dirtyShortRev}"; # TODO emulate git describe using CHANGELOG.md or fetch git? impure runCommand?
           37|           makeFlags = ["VERSION=${version}" "MTIME=${self.lastModifiedDate}"];
             |           ^
           38|           nativeBuildInputs = [ cln-meta-project ] ++ upstream.nativeBuildInputs; # FIXME why does python3 in callPackage not work? work around by placing cln-meta-project's python first

       error: attribute 'dirtyShortRev' missing

       at /nix/store/z8ap2lygq7h2hibjfwm5vl610glx4idk-source/flake.nix:36:55:

           35|           name = "cln";
           36|           version = "${self.lastModifiedDate}-flake-${self.dirtyShortRev}"; # TODO emulate git describe using CHANGELOG.md or fetch git? impure runCommand?
             |                                                       ^
           37|           makeFlags = ["VERSION=${version}" "MTIME=${self.lastModifiedDate}"];

Did you modify anything to get the dev shell loading?

@JosephGoulden
Copy link
Contributor

Yeah I got the same error at first. I updated inputs.nixpkgs.url to 24.05.

Probably worth trying a rebase after that given the branch is nearly a year old.

@chrisguida
Copy link
Contributor

Hmm @JosephGoulden I tried your change, but i get this:
image

Took like 40GB of RAM before erroring out here xD

Wonder what that's about

@chrisguida
Copy link
Contributor

Tried just getting rid of the flake.lock file and i get this:

$ nix develop
warning: Git tree '/home/cguida/work/lightning/elementsproject' is dirty
warning: creating lock file '/home/cguida/work/lightning/elementsproject/flake.lock'
warning: Git tree '/home/cguida/work/lightning/elementsproject' is dirty
error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'cln'
         whose name attribute is located at /nix/store/p9sy5nf9jdwj69gmr3c3n03npzr5kkqi-source/pkgs/stdenv/generic/make-derivation.nix:336:7

       … while evaluating attribute 'nativeBuildInputs' of derivation 'cln'

         at /nix/store/p9sy5nf9jdwj69gmr3c3n03npzr5kkqi-source/pkgs/stdenv/generic/make-derivation.nix:380:7:

          379|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          380|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          381|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       error: attribute 'legacyPackages' missing

       at /nix/store/3vqkvn1fbdmn3q3phk4h0z52z6xxcbs4-source/flake.nix:24:10:

           23|       inherit
           24|         (poetry2nix.legacyPackages.${system})
             |          ^
           25|         defaultPoetryOverrides

Just kinda shooting in the dark here

@JosephGoulden
Copy link
Contributor

If you change (poetry2nix.legacyPackages.${system}) to (poetry2nix.lib.mkPoetry2Nix { inherit pkgs; }) then you can get a little further.

I did a rebase as well then get stuck at No module named 'hatchling'.

@chrisguida
Copy link
Contributor

Not sure how you're getting that.

I'm getting this:

error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'cln'
         whose name attribute is located at /nix/store/fj33459cnk884y8lrvc7ihxgcgsixxpj-source/pkgs/stdenv/generic/make-derivation.nix:336:7

       … while evaluating attribute 'nativeBuildInputs' of derivation 'cln'

         at /nix/store/fj33459cnk884y8lrvc7ihxgcgsixxpj-source/pkgs/stdenv/generic/make-derivation.nix:380:7:

          379|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          380|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          381|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       error: attribute 'lib' missing

       at /nix/store/l7xkz1nyvk90pgsvwkxgawa2135z3q3d-source/flake.nix:24:3:

           23|       inherit
           24| 	(poetry2nix.lib.mkPoetry2Nix { inherit pkgs; })
             |   ^
           25|         defaultPoetryOverrides

@chrisguida
Copy link
Contributor

Here's my git diff

image

With this diff I get the "infinite recursion" problem described above

@JosephGoulden
Copy link
Contributor

I think we should try a slightly different approach. I created a new PR #7656 which is quite similar to the first commit in this PR.

@ShahanaFarooqui
Copy link
Collaborator

Closing with PR #7656.

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.

6 participants