Subject: Re: bug#68579: [PATCH v2] Support a local repo as URL in treesit-language-source-alist

From: Konstantin Kharlamov <[email-masked]>
Date: Sat, 27 Jan 2024 21:53:15 +0300

On Sat, 2024-01-27 at 11:37 +0200, Eli Zaretskii wrote:
> > From: Konstantin Kharlamov <[email-masked]>
> > Date: Sat, 20 Jan 2024 14:56:29 +0300
> > 
> > Sometimes people may need to bisect to find specific revision in a
> > grammar repo. In this case they'd want to point the URL to the
> > local
> > repo to avoid cloning it on every rebuild. So add support for full
> > path in treesit-language-source-alist.
> > 
> > * lisp/treesit.el (treesit--install-language-grammar-1): Test if
> > URL
> > starts with / meaning that the URL is a local path.  Then if it is,
> > avoid cloning the repo and removing the path on success.
> > (treesit--git-clone-repo): Factor out the code for cloning to a
> > separate
> > function.
> > (treesit--git-checkout-branch): A helper to checkout the revision
> > for
> > cases where we didn't clone the repo but want it to point the
> > revision.
> > ---
> 
> Thanks, but could you please send the patch as attachments created
> with "git format-patch", and include only the stuff you think should
> be in the patch and the commit log message?
> 
> Also, I have a few minor comments below.
> 
> > ++++
> > +** 'treesit-install-language-grammar' can handle local directory
> > as URL.
> > +It is now possible to pass a directory of a local repository as
> > URL
>                                                                
> ^^^^^^
> I think you mean "instead of a URL" there.

Right, sorry, I was thinking of "URL" being the name of the argument,
but your text sure is clearer.

> > +inside 'treesit-language-source-alist', so that calling
> > +'treesit-install-language-grammar' would avoid cloning the
> > repository.
> > +It may be useful for example for the purposes of bisecting a
>                    ^           ^
> Commas missing there.
> 
> > --- a/lisp/treesit.el
> > +++ b/lisp/treesit.el
> > @@ -3410,14 +3410,16 @@ treesit-explore-mode
> >  ;;; Install & build language grammar
> >  
> >  (defvar treesit-language-source-alist nil
> > -  "Configuration for downloading and installing tree-sitter
> > language grammars.
> > +  "Configuration for downloading and installing tree-sitter
> > language
> > +grammars. The grammar can also be built from a local directory if
> > +URL is an existing local path to the repo.
> 
> The first line of a doc string should be a single complete sentence
> (because various apropos commands display only the first lines of doc
> strings).

Ah, okay, in this case I'm a bit confused regarding what you wanted me
to do by asking to provide a small description of the workflow. I've
documented that URL arg may accept a local dir. I can put this new
sentence to a separate paragraph, but I'm not sure it reads too well,
given the same information is provided one paragraph below.

I've reverted this change for now.

> >  The value should be an alist where each element has the form
> >  
> >      (LANG . (URL REVISION SOURCE-DIR CC C++))
> >  
> >  Only LANG and URL are mandatory.  LANG is the language symbol.
> > -URL is the Git repository URL for the grammar.
> > +URL is the Git repository URL or directory name for the grammar.
> 
> Suggest to reword:
> 
>   URL is the URL of the grammar's Git repository or a directory where
>   the repository has been cloned.
> 
> Thanks.

Thanks! Patch is attached, everything is addressed except one question
above. I also fixed "one space" to two spaces" in one place in the
commit description.
From 87a8984366ab843f6a28a30a74cf519301978bd2 Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <[email-masked]>
Date: Fri, 19 Jan 2024 10:33:47 +0300
Subject: [PATCH] Support a local repo as URL in treesit-language-source-alist

Sometimes people may need to bisect to find specific revision in a
grammar repo.  In this case they'd want to point the URL to the local
repo to avoid cloning it on every rebuild.  So add support for full
path in treesit-language-source-alist.

* lisp/treesit.el (treesit--install-language-grammar-1): Test if URL
starts with / meaning that the URL is a local path.  Then if it is,
avoid cloning the repo and removing the path on success.
(treesit--git-clone-repo): Factor out the code for cloning to a separate
function.
(treesit--git-checkout-branch): A helper to checkout the revision for
cases where we didn't clone the repo but want it to point the
revision.
---
 etc/NEWS        |  8 ++++++++
 lisp/treesit.el | 46 ++++++++++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 735a05f6579..9bdc3af5e71 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1832,6 +1832,14 @@ The 'test' parameter is omitted if it is 'eql' (the default), as is
 'data' if empty.  'rehash-size', 'rehash-threshold' and 'size' are
 always omitted, and ignored if present when the object is read back in.
 
++++
+** 'treesit-install-language-grammar' can handle local directory instead of URL.
+It is now possible to pass a directory of a local repository as URL
+inside 'treesit-language-source-alist', so that calling
+'treesit-install-language-grammar' would avoid cloning the repository.
+It may be useful, for example, for the purposes of bisecting a
+treesitter grammar.
+
 
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/treesit.el b/lisp/treesit.el
index c8b473c7bb8..b1f9d551442 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -3417,7 +3417,8 @@ treesit-language-source-alist
     (LANG . (URL REVISION SOURCE-DIR CC C++))
 
 Only LANG and URL are mandatory.  LANG is the language symbol.
-URL is the Git repository URL for the grammar.
+URL is the URL of the grammar's Git repository or a directory
+where the repository has been cloned.
 
 REVISION is the Git tag or branch of the desired version,
 defaulting to the latest default branch.
@@ -3551,6 +3552,26 @@ treesit--call-process-signal
                                  (buffer-string)))
     (erase-buffer)))
 
