Subject: Re: [bug#49946] [PATCH v7 01/32] gnu: tree-sitter: Move to its own module.

From: Andrew Tropin <[email-masked]>
Date: Sun, 12 Feb 2023 10:05:30 +0400

On 2023-02-10 17:02, Pierre Langlois wrote:

> Pierre Langlois <[email-masked]> writes:
>
>> [[PGP Signed Part:Undecided]]
>> Hi Andrew, thanks for pushing this along! It's great to see things
>> getting merged.
>>
>> Andrew Tropin <[email-masked]> 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 <[email-masked]> 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:[email-masked]
>>>>> http://issues.guix.gnu.org/msgid/[email-masked]
>>>>>
>>>>> [bug#49946] [PATCH v7 05/32] gnu: Add tree-sitter-cli.
>>>>> id:[email-masked]
>>>>> http://issues.guix.gnu.org/msgid/[email-masked]
>>>>>
>>>>> 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