Tree sitter parsers #35461

msva · 2024-02-21T10:03:08Z

New grammars.

Most of them also required in runtime by app-edittors/neovim-9999 (and it seems will be required by next non-patch release too)

Ref: https://github.com/neovim/neovim/blob/master/cmake.deps/deps.txt#L46 and below

gentoo-bot · 2024-02-21T10:04:12Z

Pull Request assignment

Submitter: @msva
Areas affected: ebuilds
Packages affected: dev-libs/tree-sitter-lua, dev-libs/tree-sitter-markdown, dev-libs/tree-sitter-markdown-inline, dev-libs/tree-sitter-meta, dev-libs/tree-sitter-query...

dev-libs/tree-sitter-lua: @gentoo/proxy-maint (new package)
dev-libs/tree-sitter-markdown: @gentoo/proxy-maint (new package)
dev-libs/tree-sitter-markdown-inline: @gentoo/proxy-maint (new package)
dev-libs/tree-sitter-meta: @MatthewGentoo, @sarnex
dev-libs/tree-sitter-query: @gentoo/proxy-maint (new package)
dev-libs/tree-sitter-vim: @gentoo/proxy-maint (new package)
dev-libs/tree-sitter-vimdoc: @gentoo/proxy-maint (new package)

Linked bugs

Bugs linked: 922146

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide

MatthewGentoo · 2024-02-21T10:23:15Z

Thanks!

Think that these new grammars need test masking for rustless profiles: https://github.com/gentoo/gentoo/blob/master/profiles/features/wd40/package.use.mask#L79

msva · 2024-02-21T10:52:21Z

@MatthewGentoo done

msva · 2024-02-21T10:58:58Z

Please, leave a note, if all looks good for you, and I'll push it in gentoo myself

(this PR is for review purposes only, I have push access myself, if any)

gentoo-repo-qa-bot · 2024-02-21T11:08:15Z

Pull request CI report

Report generated at: 2024-02-21 11:08 UTC
Newest commit scanned: 6967658
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/b60d008f6a/output.html

MatthewGentoo

The packages look fine, but some of the tests in tree-sitter-markdown-inline are failing.

8 failures:

expected / actual

  1. Basic Wiki-link parsing.:

    (inline
      (shortcut_link
        (link_text)))
      (wiki_link
        (link_destination)))


  2. Wiki-link to a file:

    (inline
      (shortcut_link
        (link_text)))
      (wiki_link
        (link_destination)))


  3. Wiki-link to a heading in a note:

    (inline
      (shortcut_link
        (link_text)))
      (wiki_link
        (link_destination)))


  4. Wiki-link with title:

    (inline
      (shortcut_link
      (wiki_link
        (link_destination)
        (link_text)))


  5. Wiki-link version of Example 556:

    (inline
      (shortcut_link
        (link_text)))
      (wiki_link
        (link_destination)))


  6. Example 324 - https://github.github.com/gfm/#example-324:

    (inline)

    (inline
      (tag)
      (tag))


  7. Example 638 - https://github.github.com/gfm/#example-638:

    (inline)

    (inline
      (tag))


  8. Tags are working:

    (inline)

    (inline
      (tag)
      (tag)
      (tag))

I have dev-libs/tree-sitter-0.20.9-r1.

thesamesam · 2024-02-21T10:19:45Z

dev-libs/tree-sitter-lua/tree-sitter-lua-0.0.19.ebuild

+
+LICENSE="MIT"
+SLOT="0"
+KEYWORDS="~amd64 ~arm ~arm64 ~ppc ~ppc64"


Please don't add them with other keywords: https://devmanual.gentoo.org/keywording/index.html#keywording-new-packages.

I did tested that it compiles fine on all of that arches.
Although, I only tested compilation itself, and didn't tested them in co-operation with "clients".
And also I tested ppc and ppc64 on qemu targets (arm and arm64 on "real" boards (orange pi zero and pine a64+)).
Isn't it enough?
If so - okay, I will remove anything except ~amd64 🤷

No, it's not enough. We do keywording via arch teams via bugs. That's how it always works. You also run the test suite for them, not just building.

You have to ask for permission otherwise.

Can I ask where you got the impression this was the done thing? This is covered in the quizzes and in the devmanual quite clearly?

