Package: guix-patches;
Reported by: Pierre Langlois <pierre.langlois <at> gmx.com>
Date: Sun, 8 Aug 2021 23:27:01 UTC
Severity: normal
Tags: patch
Message #545 received at 49946 <at> debbugs.gnu.org (full text, mbox):
From: Pierre Langlois <pierre.langlois <at> gmx.com> To: Maxime Devos <maximedevos <at> telenet.be> Cc: Pierre Langlois <pierre.langlois <at> gmx.com>, 49946 <at> debbugs.gnu.org Subject: Re: [bug#49946] [PATCH v5 27/27] gnu: Add emacs-tree-sitter-langs. Date: Sun, 15 May 2022 13:20:48 +0100
[Message part 1 (text/plain, inline)]
Hi Maxime, sorry for the late reply! Maxime Devos <maximedevos <at> telenet.be> writes: > [[PGP Signed Part:Undecided]] > Pierre Langlois schreef op di 29-03-2022 om 20:43 [+0100]: >> + ;; The BUNDLE-VERSION file prevents emacs-tree-sitter-langs >> + ;; from downloading libraries at load time. > > WDYT of patching emacs-tree-sitter-langs to not download, such that it > doesn't download and run non-Guix libraries behind the user's back? The way the current patchset works, by providing a compatible bundle, we already prevent emacs-tree-sitter-langs from downloading binaries by default. I agree we could go further though, and entirely remove the code that downloads binaries. However I'm not sure about it. Essentially, there is a tree-sitter-langs-build.el file [0] that can either download binaries or fetch sources and compile locally. So a user could decide to opt-out of using Guix binaries and instead use an alternative method. I 100% agree that by default we should make the package use Guix libraries, this way it also /just works/. However, if somebody wants to do things differently, I'm not sure we want to get in the way. In the end, this isn't so different from letting people use an alternative package manager if they like. We'd rather people used Guix of course :-). I don't have a really strong opinion about this though, so if you still prefer to delete the downloading code, I've attached an example patch that entirely replace it with a bare-bones implementation as an example. Let me know what you think! [0]: https://github.com/emacs-tree-sitter/tree-sitter-langs/blob/master/tree-sitter-langs-build.el > Also, why do we need a bundle at all, would simply installing emacs- > tree-sitter, and, e.g., tree-sitter-java, just work? Yeah, having a bundle is fundamentally how this package works AFAICT. I think the main reason is that in order to do highlighting effectively in emacs, it's not enough to install the tree-sitter runtime and a tree-sitter-<lang> grammar. You also want to "configure" how the highlighting is done by providing custom "queries" expressions. And this package provides queries for every language that it supports: https://github.com/emacs-tree-sitter/tree-sitter-langs/tree/master/queries All that being said, I believe that long-term the idea is that upstream language-specific packages would eventually gain support for tree-sitter and then this bundle "glue" package will no longer be necessary. Especially if one day emacs proper gains native support for tree-sitter (I think I saw some discussions about that on emacs-devel last year). But given this package is quite useful though, I'd be surprised if it goes away soon. Hope this makes sense! Thanks, Pierre > > Greetings, > Maxime. > > [[End of PGP Signed Part]]
[signature.asc (application/pgp-signature, inline)]
[0001-wip.patch (text/x-patch, inline)]
From 7ad62ccef2446011dfbdfb2dbe8cc58f46fb05d8 Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois <at> gmx.com>
Date: Sat, 2 Apr 2022 19:22:52 +0100
Subject: [PATCH] wip
---
gnu/packages/tree-sitter.scm | 49 ++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/gnu/packages/tree-sitter.scm b/gnu/packages/tree-sitter.scm
index 7e14ebd1e3..d6892db6e3 100644
--- a/gnu/packages/tree-sitter.scm
+++ b/gnu/packages/tree-sitter.scm
@@ -795,11 +795,11 @@ (define-public emacs-tree-sitter
@end enumerate")
(license license:expat)))
-(define (make-emacs-tree-sitter-langs-grammar-bundle version)
+(define emacs-tree-sitter-langs-grammar-bundle
(package
(name "emacs-tree-sitter-langs-grammar-bundle")
(source #f)
- (version version)
+ (version (package-version tree-sitter))
(build-system trivial-build-system)
(inputs
;; FIXME: Support for some languages is still left to package.
@@ -836,11 +836,7 @@ (define (make-emacs-tree-sitter-langs-grammar-bundle version)
(map (match-lambda
((name directory)
(string-append directory "/lib/tree-sitter")))
- '#$(package-inputs this-package))))
- ;; The BUNDLE-VERSION file prevents emacs-tree-sitter-langs
- ;; from downloading libraries at load time.
- (call-with-output-file (string-append #$output "/BUNDLE-VERSION")
- (lambda (port) (display #$version port)))))))
+ '#$(package-inputs this-package))))))))
(synopsis #f)
(description #f)
(home-page #f)
@@ -861,7 +857,7 @@ (define-public emacs-tree-sitter-langs
"1p2zbb6ac7wi6x6zpbczcmpkb2p45md2csd2bj43d8s56ckzw5mp"))))
(build-system emacs-build-system)
(inputs
- (list (make-emacs-tree-sitter-langs-grammar-bundle version)))
+ (list emacs-tree-sitter-langs-grammar-bundle))
(propagated-inputs
(list emacs-tree-sitter))
(arguments
@@ -870,15 +866,36 @@ (define-public emacs-tree-sitter-langs
#:test-command ''("script/test")
#:phases
#~(modify-phases %standard-phases
+ (add-after 'unpack 'disable-downloader
+ (lambda _
+ (call-with-output-file "tree-sitter-langs-build.el"
+ (lambda (port)
+ (let ((on-load-message
+ (string-append
+ "tree-sitter-langs: Grammar bundle already installed "
+ "via Guix. Installing external grammars via this "
+ "function isn't supported, if a language you need is "
+ "missing please report a bug at bug-guix <at> gnu.org.")))
+ (format
+ port
+ ";;;###autoload
+ (defun tree-sitter-langs-install-grammars
+ (&optional skip-if-installed version os
+ keep-bundle)
+ (interactive)
+ (message \"~a\"))
+ (defconst tree-sitter-langs--queries-dir
+ (file-name-as-directory
+ (concat (file-name-directory (locate-library \"tree-sitter-langs.el\"))
+ \"queries\")))
+ (defun tree-sitter-langs--bin-dir () \"~a\")
+ (provide 'tree-sitter-langs-build)"
+ on-load-message
+ #$emacs-tree-sitter-langs-grammar-bundle))))))
(add-after 'unpack 'remove-cask
(lambda _
(substitute* "script/test"
(("cask") ""))))
- (add-before 'check 'bundle-for-testing
- (lambda* (#:key inputs #:allow-other-keys)
- (delete-file-recursively "bin")
- (symlink #$(make-emacs-tree-sitter-langs-grammar-bundle version)
- "bin")))
(add-before 'check 'patch-tree-sitter-require-test
(lambda _
(use-modules (ice-9 regex))
@@ -906,12 +923,6 @@ (define-public emacs-tree-sitter-langs
(lambda _
(delete-file-recursively "queries/hcl")
(delete-file-recursively "queries/pgn")))
- (add-before 'install 'install-bundle
- (lambda _
- (let ((elpa (elpa-directory #$output)))
- (mkdir-p elpa)
- (symlink #$(make-emacs-tree-sitter-langs-grammar-bundle version)
- (string-append elpa "/bin")))))
(add-after 'install 'install-queries
(lambda* (#:key outputs #:allow-other-keys)
(let ((elpa (elpa-directory (assoc-ref outputs "out"))))
--
2.36.0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.