all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 68579@debbugs.gnu.org
Subject: bug#68579: [PATCH v2] Support a local repo as URL in treesit-language-source-alist
Date: Sat, 27 Jan 2024 21:53:15 +0300	[thread overview]
Message-ID: <fb594de83fee91732dcf8fb2b68d31c07a89e2d7.camel@yandex.ru> (raw)
In-Reply-To: <86sf2j86co.fsf@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 3629 bytes --]

On Sat, 2024-01-27 at 11:37 +0200, Eli Zaretskii wrote:
> > From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> > 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.

[-- Attachment #2: 1.patch --]
[-- Type: text/x-patch, Size: 5623 bytes --]

From 87a8984366ab843f6a28a30a74cf519301978bd2 Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
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.
+
 \f
 * 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


  reply	other threads:[~2024-01-27 18:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-19  8:08 bug#68579: [PATCH] Support a local repo as URL in treesit-language-source-alist Konstantin Kharlamov
2024-01-19  8:33 ` Eli Zaretskii
2024-01-19  8:57   ` Konstantin Kharlamov
2024-01-19 11:46     ` Eli Zaretskii
2024-01-19 14:33       ` Konstantin Kharlamov
2024-01-19 15:06         ` Eli Zaretskii
2024-01-19 16:06           ` Konstantin Kharlamov
2024-01-19 16:26             ` Eli Zaretskii
2024-01-20 12:00               ` bug#68579: [PATCH v2] " Konstantin Kharlamov
2024-01-20 12:04                 ` Konstantin Kharlamov
2024-01-20 11:56 ` Konstantin Kharlamov
2024-01-27  9:37   ` Eli Zaretskii
2024-01-27 18:53     ` Konstantin Kharlamov [this message]
2024-01-27 19:21       ` Eli Zaretskii
2024-02-01 10:06     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fb594de83fee91732dcf8fb2b68d31c07a89e2d7.camel@yandex.ru \
    --to=hi-angel@yandex.ru \
    --cc=68579@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.