msva · 2024-02-21T11:32:06Z

but some of the tests in tree-sitter-markdown-inline are failing.

Unfortunately, I have no idea what can I do with that 🤷
(Not sure how I can fix it to pass tests)

Can you give some advice, please?

Maybe just drop markdown-inline grammar?

thesamesam · 2024-02-21T12:01:07Z

First, you check if you can reproduce it.

If you can, you poke at it, and report it upstream?

Did you not run the tests for it before opening the PR?

msva · 2024-02-21T13:26:46Z

If you can, you poke at it, and report it upstream?

Yes, it reproduces here
And, okay, I reported it upstream: tree-sitter-grammars/tree-sitter-markdown#136
(although, I have some doubts that they will fix it in nearest future)

Did you not run the tests for it before opening the PR?

I did. But this grammar passes many other tests, and can still be useful.

Althouth, as it is not a neovim dependency (unlike non-inline markdown grammar), I not insisting on adding it to the gentoo repo until it will pass all tests, and I okay with dropping it from PR 🤷

thesamesam · 2024-02-21T13:34:56Z

I did. But this grammar passes many other tests, and can still be useful.

...? Did you hit the failure, then just not mention it at all? That's bizarre?

msva · 2024-02-21T14:09:28Z

Did you hit the failure, then just not mention it at all

Well, it is really my fault, that I forgot to mention that (altough, that is the reason I created PR for: to other co-maintainers re-checked the things I forgot about).

As "bad excuse" (that actually not excusinng, but sheds a light on reasons why did it happened) for that, I can mention that I'm working on this PR for about a month already (by minor steps when getting some spare hours and not-too-tired after work), so at current point I just forgot about this "speciality" of this not-so-necessary package (that was added mostly because it is co-packaged with not-inline version), and remembered that fact only when @MatthewGentoo mentioned that.

And when I hit that with personal testing, IIRC (that was some time ago), I was guided by the logic like (I mean, "my thoughts was"):

1. it still does most of its work,
2. the failed tests looks like not so related to things where it will be needed as "inline" (and looks like inherited from non-inline ("main") grammar), # although, I'm not a big specialist here :shrug: 
3. this package is far from "important ones", and such fails of tests don't lead to serious system failures, so most probably it is not a reason to not package it at all (especially in case I've many times seen packages that fails tests in the gentoo repo), so I'll mention that later # (here is where I mostly failed)

So, yes, it is definitelly my fault in forgetting about it, but I don't agree with "bizzare".

It is just "artifact" of a process how I worked on this, which was hopefuly catched by a process specifically designed for catching things like this 🤷

Also, as I said, this package (unlike others in the PR) is not a dependency of neovim, so it is not strictly needed.

===

Well, I already dropped it in local copy of branch, and will force-push commits without it 🤷

(I anyway planned to force-push as there is another change: neovim had moved vim grammar to tree-sitter-grammars organization while I worked on other things)

msva · 2024-02-21T14:12:16Z

also, I have dropped arches other than ~amd64 (just in case, although, as I said, I did tested that they compiles there fine).
// Although, I'd like them to be also available on arm64 out-of-the-box.
So, should I cast here anyone for correct arch-testng of arm64?

msva · 2024-02-21T16:56:07Z

@MatthewGentoo I've re-pushed, can you, please, re-review?

Also
@thesamesam @arthurzam @blueness @Flowdalic @leio @NeddySeagoon
can you please correctly arch-test it on ARM64, please? (I have only tested that it compiles)
I'd very like to push it with at least ~amd64 ~arm64

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-util/tree-sitter-cli is a conditional test dep for packages inheriting tree-sitter-grammar.eclass, but is written in rust, so mask it on rustless profiles. Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

msva · 2024-02-21T17:09:11Z

(re-pushed again because of lost (during previous force-push) "Bug" header in grammar commits)

gentoo-repo-qa-bot · 2024-02-21T17:43:19Z

Pull request CI report

Report generated at: 2024-02-21 17:43 UTC
Newest commit scanned: 1893677
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/4f4f9ec582/output.html

msva · 2024-02-21T20:34:13Z

