NotAShelf / nvf

A highly modular, extensible and distro-agnostic Neovim configuration framework for Nix/NixOS.
https://notashelf.github.io/nvf/
MIT License
209 stars 28 forks source link

Build failing after updating nixpkgs-unstable with `ln: failed to create symbolic link '/nix/store/...-vimplugin-treesitter-grammars/parser/lua.so': File exists` #368

Closed uncenter closed 1 month ago

uncenter commented 2 months ago

⚠️ Please verify that this bug has NOT been reported before.

Description

Started getting this ln: failed to create symbolic link '/nix/store/...-vimplugin-treesitter-grammars/parser/lua.so': File exists build failure on both my Darwin and WSL machines after updating my nixpkgs input (which nvf follows) to the latest nixpkgs-unstable. I should note that the install: skipping file '/dev/fd/63', as it was replaced while being copied line in the logs below is not relevant, see https://github.com/NixOS/nixpkgs/issues/335016.

👟 Reproduction steps

https://github.com/uncenter/flake

No changes to my flake configuration, just update(s) to Nixpkgs seemingly broke this. Not sure where this issue is coming from, https://github.com/NixOS/nixpkgs/pull/319233 might have made it into nixpkgs-unstable around the time I updated the input?

👀 Expected behavior

Build succeeding for nvf.

😓 Actual Behavior

Build failed for nvf.

💻 Metadata

📝 Relevant log output

error: builder for '/nix/store/m25bdw4b80qyzv8qwby5gfzdgw9j59m6-vimplugin-treesitter-grammars.drv' failed with exit code 1;
       last 2 log lines:
> install: skipping file '/dev/fd/63', as it was replaced while being copied
> ln: failed to create symbolic link '/nix/store/8r6a1wb959pq23g1vv0p23h1wml5lnlq-vimplugin-treesitter-grammars/parser/lua.so': File exists
       For full logs, run 'nix log /nix/store/m25bdw4b80qyzv8qwby5gfzdgw9j59m6-vimplugin-treesitter-grammars.drv'.
error: 1 dependencies of derivation '/nix/store/ggpgscm87iang25yvfr154fimlflizhg-start-configdir.drv' failed to build
error: 1 dependencies of derivation '/nix/store/sni4sn24a8bwdscqawx2j4az2bc0idsb-neovim-pack-dir.drv' failed to build
error: 1 dependencies of derivation '/nix/store/k0bhynvxxvh1kf2gl4zc5agnqhibafbp-mnw-nvf-0.10.1.drv' failed to build
uncenter commented 2 months ago

I was looking at #361 but as far as I can tell this is a different issue, though again related to neovim-pack-dir.

NotAShelf commented 2 months ago

CC @Gerg-L since he mentioned breaking darwin before.

Gerg-L commented 2 months ago

Treesitter build failure prob nixpkgs fault

NotAShelf commented 2 months ago

To nobody's surprise...

stephen-huan commented 2 months ago

Not sure where this issue is coming from, NixOS/nixpkgs#319233 might have made it into nixpkgs-unstable around the time I updated the input?

Treesitter build failure prob nixpkgs fault

I wrote https://github.com/NixOS/nixpkgs/pull/319233; I haven't tested with my personal NixOS configuration yet but it should have been built when I ran the package tests and others (r-vdp, PerchunPak) say it works for them. I ran nix build .#vimPlugins.nvim-treesitter.tests.check-queries on the latest nixpkgs-unstable (https://github.com/NixOS/nixpkgs/commit/4f9cb71da3ec4f76fd406a0d87a1db491eda6870) and it works for me.

The error message (file exists) is particularly strange, either that means there's two different grammars named lua (unlikely) or the build is somehow in an improper state, since there should never be files left from previous builds. Does nix store verify --all do anything? Have you bisected the error to the PR https://github.com/NixOS/nixpkgs/pull/319233?

FrothyMarrow commented 2 months ago

It's happening because it's trying to copy the Lua parser twice.

The Lua language module adds the Lua parser to vim.treesitter.grammars -> vim.treesitter gets enabled -> Treesitter module tries to add all the vim.defaultGrammars to vim.treesitter.grammars (which includes Lua) -> triggers the Lua parser copy again.

As a workaround, you can disable adding the default grammar by setting vim.treesitter.addDefaultGrammars to false and add the the grammars you need manually to vim.treesitter.grammars.

The changes introduced in https://github.com/NixOS/nixpkgs/pull/319233 don't handle duplicate entries.

stephen-huan commented 2 months ago

I see. Should I change the ln -s to ln -sf? Or try to de-duplicate in a more clever way? I am not sure exactly how neovim handles two parsers with the same name in different runtimepath directories. I suppose it might take the first one, since I've had situations where the treesitter grammars provided by nix work but not the built-in grammars.

The module could also filter grammars with duplicate names before passing it to nvim-treesitter.withPlugins.

https://github.com/NotAShelf/nvf/blob/c757d28ff768ae5c8522c3059dfb8fb90e74ba07/modules/default.nix#L52-L55

