On 2023-02-10 17:02, Pierre Langlois wrote: > Pierre Langlois writes: > >> [[PGP Signed Part:Undecided]] >> Hi Andrew, thanks for pushing this along! It's great to see things >> getting merged. >> >> Andrew Tropin writes: >> >>> [[PGP Signed Part:Undecided]] >>> On 2023-02-09 18:04, Andrew Tropin wrote: >>> >>>> On 2023-02-09 13:39, zimoun wrote: >>>> >>>>> Hi, >>>>> >>>>> On Thu, 09 Feb 2023 at 14:11, Andrew Tropin wrote: >>>>> >>>>>> I applied tree-sitter and tree-sitter-cli patches, >>>>> >>>>> Just to be sure to understand, you have only applied 02/32 and 05/32, >>>>> right? >>>>> >>>>> >>>>> [bug#49946] [PATCH v7 02/32] gnu: tree-sitter: Update to 0.20.7. >>>>> id:20221125012142.22579-3-pierre.langlois@gmx.com >>>>> http://issues.guix.gnu.org/msgid/20221125012142.22579-3-pierre.langlois@gmx.com >>>>> >>>>> [bug#49946] [PATCH v7 05/32] gnu: Add tree-sitter-cli. >>>>> id:20221125012142.22579-6-pierre.langlois@gmx.com >>>>> http://issues.guix.gnu.org/msgid/20221125012142.22579-6-pierre.langlois@gmx.com >>>>> >>>>> Leaving out all the others, right? >>>> >>>> Merged first 5 patches from 01 to 05, also added one more commit, which >>>> addresses some things from reviews and one commit, which adds html >>>> grammar. >>>> >>>> The html grammar is added for the testing purposes. It relies on >>>> generated parser.c and scanner.c and we will need to repackage it using >>>> grammar.js instead. I'm not sure if a separate build system is needed >>>> for this, I guess we can just rewrite tree-sitter-grammar function, >>>> which generates packages as in example with tree-sitter-grammar-html: >>>> https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/tree-sitter.scm?h=53b00b91b73bd60412d5bd057e22e6d63194a7f7#n158 >>>> >>>> Anyway, I only skimmed tree-sitter-build-system source code, and plan to >>>> read it carefully, evaluate and either introduce new build system or >>>> just move all needed parts to tree-sitter-grammar function. WDYT? >>>> After we done with it we can package all other grammars. >>> >>> Ok, I realized that the proper build process for tree-sitter grammars is >>> a little harder than I expected, tree-sitter-build system make sense. I >>> reviewed it, made a small change: >> >> Ah great, I was going to comment to try and push for us to keep the >> build system. I originally went with a template package and inheritance, >> but Maxime suggested moving to a build-system which ended up making the >> package definitions a *lot* nicer IMO (see previous discussion here >> https://issues.guix.gnu.org/49946#144). It also allows us to deal with >> grammars that depend on each other more nicely I think. >> >>> >>> @@ -29,7 +29,7 @@ (define-module (guix build tree-sitter-build-system) >>> ;; Commentary: >>> ;; >>> ;; Build procedures for tree-sitter grammar packages. This is the >>> -;; builder-side code, which builds on top fo the node build-system. >>> +;; builder-side code, which builds on top of the node build-system. >>> ;; >>> ;; Tree-sitter grammars are written in JavaScript and compiled to a native >>> ;; shared object. The `tree-sitter generate' command invokes `node' in order >>> @@ -114,7 +114,7 @@ (define (compile-language dir) >>> "-fno-exceptions" >>> "-O2" >>> "-g" >>> - "-o" ,(string-append lib "/" lang ".so") >>> + "-o" ,(string-append lib "/libtree-sitter-" lang ".so") >>> ;; An additional `scanner.{c,cc}' file is sometimes >>> ;; provided. >>> ,@(cond >>> >>> >>> rewrote html grammar to use this build system and made it work with >>> built-in treesit package. Also, tried examples of c and cpp grammars >>> from patches in this thread. >>> >>> If you ok with it, I'll push the build system to master and update the >>> html grammar accordingly. > > Oh, I forgot to say, this change to the build system LGTM! I'm really > happy to see it merged soon :-). The path change will probably break the > emacs-28-based tree-sitter support, but that's OK, it's better for the > build-system to be made to target emacs 29's builtin support. I'm sure I > can work around for emacs 28. Actually, I think we can build grammars with both names, just providing two .so files instead of one. If you won't find a better workaround we can go this way. -- Best regards, Andrew Tropin