nix-community / neovim-nightly-overlay

[maintainer=@GaetanLepage, @willruggiano]
https://matrix.to/#/#neovim-nightly-overlay:nixos.org
287 stars 39 forks source link

feat: move neovim flake into the overlay itself #516

Closed GaetanLepage closed 1 month ago

GaetanLepage commented 1 month ago

Maintaing the flake in the official neovim repository became too much of a burden so we are moving it out to a nix-community repository.

This is the opportunity to do more and add automation so that the neovim nix overlay doesnt ever break !

Follow up of #515

willruggiano commented 1 month ago

Probably also worth noting in the README how one can --override-input neovim-src, which is useful for developing against the neovim source code.

GaetanLepage commented 1 month ago
❮ nix flake check
error: builder for '/nix/store/lppg8w1ldhrsdhywg00x2v2jiin9j9pc-shlint.drv' failed with exit code 2;
       last 1 log lines:
       > make: *** No rule to make target 'shlint'.  Stop.
       For full logs, run 'nix log /nix/store/lppg8w1ldhrsdhywg00x2v2jiin9j9pc-shlint.drv'.

@willruggiano any idea ?

GaetanLepage commented 1 month ago

Are you guys fine with using alejandra as the formatter ?

willruggiano commented 1 month ago
❮ nix flake check
error: builder for '/nix/store/lppg8w1ldhrsdhywg00x2v2jiin9j9pc-shlint.drv' failed with exit code 2;
       last 1 log lines:
       > make: *** No rule to make target 'shlint'.  Stop.
       For full logs, run 'nix log /nix/store/lppg8w1ldhrsdhywg00x2v2jiin9j9pc-shlint.drv'.

@willruggiano any idea ?

Looks like upstream removed it.

willruggiano commented 1 month ago

re shlint, looks like these targets are the ones to use now: https://github.com/neovim/neovim/blob/01b4da65c229f05ccb26c55db4e0d30ed9bac10b/Makefile#L119

Although I don't particularly see the value in having those as flake checks? It's an extra burden on the maintainers of this repo to ensure they're in sync with upstream. Also I guess it kinda depends if we want the nightlies to only include upstream nightlies that pass these checks. Could go either way honestly

GaetanLepage commented 1 month ago

re shlint, looks like these targets are the ones to use now: https://github.com/neovim/neovim/blob/01b4da65c229f05ccb26c55db4e0d30ed9bac10b/Makefile#L119

Although I don't particularly see the value in having those as flake checks? It's an extra burden on the maintainers of this repo to ensure they're in sync with upstream. Also I guess it kinda depends if we want the nightlies to only include upstream nightlies that pass these checks. Could go either way honestly

@teto , what's your opinion on this ? Should we drop the checks altogether ?

GaetanLepage commented 1 month ago

The references of neovim-nightly in the README should also be changed for neovim right ?