FrothyMarrow commented 2 months ago

To handle multiple revisions of parsers, I suggest adding a suffix to each one to tell them apart, considering Neovim supports multiple parsers.

Neovim loads the first available parser and parsers are searched for as parser/{lang}.* in runtimepath according the docs.

nix-repl> pkgs.vimPlugins.nvim-treesitter.builtGrammars.c.name
"c-grammar-0.0.0+rev=deca017"

Grammars already have a revision tag, so it should be an easy task. Identical parsers are unintentional, so the error makes sense. Not sure how it was handled before the changes, though.

ppenguin commented 2 months ago

It's happening because it's trying to copy the Lua parser twice.

The Lua language module adds the Lua parser to vim.treesitter.grammars -> vim.treesitter gets enabled -> Treesitter module tries to add all the vim.defaultGrammars to vim.treesitter.grammars (which includes Lua) -> triggers the Lua parser copy again.

As a workaround, you can disable adding the default grammar by setting vim.treesitter.addDefaultGrammars to false and add the the grammars you need manually to vim.treesitter.grammars.

The changes introduced in NixOS/nixpkgs#319233 don't handle duplicate entries.

Thanks, will try that! As an extra data point (probably amounting to the same root cause): Inspecting the closures with nix-tree after setting vim.languages.enableTreesitter = false I still get an almost 400MB closure called start-configdir with a dependency vimplugin-treesitter-grammars. But I guess from that PoV the merging behaviour under vim.languages... should be made to correctly take care of duplicates?

NotAShelf commented 2 months ago

I thought it could have been mnw, but it's not.

As far as I remember, we add a few treesitter grammars by default to suppress Neovim health-checks. Markdown, markdown in-line, vim, Lua and one or two more. Those could be the reason behind the increased closure size. I will need to take a look.

I have also been meaning to switch to https://github.com/viperML/tree-sitter/ for the grammars. Maybe this is a good excuse to do so.

ppenguin commented 2 months ago

@NotAShelf Just to let you know: the (perceived) large size of start-configdir seems to be mostly caused by markdown-preview.nvim-2023-10-17, so it's not "the fault" of any duplicates...

BirdeeHub commented 2 months ago

ngl the by-default grammars with neovim-unwrapped are a real pain but theyre tiny

when doing lazy.nvim you have to do rtp reset = false and THEN do the rtp reset yourself and place them before the builtins in the rtp when you do so.

if they were removed, simply rtp.reset = false would work

The nixpkgs ones only work with the wrappers because they are in the packpath which is first on the rtp so they shadow the builtins

BirdeeHub commented 2 months ago

also, withAllGrammars didnt include any queries dirs until recently, but they worked before? and when those were added some parsers started having issues when lazy loaded, meaning there are some being included for all the grammars not just the builtins, SOMEHOW?

The issue is IDK how they were being included before, but it still finds them with just the parsers in the rtp.

Does anyone have any insight for me as to how the queries are being resolved when only the parser is in the rtp?

horriblename commented 2 months ago

withAllGrammars didnt include any queries dirs until recently, but they worked before? 

nvim-treesitter has queries for many languages so that's probably where

https://github.com/nvim-treesitter/nvim-treesitter/tree/master/queries

you can use vim.api.nvim__get_runtime({'queries/rust'}, true, {}) to list matching dirs in rtp

BirdeeHub commented 2 months ago

TYSM theyre literally copied into the nvim-treesitter directory hahaha now to hunt down from where.

Edit: oh literally just from the repo..... hmmmm

IDK what to do with this information tbh... ill have to sit on it for a bit

BirdeeHub commented 2 months ago

anyway thank you so much I somehow forgot to look at the original source when I was looking at nixpkgs nvim-treesitter....

PerchunPak commented 1 month ago

https://github.com/NixOS/nixpkgs/pull/339589 (tracker) was just merged, so this can be closed. Please ping me if you see any other issues with current solution

NotAShelf commented 1 month ago

I'll keep this open until the PR hits unstable so that I don't forget about it.

NotAShelf commented 1 month ago

But looks like the universe wants it closed...

PerchunPak commented 1 month ago

The fix is in the nixos-unstable! Though not yet in nixpkgs-unstable

ppenguin commented 1 month ago

The fix is in the nixos-unstable! Though not yet in nixpkgs-unstable

:+1: Cool, usually it's the other way around though, or am I mistaken?

BirdeeHub commented 1 month ago

usually yeah. idk why but like 25% of the time its not XD

xvrqt commented 1 month ago

TIL

NotAShelf commented 1 month ago

Ignore my previous silly comment...

I consider this issue resolved, but I will keep it open until I am able to test the unstable branch myself. Thank you all for chiming in.

NotAShelf commented 1 month ago

Closed as the fix is now in all branches, the nixpkgs input has been updated.

P.S. @FrothyMarrow I know you love me, stop being toxic

PerchunPak commented 1 month ago

fyi revert that should fix every other problem was just merged https://github.com/NixOS/nixpkgs/pull/341079 (tracker)