+(defun treesit--git-checkout-branch (repo-dir revision)
+  "Checkout REVISION in a repo located in REPO-DIR."
+  (treesit--call-process-signal
+   "git" nil t nil "-C" repo-dir "checkout" revision))
+
+(defun treesit--git-clone-repo (url revision workdir)
+  "Clone repo pointed by URL at commit REVISION to WORKDIR.
+
+REVISION may be nil, in which case the cloned repo will be at its
+default branch."
+  (message "Cloning repository")
+  ;; git clone xxx --depth 1 --quiet [-b yyy] workdir
+  (if revision
+      (treesit--call-process-signal
+       "git" nil t nil "clone" url "--depth" "1" "--quiet"
+       "-b" revision workdir)
+    (treesit--call-process-signal
+     "git" nil t nil "clone" url "--depth" "1" "--quiet"
+     workdir)))
+
 (defun treesit--install-language-grammar-1
     (out-dir lang url &optional revision source-dir cc c++)
   "Install and compile a tree-sitter language grammar library.
@@ -3564,8 +3585,12 @@ treesit--install-language-grammar-1
 `treesit-language-source-alist'.  If anything goes wrong, this
 function signals an error."
   (let* ((lang (symbol-name lang))
+         (maybe-repo-dir (expand-file-name url))
+         (url-is-dir (file-accessible-directory-p maybe-repo-dir))
          (default-directory (make-temp-file "treesit-workdir" t))
-         (workdir (expand-file-name "repo"))
+         (workdir (if url-is-dir
+                      maybe-repo-dir
+                    (expand-file-name "repo")))
          (source-dir (expand-file-name (or source-dir "src") workdir))
          (cc (or cc (seq-find #'executable-find '("cc" "gcc" "c99"))
                  ;; If no C compiler found, just use cc and let
@@ -3580,15 +3605,10 @@ treesit--install-language-grammar-1
          (lib-name (concat "libtree-sitter-" lang soext)))
     (unwind-protect
         (with-temp-buffer
-          (message "Cloning repository")
-          ;; git clone xxx --depth 1 --quiet [-b yyy] workdir
-          (if revision
-              (treesit--call-process-signal
-               "git" nil t nil "clone" url "--depth" "1" "--quiet"
-               "-b" revision workdir)
-            (treesit--call-process-signal
-             "git" nil t nil "clone" url "--depth" "1" "--quiet"
-             workdir))
+          (if url-is-dir
+              (when revision
+                (treesit--git-checkout-branch workdir revision))
+            (treesit--git-clone-repo url revision workdir))
           ;; We need to go into the source directory because some
           ;; header files use relative path (#include "../xxx").
           ;; cd "${sourcedir}"
@@ -3635,7 +3655,9 @@ treesit--install-language-grammar-1
             ;; Ignore errors, in case the old version is still used.
             (ignore-errors (delete-file old-fname)))
           (message "Library installed to %s/%s" out-dir lib-name))
-      (when (file-exists-p workdir)
+      ;; Remove workdir if it's not a repo owned by user and we
+      ;; managed to create it in the first place.
+      (when (and (not url-is-dir) (file-exists-p workdir))
         (delete-directory workdir t)))))
 
 ;;; Etc
-- 
2.43.0