by the way, tree-sitter-markdown's upstream said to "try passing ALL_EXTENSIONS=1".
So, I tried re-generating the things by calling ALL_EXTENSIONS=1 tree-sitter generate --no-bindings in tree-sitter-inline directory, and re-run the test, and it passes all the things.
But as it requires calling of tree-sitter-cli which is unavailable on rustless profiles, I'm not sure about the correct way to go here.

So, I think, I'll leave the inline grammar and will not add an ebuild for it for now.
In case if it will be needed in future - it can be made from non-inline ebuild with minor modificatitons.

thesamesam · 2024-02-22T00:57:31Z

regarding keywords: No, please follow the policy I cited at #35461 (comment). This package does not get special treatment. You get it merged, then you request (re)keywording via a bug.

It is enormous hassle to deviate from that process, especially to handle keywording for things not even in-tree yet.

thesamesam · 2024-02-22T01:03:43Z

Did you hit the failure, then just not mention it at all

Well, it is really my fault, that I forgot to mention that (altough, that is the reason I created PR for: to other co-maintainers re-checked the things I forgot about).

It is just "artifact" of a process how I worked on this, which was hopefuly catched by a process specifically designed for catching things like this 🤷

In future, you should take care to run the test suites (again before pushing) and to mention any test failures immediately in a comment in the ebuild or similar for further investigation.

We can't do much about people forgetting things - that's why it's good to not let some stuff run on for months indeed.

Well, I already dropped it in local copy of branch, and will force-push commits without it 🤷

I am not sure why you're using 🤷 so much. It might appear dismissive.

msva · 2024-02-22T07:48:31Z

In future, you should take care to run the test suites (again before pushing)

okay, I will, thanks

=========

<offtop>

it's good to not let some stuff run on for months indeed.

Well, I totally agree, that it will be good to do it another (better) way, but...

As you can see, it wasn't done by anybody else during that time (for more than month since I mentioned that things in a bug on BZ). So I worked on it in "small steps" manner for that month+ (and would be okay if anybody finished this before me).

Not sure if a month+ is enough time to make any conclusions, aspecially in that case, but I think, I can assume that others also have "timing issues".

So, it looks like there is two possible ways:

doing it in such a bad way like this (for months, by small steps, with corresponding consequences), or
having nothing done at all (because nobody have enough spare time in-a-row to make all the things all-at-once).

// imho

</offtop>

msva · 2024-02-22T07:49:31Z

Btw, @thesamesam, can you, please re-review, and approve the merge if all is okay?

Github shows that you have requested changes, but says that it can't find corresponding things because of force-push

msva · 2024-03-04T20:39:12Z

By the way, I looked on one of my arm-boards (the one with ubuntu), and found that neovim is packaged with markdown-inline there.

/usr/lib/arm-linux-gnueabihf/nvim/parser/markdown_inline.so

So... Then there is again appears a question: should we package it or not.

Without, for example, Lua grammar, nvim fires an error when opening any lua file even with "clean" profile.

Without Markdown grammars (both) nvim doesn't fire errors on opening markdown files (at least, for now).
So, it seems kinda optional for now (even despite upstream bundles both inline and non-inline markdown grammars (among others) on other OSes).

And, if we decide to package (again) inline grammar, would it be okay to regenerate it with tree-sitter-cli (as upstream suggests) before building? It will definitelly make it unavailable (or failing tests) on ~~wd40~~ rustless architectures...

@MatthewGentoo @sarnex WDYT?

MatthewGentoo · 2024-03-05T09:10:49Z

And, if we decide to package (again) inline grammar, would it be okay to regenerate it with tree-sitter-cli (as upstream suggests) before building? It will definitelly make it unavailable (or failing tests) on wd40 rustless architectures...

We could regenerate the parser ourselves with extensions enabled, and upload to dev.gentoo.org. Maybe this is a bad idea though, as it looks like NeoVim doesn't build with the extensions either: https://github.com/neovim/neovim/blob/5b312cd5f67646005312d2ebb2ef0d1bc584dcff/cmake.deps/cmake/MarkdownParserCMakeLists.txt#L17
We could set RESTRICT=test
We could patch to delete the failing tests that require extensions

I think the main thing is you are happy it works with your editor :)

Other grammars look good.

gentoo-bot added new package The PR is adding a new package. assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Feb 21, 2024

MatthewGentoo approved these changes Feb 21, 2024

