NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
16.6k stars 13.08k forks source link

tree-sitter: incorrect pname for some grammars #324764

Open mightyiam opened 1 week ago

mightyiam commented 1 week ago

EDIT: skip this and read the recap


Describe the bug

The grammars tree-sitter.builtGrammars.tree-sitter-{org-nvim,markdown-inline} have a wrong pname: org-grammar and markdown_inline-grammar, respectively. While the rest of the grammars would have the correct pname; for example tree-sitter-perl-grammar.

Steps To Reproduce

Steps to reproduce the behavior:

$ nix eval .#tree-sitter.builtGrammars.tree-sitter-org-nvim.pname
"org-grammar"
$ nix eval .#tree-sitter.builtGrammars.tree-sitter-perl.pname
"tree-sitter-perl-grammar"

Additional context

Discovered during #320783.

Seems to have been introduced in #156911. Argument language seems to never have been used, introduced in cf817454041353a235f02d34245e0873a072f31a. PR for fixing this coming up.

Notify maintainers

@Profpatsch

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.36, NixOS, 24.11 (Vicuna), 24.11.20240704.1afc544`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.23.1`
 - channels(mightyiam): `""`
 - channels(root): `""`
 - nixpkgs: `/nix/store/xxmxxf2s5c4jq98yk5mr9crm2ig4mv28-source`

Add a :+1: reaction to issues you find important.

@stepbrobd

stepbrobd commented 1 week ago

From these lines:

https://github.com/NixOS/nixpkgs/blob/c9d9fd0c619e2f73f94e4a79abad722e578ff7f1/pkgs/development/tools/parsing/tree-sitter/grammar.nix#L10-L11

https://github.com/NixOS/nixpkgs/blob/c9d9fd0c619e2f73f94e4a79abad722e578ff7f1/pkgs/development/tools/parsing/tree-sitter/grammar.nix#L20

It seems that the correct format should be <language>-grammar.

mightyiam commented 2 days ago

Recap, because we noticed nuance and changed our minds:

Problem

This is about the pnames of packages in #tree-sitter.builtGrammars (they show up in other paths, as well, by the way).

From the implementation code, it would appear as if their pnames should be <language>-grammar:

https://github.com/NixOS/nixpkgs/blob/c9d9fd0c619e2f73f94e4a79abad722e578ff7f1/pkgs/development/tools/parsing/tree-sitter/grammar.nix#L10-L11

https://github.com/NixOS/nixpkgs/blob/c9d9fd0c619e2f73f94e4a79abad722e578ff7f1/pkgs/development/tools/parsing/tree-sitter/grammar.nix#L20

But in reality, only two of them (#tree-sitter.builtGrammars.tree-sitter-{org-nvim,markdown-inline}) have these pnames. The rest have tree-sitter-<language>-grammar.

And that is because that implementation is called with the value provided as language being "tree-sitter-<language>" except for the two mentioned above. Here that is:

https://github.com/NixOS/nixpkgs/blob/c9d9fd0c619e2f73f94e4a79abad722e578ff7f1/pkgs/development/tools/parsing/tree-sitter/default.nix#L58

Suggested "solution A"

Accept the pname format tree-sitter-<language>-grammar — which all but two packages have. And refactor the implementation to make that appear intentional. Not as a means of covering up the blooper :sweat_smile:, though.