GaetanLepage commented 1 month ago
neovim-nightly-overlay on  flake 
❮ nix build --show-trace
error:
       … while calling the 'derivationStrict' builtin

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

       … while evaluating derivation 'neovim-unwrapped-nightly'
         whose name attribute is located at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/pkgs/stdenv/generic/make-derivation.nix:331:7

       … while evaluating attribute 'preConfigure' of derivation 'neovim-unwrapped-nightly'

         at /nix/store/8g5455rmasxdrkas1b8f8ycscjavvcp1-source/flake/packages/neovim.nix:86:5:

           85|     inherit src;
           86|     preConfigure = ''
             |     ^
           87|       ${oa.preConfigure}

       … while calling anonymous lambda

         at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/lib/attrsets.nix:1096:10:

         1095|     attrs:
         1096|     map (name: f name attrs.${name}) (attrNames attrs);
             |          ^
         1097|

       … from call site

         at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/lib/attrsets.nix:1096:16:

         1095|     attrs:
         1096|     map (name: f name attrs.${name}) (attrNames attrs);
             |                ^
         1097|

       … while calling anonymous lambda

         at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/pkgs/by-name/ne/neovim-unwrapped/package.nix:172:18:

          171|     '' + lib.concatStrings (lib.mapAttrsToList
          172|       (language: src: ''
             |                  ^
          173|         ln -s \

       … while evaluating derivation 'bash-grammar-neovim-nightly'
         whose name attribute is located at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/pkgs/stdenv/generic/make-derivation.nix:331:7

       … while evaluating attribute 'src' of derivation 'bash-grammar-neovim-nightly'

         at /nix/store/0kl2p60mvnpscnza21p8583sz3f60jxf-source/pkgs/by-name/ne/neovim-unwrapped/package.nix:175:20:

          174|           ${tree-sitter.buildGrammar {
          175|             inherit language src;
             |                    ^
          176|             version = "neovim-${finalAttrs.version}";

       error: cannot coerce a set to a string

Hmmmm

teto commented 1 month ago

look at my PR, that's because neovim expects src = so you need to convert the generated grammar sets into a src= fetchurl attribute entry

willruggiano commented 1 month ago

look at my PR, that's because neovim expects src = so you need to convert the generated grammar sets into a src= fetchurl attribute entry

Sort of. That is how it worked a few days ago, but that commit has since been reverted; https://github.com/NixOS/nixpkgs/commit/76ef4c7888c52bd4eed566011c24da9eb437a3c8

teto commented 1 month ago

damn. We could pin nixpkgs to an earlier version then, the code is going to be re-reverted again after branch-off . We are more interested in neovim bump than nixpkgs-bumps anyway

willruggiano commented 1 month ago

damn. We could pin nixpkgs to an earlier version then, the code is going to be re-reverted again after branch-off . We are more interested in neovim bump than nixpkgs-bumps anyway

Yeah, we should also (temporarily) prevent hercules-ci from auto updating that input. No idea how to do that though lol.

EDIT: looks like hercules-ci.flake-update.flakes.".".inputs = [ ...everything but nixpkgs for now... ]; should do the trick

GaetanLepage commented 1 month ago

The neovim update PR has been re-reverted, so the current neovim version in nixpkgs is now 0.10.0 again. Should I still pin nixpkgs ?

teto commented 1 month ago

it's a bit peculiar to have everything scattered across multiple files but that's an implementation detail. It seems to work. I wonder why the CI check is waiting ? @kranzes is a server unreachable ? I will merge anyway, the neovim core team is impatient to get rid of the flake and the basics seems to work, we can refine the details later. Thanks to everyone involved

teto commented 1 month ago

@Kranzes seems like I can't force a merge without CI. I have limited access to the repository page. Can you merge this/fix CI ? also maybe invite @willruggiano and @GaetanLepage if you agree ?

Kranzes commented 1 month ago

Adding y'all now...

GaetanLepage commented 1 month ago

@Kranzes is the CI doing nix flake check ? or more ? or less ?

Kranzes commented 1 month ago

I added @GaetanLepage but wasn't able to add @willruggiano as they are not part of the nix-community org, can you ask for an invite on the nix-community matrix room? cheers.

Kranzes commented 1 month ago

@Kranzes seems like I can't force a merge without CI. I have limited access to the repository page. Can you merge this/fix CI ? also maybe invite @willruggiano and @GaetanLepage if you agree ?

Hercules-CI currently doesn't support running on forks, you could use some GitHub feature/bot to pull it into a different branch on this repo so it runs, but I haven't needed to figure that out yet. I suggest for further development here you should use your own branch here rather than a fork.

Kranzes commented 1 month ago

@Kranzes is the CI doing nix flake check ? or more ? or less ?

The CI builds all of the outputs and pushes them to the binary cache, it also runs nix flake update every midnight and makes a PR for it and auto-merges it if it builds for Linux.

nixos-discourse commented 1 month ago

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

https://discourse.nixos.org/t/neovims-flake-nix-moved-to-nix-community/45887/1

willruggiano commented 1 month ago

I added @GaetanLepage but wasn't able to add @willruggiano as they are not part of the nix-community org, can you ask for an invite on the nix-community matrix room? cheers.

@Kranzes this one? #nix-community:nixos.org

meetmangukiya commented 1 month ago

had this in my home.nix which doesnt work now:

  nixpkgs.overlays = [
    (import (builtins.fetchTarball {
      url = https://github.com/nix-community/neovim-nightly-overlay/archive/master.tar.gz;
    }))
  ];

is there anything to change here? an update to readme would be nice

GaetanLepage commented 1 month ago

@Kranzes this one? #nix-community:nixos.org

This one: https://matrix.to/#/#neovim-nightly-overlay:nixos.org

GaetanLepage commented 1 month ago

This one: https://matrix.to/#/#neovim-nightly-overlay:nixos.org

Were you able to join @willruggiano ?