View reviewed changes

thesamesam suggested changes Feb 21, 2024

View reviewed changes

msva force-pushed the tree-sitter-parsers branch from 6967658 to 6021e8e Compare February 21, 2024 16:44

msva added 7 commits February 22, 2024 00:04

dev-libs/tree-sitter-lua: new package, add 0.0.19

aab838c

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-libs/tree-sitter-query: new package, add 0.1.0

c72fe3b

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-libs/tree-sitter-vim: new package, add 0.3.0

ad5b5c1

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-libs/tree-sitter-vimdoc: new package, add 2.1.0

8048143

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-libs/tree-sitter-markdown: new package, add 0.1.7

df05706

Bug: https://bugs.gentoo.org/922146 Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

profiles/features/wd40: mask USE=test on new tree-sitter-grammar pkgs

c9be289

dev-util/tree-sitter-cli is a conditional test dep for packages inheriting tree-sitter-grammar.eclass, but is written in rust, so mask it on rustless profiles. Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

dev-libs/tree-sitter-meta: new grammars

1893677

Signed-off-by: Vadim Misbakh-Soloviov <mva@gentoo.org>

msva force-pushed the tree-sitter-parsers branch from 6021e8e to 1893677 Compare February 21, 2024 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tree sitter parsers #35461

Tree sitter parsers #35461

msva commented Feb 21, 2024

gentoo-bot commented Feb 21, 2024

MatthewGentoo commented Feb 21, 2024

msva commented Feb 21, 2024

msva commented Feb 21, 2024

gentoo-repo-qa-bot commented Feb 21, 2024

MatthewGentoo left a comment

thesamesam Feb 21, 2024

msva Feb 21, 2024

thesamesam Feb 21, 2024 •

edited

thesamesam Feb 21, 2024

msva commented Feb 21, 2024 •

edited

thesamesam commented Feb 21, 2024

msva commented Feb 21, 2024 •

edited

thesamesam commented Feb 21, 2024

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024

msva commented Feb 21, 2024 •

edited

gentoo-repo-qa-bot commented Feb 21, 2024

msva commented Feb 21, 2024 •

edited

thesamesam commented Feb 22, 2024 •

edited

thesamesam commented Feb 22, 2024

msva commented Feb 22, 2024 •

edited

msva commented Feb 22, 2024 •

edited

msva commented Mar 4, 2024 •

edited

MatthewGentoo commented Mar 5, 2024

Tree sitter parsers #35461

Are you sure you want to change the base?

Tree sitter parsers #35461

Conversation

msva commented Feb 21, 2024

gentoo-bot commented Feb 21, 2024

Pull Request assignment

Linked bugs

MatthewGentoo commented Feb 21, 2024

msva commented Feb 21, 2024

msva commented Feb 21, 2024

gentoo-repo-qa-bot commented Feb 21, 2024

Pull request CI report

MatthewGentoo left a comment

Choose a reason for hiding this comment

thesamesam Feb 21, 2024

Choose a reason for hiding this comment

msva Feb 21, 2024

Choose a reason for hiding this comment

thesamesam Feb 21, 2024 • edited

Choose a reason for hiding this comment

thesamesam Feb 21, 2024

Choose a reason for hiding this comment

msva commented Feb 21, 2024 • edited

thesamesam commented Feb 21, 2024

msva commented Feb 21, 2024 • edited

thesamesam commented Feb 21, 2024

msva commented Feb 21, 2024 • edited

msva commented Feb 21, 2024 • edited

msva commented Feb 21, 2024

msva commented Feb 21, 2024 • edited

gentoo-repo-qa-bot commented Feb 21, 2024

Pull request CI report

msva commented Feb 21, 2024 • edited

thesamesam commented Feb 22, 2024 • edited

thesamesam commented Feb 22, 2024

msva commented Feb 22, 2024 • edited

msva commented Feb 22, 2024 • edited

msva commented Mar 4, 2024 • edited

MatthewGentoo commented Mar 5, 2024

thesamesam Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

msva commented Feb 21, 2024 •

edited

thesamesam commented Feb 22, 2024 •

edited

msva commented Feb 22, 2024 •

edited

msva commented Feb 22, 2024 •

edited

msva commented Mar 4, 2024 •

edited