bug#60418: [PATCH] Add :vc keyword to use-package

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi,

this is a complete (and clean) rewrite of my package [vc-use-package],
in order to properly integrate it with the Emacs core.  Basically, it
adds a new :vc keyword so one can leverage the 'package-vc-install*'
function(s) from within a use-package declaration.  For example,
specifying

    (use-package foo
      :vc (:url "bar")) 

would expand to (more or less, concentrating on the relevant part)

    (unless (package-installed-p 'foo)
      (package-vc-install '(foo :url "bar") nil))

This makes installing packages from remote sources a breeze.  There is
also support, via 'package-vc-install-from-checkout', for installing
local packages by additionally specifying a load path:

    (use-package foo
      :vc bar             ; optional name, use t or nil for foo
      :load-path "/path")
  ⇔
    (progn (eval-and-compile (add-to-list 'load-path <<path>>))
           (unless (package-installed-p 'bar)
             (package-vc-install-from-checkout <<path>> "bar"))
           …)

I don't know what the policy here is regarding sending multiple patches
in the same email, but since the second one is just adding
documentation, I didn't deem it "worth" a second mail.  Sorry (and do
let me know!) if this is disruptive to anyone.

Best,
  Tony

[vc-use-package]: https://github.com/slotThe/vc-use-package


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-vc-keyword-to-use-package.patch --]
[-- Type: text/x-diff, Size: 8911 bytes --]

From 2aa5eed4186dab086684e8d0623b7f10cab07100 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 11:05:04 +0100
Subject: [PATCH 1/2] Add :vc keyword to use-package

* lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
(use-package-handler/:load-path): Insert 'load-path' into 'state'.
(use-package-handler/:vc): Handler for the :vc keyword.
(use-package-normalize--vc-arg): Normalization for more complex
arguments to 'use-package-normalize/:vc', in order to make them
compatible with the specification of 'package-vc-selected-packages'.
(use-package-normalize/:vc): Normalizer for the :vc keyword.
(use-package): Document :vc.

* lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
Do not ensure a package when :vc is used in the declaration.

* test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
(use-package-test/:vc-2):
(use-package-test/:vc-3):
(use-package-test/:vc-4):
(use-package-test-normalize/:vc):
Add tests for :vc.
---
 lisp/use-package/use-package-core.el       | 73 +++++++++++++++++++++-
 lisp/use-package/use-package-ensure.el     |  3 +-
 test/lisp/use-package/use-package-tests.el | 46 ++++++++++++++
 3 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 1dee08e55b..71a03c419a 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -76,6 +76,7 @@ use-package-keywords
     :functions
     :preface
     :if :when :unless
+    :vc
     :no-require
     :catch
     :after
@@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
     #'use-package-normalize-paths))
 
 (defun use-package-handler/:load-path (name _keyword arg rest state)
-  (let ((body (use-package-process-keywords name rest state)))
+  (let ((body (use-package-process-keywords name rest
+                (plist-put state :load-path arg))))
     (use-package-concat
      (mapcar #'(lambda (path)
                  `(eval-and-compile (add-to-list 'load-path ,path)))
@@ -1577,6 +1579,72 @@ use-package-handler/:config
      (when use-package-compute-statistics
        `((use-package-statistics-gather :config ',name t))))))
 
+;;;; :vc
+
+(defun use-package-handler/:vc (name _keyword arg rest state)
+  "Generate code for the :vc keyword."
+  (pcase-let ((body (use-package-process-keywords name rest state))
+              (local-path (car (plist-get state :load-path)))
+              (`(,name ,opts ,rev) arg))
+    (use-package-concat
+     `((unless (package-installed-p ',name)
+         ,(if local-path
+              `(package-vc-install-from-checkout ,local-path ,(symbol-name name))
+            `(package-vc-install ',(cons name opts) ,rev))))
+     body)))
+
+(defun use-package-normalize--vc-arg (arg)
+  "Normalize possible arguments to the :vc keyword.
+ARG is a cons-cell of approximately the form that
+`package-vc-selected-packages' accepts, plus an additional `:rev'
+keyword.
+
+Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
+with `package-vc-selected-packages' and REV is a (possibly nil)
+revision."
+  (cl-flet* ((mk-string (s)
+	       (if (stringp s) s (symbol-name s)))
+             (mk-sym (s)
+               (if (stringp s) (intern s) s))
+	     (normalize (k v)
+               (when v
+	         (pcase k
+                   (:vc-backend (mk-sym v))
+                   (:rev (if (eq v :last-release) v (mk-string v)))
+                   (_ (mk-string v))))))
+    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
+                (`(,name . ,opts) arg))
+      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
+          (list name opts)
+        ;; Error handling
+        (cl-loop for (k _) on opts by #'cddr
+                 if (not (member k valid-kws))
+                 do (use-package-error
+                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
+                             k valid-kws)))
+        ;; Actual normalization
+        (list name
+              (cl-loop for (k v) on opts by #'cddr
+                       if (not (eq k :rev))
+                       nconc (list k (normalize k v)))
+              (normalize :rev (plist-get opts :rev)))))))
+
+(defun use-package-normalize/:vc (name _keyword args)
+  (let ((arg (car args)))
+    (pcase arg
+      ((or 'nil 't) (list name))                 ; guess name
+      ((pred symbolp) (list arg))                ; use this name
+      ((pred stringp) (list name arg))           ; version string + guess name
+      ((pred plistp)                             ; plist + guess name
+       (use-package-normalize--vc-arg (cons name arg)))
+      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
+                                (pred stringp)))
+       (use-package-normalize--vc-arg arg))
+      (_ (use-package-error "Unrecognised argument to :vc.\
+ The keyword wants an argument of nil, t, a name of a package,\
+ or a cons-cell as accepted by `package-vc-selected-packages', where \
+ the accepted plist is augmented by a `:rev' keyword.")))))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
 ;;; The main macro
@@ -1666,7 +1734,8 @@ use-package
                  (compare with `custom-set-variables').
 :custom-face     Call `custom-set-faces' with each face definition.
 :ensure          Loads the package using package.el if necessary.
-:pin             Pin the package to an archive."
+:pin             Pin the package to an archive.
+:vc              Integration with `package-vc.el'."
   (declare (indent defun))
   (unless (memq :disabled args)
     (macroexp-progn
diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
index dae0312dba..27d4f5ad4f 100644
--- a/lisp/use-package/use-package-ensure.el
+++ b/lisp/use-package/use-package-ensure.el
@@ -182,7 +182,8 @@ use-package-ensure-elpa
 
 ;;;###autoload
 (defun use-package-handler/:ensure (name _keyword ensure rest state)
-  (let* ((body (use-package-process-keywords name rest state)))
+  (let* ((body (use-package-process-keywords name rest state))
+         (ensure (unless (plist-member rest :vc) ensure)))
     ;; We want to avoid installing packages when the `use-package' macro is
     ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
     ;; but still install packages when byte-compiling, to avoid requiring
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index e4586b04f2..4110f3a567 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1951,6 +1951,52 @@ bind-key/845
     (should (eq (nth 1 binding) 'ignore))
     (should (eq (nth 2 binding) nil))))
 
+(ert-deftest use-package-test/:vc-1 ()
+  (match-expansion
+   (use-package foo :vc (:url "bar"))
+   `(unless (package-installed-p 'foo)
+      (package-vc-install '(foo :url "bar") nil))))
+
+(ert-deftest use-package-test/:vc-2 ()
+  (match-expansion
+   (use-package foo
+     :vc (baz . (:url "baz" :vc-backend "Git"
+                 :main-file qux.el :rev "rev-string")))
+   `(unless (package-installed-p 'baz)
+      (package-vc-install '(baz :url "baz" :vc-backend Git :main-file "qux.el")
+                          "rev-string"))))
+
+(ert-deftest use-package-test/:vc-3 ()
+  (match-expansion
+   (use-package foo :vc (bar . "baz"))
+   `(unless (package-installed-p 'bar)
+      (package-vc-install '(bar . "baz") nil))))
+
+(ert-deftest use-package-test/:vc-4 ()
+  (let ((load-path? '(pred (apply-partially
+                            #'string=
+                            (expand-file-name "bar" user-emacs-directory)))))
+    (match-expansion
+     (use-package foo :vc other-name :load-path "bar")
+     `(progn (eval-and-compile
+               (add-to-list 'load-path ,load-path?))
+             (unless (package-installed-p 'other-name)
+               (package-vc-install-from-checkout ,load-path? "other-name"))))))
+
+(ert-deftest use-package-test-normalize/:vc ()
+  (should (equal '(foo "version-string")
+                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
+  (should (equal '(bar "version-string")
+                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
+  (should (equal '(foo (:url "bar") "baz")
+                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc '(t))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc nil)))
+  (should (equal '(bar)
+                 (use-package-normalize/:vc 'foo :vc '(bar)))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; no-update-autoloads: t
-- 
2.39.0


[-- Attachment #3: 0002-Document-use-package-s-vc-keyword.patch --]
[-- Type: text/x-diff, Size: 3402 bytes --]

From 9e54b103366aaef8e303bf65787ad841c56483d1 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 12:23:56 +0100
Subject: [PATCH 2/2] ; Document use-package's :vc keyword

* doc/misc/use-package.texi (Installing packages):
(Install package):
Add documentation for :vc and link to the related chapter in the Emacs
manual.

* etc/NEWS: Mention :vc keyword
---
 doc/misc/use-package.texi | 40 +++++++++++++++++++++++++++++++++++++--
 etc/NEWS                  | 10 ++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
index c587d23d74..a4abbb77f9 100644
--- a/doc/misc/use-package.texi
+++ b/doc/misc/use-package.texi
@@ -1564,8 +1564,10 @@ Installing packages
 (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
 @code{use-package} macro provides the @code{:ensure} and @code{:pin}
 keywords that interface with that package manager to automatically
-install packages.  This is particularly useful if you use your init
-file on more than one system.
+install packages.  Further, the @code{:vc} keyword may be used to
+control how package sources are fetched (@pxref{Fetching Package
+Sources,,, emacs, GNU Emacs Manual}).  This is particularly useful if
+you use your init file on more than one system.
 
 @menu
 * Install package::
@@ -1617,6 +1619,40 @@ Install package
 You can override the above setting for a single package by adding
 @w{@code{:ensure nil}} to its declaration.
 
+@findex :vc
+The @code{:vc} keyword can be used to control how packages are
+fetched.  It accepts the same arguments as
+@code{package-vc-selected-packages}, except that a name need not
+explicitly given: it is inferred from the declaration.  Further, the
+accepted property list is augmented by a @code{:rev} keyword, which
+has the same shape as the @code{REV} argument to
+@code{package-vc-install}.
+
+For example,
+
+@lisp
+@group
+(use-package foo
+  :vc (:url "https://bar.com/foo"))
+@end group
+@end lisp
+
+would try—by invoking @code{package-vc-install}—to install the package
+@code{foo} from the specified remote.
+
+This can also be used for local packages, by combining it with the
+@code{:load-path} (@pxref{Load path}) keyword:
+
+@lisp
+@group
+(use-package foo
+  :vc t
+  :load-path "/path/to/foo/)
+@end group
+@end lisp
+
+The above dispatches to @code{package-vc-install-from-checkout}.
+
 @node Pinning packages
 @section Pinning packages using @code{:pin}
 @cindex installing package from specific archive
diff --git a/etc/NEWS b/etc/NEWS
index 83aa81eb4b..d8e8ed0dd2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -108,6 +108,16 @@ This command either fills a single paragraph in a defun, such as a
 doc-string, or a comment, or (re)indents the surrounding defun if
 point is not in a comment or a string.  It is by default bound to
 'M-q' in 'prog-mode' and all its descendants.
+
+** use-package
+
+*** New ':vc' keyword
+This keyword enables the user to control how packages are fetched by
+utilising 'package-vc.el'.  By default, it relays its arguments to
+'package-vc-install', but—when combined with the ':load-path'
+keyword—it can also call upon 'package-vc-install-from-checkout'
+instead.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
-- 
2.39.0


[-- Attachment #4: Type: text/plain, Size: 44 bytes --]


-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

I was told to ping this again (and to Cc Stefan), so I'm going to do
that.

Thanks!
  Tony

On Thu, Dec 29 2022 19:43, Tony Zorman wrote:
> Hi,
>
> this is a complete (and clean) rewrite of my package [vc-use-package],
> in order to properly integrate it with the Emacs core.  Basically, it
> adds a new :vc keyword so one can leverage the 'package-vc-install*'
> function(s) from within a use-package declaration.  For example,
> specifying
>
>     (use-package foo
>       :vc (:url "bar")) 
>
> would expand to (more or less, concentrating on the relevant part)
>
>     (unless (package-installed-p 'foo)
>       (package-vc-install '(foo :url "bar") nil))
>
> This makes installing packages from remote sources a breeze.  There is
> also support, via 'package-vc-install-from-checkout', for installing
> local packages by additionally specifying a load path:
>
>     (use-package foo
>       :vc bar             ; optional name, use t or nil for foo
>       :load-path "/path")
>   ⇔
>     (progn (eval-and-compile (add-to-list 'load-path <<path>>))
>            (unless (package-installed-p 'bar)
>              (package-vc-install-from-checkout <<path>> "bar"))
>            …)
>
> I don't know what the policy here is regarding sending multiple patches
> in the same email, but since the second one is just adding
> documentation, I didn't deem it "worth" a second mail.  Sorry (and do
> let me know!) if this is disruptive to anyone.
>
> Best,
>   Tony
>
> [vc-use-package]: https://github.com/slotThe/vc-use-package
>
> From 2aa5eed4186dab086684e8d0623b7f10cab07100 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 11:05:04 +0100
> Subject: [PATCH 1/2] Add :vc keyword to use-package
>
> * lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
> (use-package-handler/:load-path): Insert 'load-path' into 'state'.
> (use-package-handler/:vc): Handler for the :vc keyword.
> (use-package-normalize--vc-arg): Normalization for more complex
> arguments to 'use-package-normalize/:vc', in order to make them
> compatible with the specification of 'package-vc-selected-packages'.
> (use-package-normalize/:vc): Normalizer for the :vc keyword.
> (use-package): Document :vc.
>
> * lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
> Do not ensure a package when :vc is used in the declaration.
>
> * test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
> (use-package-test/:vc-2):
> (use-package-test/:vc-3):
> (use-package-test/:vc-4):
> (use-package-test-normalize/:vc):
> Add tests for :vc.
> ---
>  lisp/use-package/use-package-core.el       | 73 +++++++++++++++++++++-
>  lisp/use-package/use-package-ensure.el     |  3 +-
>  test/lisp/use-package/use-package-tests.el | 46 ++++++++++++++
>  3 files changed, 119 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 1dee08e55b..71a03c419a 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -76,6 +76,7 @@ use-package-keywords
>      :functions
>      :preface
>      :if :when :unless
> +    :vc
>      :no-require
>      :catch
>      :after
> @@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
>      #'use-package-normalize-paths))
>  
>  (defun use-package-handler/:load-path (name _keyword arg rest state)
> -  (let ((body (use-package-process-keywords name rest state)))
> +  (let ((body (use-package-process-keywords name rest
> +                (plist-put state :load-path arg))))
>      (use-package-concat
>       (mapcar #'(lambda (path)
>                   `(eval-and-compile (add-to-list 'load-path ,path)))
> @@ -1577,6 +1579,72 @@ use-package-handler/:config
>       (when use-package-compute-statistics
>         `((use-package-statistics-gather :config ',name t))))))
>  
> +;;;; :vc
> +
> +(defun use-package-handler/:vc (name _keyword arg rest state)
> +  "Generate code for the :vc keyword."
> +  (pcase-let ((body (use-package-process-keywords name rest state))
> +              (local-path (car (plist-get state :load-path)))
> +              (`(,name ,opts ,rev) arg))
> +    (use-package-concat
> +     `((unless (package-installed-p ',name)
> +         ,(if local-path
> +              `(package-vc-install-from-checkout ,local-path ,(symbol-name name))
> +            `(package-vc-install ',(cons name opts) ,rev))))
> +     body)))
> +
> +(defun use-package-normalize--vc-arg (arg)
> +  "Normalize possible arguments to the :vc keyword.
> +ARG is a cons-cell of approximately the form that
> +`package-vc-selected-packages' accepts, plus an additional `:rev'
> +keyword.
> +
> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
> +with `package-vc-selected-packages' and REV is a (possibly nil)
> +revision."
> +  (cl-flet* ((mk-string (s)
> +	       (if (stringp s) s (symbol-name s)))
> +             (mk-sym (s)
> +               (if (stringp s) (intern s) s))
> +	     (normalize (k v)
> +               (when v
> +	         (pcase k
> +                   (:vc-backend (mk-sym v))
> +                   (:rev (if (eq v :last-release) v (mk-string v)))
> +                   (_ (mk-string v))))))
> +    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
> +                (`(,name . ,opts) arg))
> +      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
> +          (list name opts)
> +        ;; Error handling
> +        (cl-loop for (k _) on opts by #'cddr
> +                 if (not (member k valid-kws))
> +                 do (use-package-error
> +                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
> +                             k valid-kws)))
> +        ;; Actual normalization
> +        (list name
> +              (cl-loop for (k v) on opts by #'cddr
> +                       if (not (eq k :rev))
> +                       nconc (list k (normalize k v)))
> +              (normalize :rev (plist-get opts :rev)))))))
> +
> +(defun use-package-normalize/:vc (name _keyword args)
> +  (let ((arg (car args)))
> +    (pcase arg
> +      ((or 'nil 't) (list name))                 ; guess name
> +      ((pred symbolp) (list arg))                ; use this name
> +      ((pred stringp) (list name arg))           ; version string + guess name
> +      ((pred plistp)                             ; plist + guess name
> +       (use-package-normalize--vc-arg (cons name arg)))
> +      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
> +                                (pred stringp)))
> +       (use-package-normalize--vc-arg arg))
> +      (_ (use-package-error "Unrecognised argument to :vc.\
> + The keyword wants an argument of nil, t, a name of a package,\
> + or a cons-cell as accepted by `package-vc-selected-packages', where \
> + the accepted plist is augmented by a `:rev' keyword.")))))
> +
>  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  ;;
>  ;;; The main macro
> @@ -1666,7 +1734,8 @@ use-package
>                   (compare with `custom-set-variables').
>  :custom-face     Call `custom-set-faces' with each face definition.
>  :ensure          Loads the package using package.el if necessary.
> -:pin             Pin the package to an archive."
> +:pin             Pin the package to an archive.
> +:vc              Integration with `package-vc.el'."
>    (declare (indent defun))
>    (unless (memq :disabled args)
>      (macroexp-progn
> diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
> index dae0312dba..27d4f5ad4f 100644
> --- a/lisp/use-package/use-package-ensure.el
> +++ b/lisp/use-package/use-package-ensure.el
> @@ -182,7 +182,8 @@ use-package-ensure-elpa
>  
>  ;;;###autoload
>  (defun use-package-handler/:ensure (name _keyword ensure rest state)
> -  (let* ((body (use-package-process-keywords name rest state)))
> +  (let* ((body (use-package-process-keywords name rest state))
> +         (ensure (unless (plist-member rest :vc) ensure)))
>      ;; We want to avoid installing packages when the `use-package' macro is
>      ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
>      ;; but still install packages when byte-compiling, to avoid requiring
> diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
> index e4586b04f2..4110f3a567 100644
> --- a/test/lisp/use-package/use-package-tests.el
> +++ b/test/lisp/use-package/use-package-tests.el
> @@ -1951,6 +1951,52 @@ bind-key/845
>      (should (eq (nth 1 binding) 'ignore))
>      (should (eq (nth 2 binding) nil))))
>  
> +(ert-deftest use-package-test/:vc-1 ()
> +  (match-expansion
> +   (use-package foo :vc (:url "bar"))
> +   `(unless (package-installed-p 'foo)
> +      (package-vc-install '(foo :url "bar") nil))))
> +
> +(ert-deftest use-package-test/:vc-2 ()
> +  (match-expansion
> +   (use-package foo
> +     :vc (baz . (:url "baz" :vc-backend "Git"
> +                 :main-file qux.el :rev "rev-string")))
> +   `(unless (package-installed-p 'baz)
> +      (package-vc-install '(baz :url "baz" :vc-backend Git :main-file "qux.el")
> +                          "rev-string"))))
> +
> +(ert-deftest use-package-test/:vc-3 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . "baz"))
> +   `(unless (package-installed-p 'bar)
> +      (package-vc-install '(bar . "baz") nil))))
> +
> +(ert-deftest use-package-test/:vc-4 ()
> +  (let ((load-path? '(pred (apply-partially
> +                            #'string=
> +                            (expand-file-name "bar" user-emacs-directory)))))
> +    (match-expansion
> +     (use-package foo :vc other-name :load-path "bar")
> +     `(progn (eval-and-compile
> +               (add-to-list 'load-path ,load-path?))
> +             (unless (package-installed-p 'other-name)
> +               (package-vc-install-from-checkout ,load-path? "other-name"))))))
> +
> +(ert-deftest use-package-test-normalize/:vc ()
> +  (should (equal '(foo "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
> +  (should (equal '(bar "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
> +  (should (equal '(foo (:url "bar") "baz")
> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc '(t))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc nil)))
> +  (should (equal '(bar)
> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
> +
>  ;; Local Variables:
>  ;; no-byte-compile: t
>  ;; no-update-autoloads: t
> -- 
> 2.39.0
>
> From 9e54b103366aaef8e303bf65787ad841c56483d1 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 12:23:56 +0100
> Subject: [PATCH 2/2] ; Document use-package's :vc keyword
>
> * doc/misc/use-package.texi (Installing packages):
> (Install package):
> Add documentation for :vc and link to the related chapter in the Emacs
> manual.
>
> * etc/NEWS: Mention :vc keyword
> ---
>  doc/misc/use-package.texi | 40 +++++++++++++++++++++++++++++++++++++--
>  etc/NEWS                  | 10 ++++++++++
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
> index c587d23d74..a4abbb77f9 100644
> --- a/doc/misc/use-package.texi
> +++ b/doc/misc/use-package.texi
> @@ -1564,8 +1564,10 @@ Installing packages
>  (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
>  @code{use-package} macro provides the @code{:ensure} and @code{:pin}
>  keywords that interface with that package manager to automatically
> -install packages.  This is particularly useful if you use your init
> -file on more than one system.
> +install packages.  Further, the @code{:vc} keyword may be used to
> +control how package sources are fetched (@pxref{Fetching Package
> +Sources,,, emacs, GNU Emacs Manual}).  This is particularly useful if
> +you use your init file on more than one system.
>  
>  @menu
>  * Install package::
> @@ -1617,6 +1619,40 @@ Install package
>  You can override the above setting for a single package by adding
>  @w{@code{:ensure nil}} to its declaration.
>  
> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are
> +fetched.  It accepts the same arguments as
> +@code{package-vc-selected-packages}, except that a name need not
> +explicitly given: it is inferred from the declaration.  Further, the
> +accepted property list is augmented by a @code{:rev} keyword, which
> +has the same shape as the @code{REV} argument to
> +@code{package-vc-install}.
> +
> +For example,
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc (:url "https://bar.com/foo"))
> +@end group
> +@end lisp
> +
> +would try—by invoking @code{package-vc-install}—to install the package
> +@code{foo} from the specified remote.
> +
> +This can also be used for local packages, by combining it with the
> +@code{:load-path} (@pxref{Load path}) keyword:
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc t
> +  :load-path "/path/to/foo/)
> +@end group
> +@end lisp
> +
> +The above dispatches to @code{package-vc-install-from-checkout}.
> +
>  @node Pinning packages
>  @section Pinning packages using @code{:pin}
>  @cindex installing package from specific archive
> diff --git a/etc/NEWS b/etc/NEWS
> index 83aa81eb4b..d8e8ed0dd2 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -108,6 +108,16 @@ This command either fills a single paragraph in a defun, such as a
>  doc-string, or a comment, or (re)indents the surrounding defun if
>  point is not in a comment or a string.  It is by default bound to
>  'M-q' in 'prog-mode' and all its descendants.
> +
> +** use-package
> +
> +*** New ':vc' keyword
> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to
> +'package-vc-install', but—when combined with the ':load-path'
> +keyword—it can also call upon 'package-vc-install-from-checkout'
> +instead.
> +
>  \f
>  * New Modes and Packages in Emacs 30.1
>  
> -- 
> 2.39.0

-- 
Tony Zorman | https://tony-zorman.com/
 

bug#60418: [PATCH] Add :vc keyword to use-package

[Felician Nemeth]

merge 60418 61937
thanks

(I hope the above works, I've never merged bugs before.)

Philip Kaludercic <philipk@posteo.net> writes:

>   I have to admit that I am not a fan of this [adding package-vc support
>   to use-package].  Personally I install all my packages using regular
>   tarballs, and then manually select packages that I want to track in
>   source-form if I am interested in contributing a patch.
>
>   One has to keep in mind that by default you will be checking out the tip
>   of the default development branch.  This is not what GNU ELPA and NonGNU
>   ELPA do.  I know *some other* archives do this, but IMO this contributes
>   to the instability that a lot of Emacs users experience in regards to
>   the packages in the Emacs world.  Coincidentally I also believe that the
>   popularity of alternative (non-cooperative) package managers like this
>   "straight.el" boils down to the promise of being able to pin and revert
>   to previous packages states.  Personally I believe that this is an
>   up-side-down approach to solve the problem from the wrong angle.  The
>   default approach to package management should be reliable and suffice.

I think Pandora's box is already open with having package-vc.

Maybe use-package's package-vc integration should use more sensible
defaults then?  But if people don't like this feature, I personally
won't press the issue further.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Felician Nemeth <felician.nemeth@gmail.com> writes:

> merge 60418 61937
> thanks
>
> (I hope the above works, I've never merged bugs before.)
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>>   I have to admit that I am not a fan of this [adding package-vc support
>>   to use-package].  Personally I install all my packages using regular
>>   tarballs, and then manually select packages that I want to track in
>>   source-form if I am interested in contributing a patch.
>>
>>   One has to keep in mind that by default you will be checking out the tip
>>   of the default development branch.  This is not what GNU ELPA and NonGNU
>>   ELPA do.  I know *some other* archives do this, but IMO this contributes
>>   to the instability that a lot of Emacs users experience in regards to
>>   the packages in the Emacs world.  Coincidentally I also believe that the
>>   popularity of alternative (non-cooperative) package managers like this
>>   "straight.el" boils down to the promise of being able to pin and revert
>>   to previous packages states.  Personally I believe that this is an
>>   up-side-down approach to solve the problem from the wrong angle.  The
>>   default approach to package management should be reliable and suffice.
>
> I think Pandora's box is already open with having package-vc.
>
> Maybe use-package's package-vc integration should use more sensible
> defaults then?  But if people don't like this feature, I personally
> won't press the issue further.

I am afraid I don't know what you mean by more sensible?  My central
point is that I wouldn't want to encourage people to download packages
using package-vc by default.

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

> Felician Nemeth <felician.nemeth@gmail.com> writes:
>
>> merge 60418 61937
>> thanks
>>
>> (I hope the above works, I've never merged bugs before.)
>>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>   I have to admit that I am not a fan of this [adding package-vc support
>>>   to use-package].  Personally I install all my packages using regular
>>>   tarballs, and then manually select packages that I want to track in
>>>   source-form if I am interested in contributing a patch.
>>>
>>>   One has to keep in mind that by default you will be checking out the tip
>>>   of the default development branch.  This is not what GNU ELPA and NonGNU
>>>   ELPA do.  I know *some other* archives do this, but IMO this contributes
>>>   to the instability that a lot of Emacs users experience in regards to
>>>   the packages in the Emacs world.  Coincidentally I also believe that the
>>>   popularity of alternative (non-cooperative) package managers like this
>>>   "straight.el" boils down to the promise of being able to pin and revert
>>>   to previous packages states.  Personally I believe that this is an
>>>   up-side-down approach to solve the problem from the wrong angle.  The
>>>   default approach to package management should be reliable and suffice.
>>
>> I think Pandora's box is already open with having package-vc.
>>
>> Maybe use-package's package-vc integration should use more sensible
>> defaults then?  But if people don't like this feature, I personally
>> won't press the issue further.
>
> I am afraid I don't know what you mean by more sensible?  My central
> point is that I wouldn't want to encourage people to download packages
> using package-vc by default.

ping?

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Felician Nemeth]

>>>>   One has to keep in mind that by default you will be checking out the tip
>>>>   of the default development branch.  [...]

>>> Maybe use-package's package-vc integration should use more sensible
>>> defaults then?  But if people don't like this feature, I personally
>>> won't press the issue further.
>>
>> I am afraid I don't know what you mean by more sensible?  My central
>> point is that I wouldn't want to encourage people to download packages
>> using package-vc by default.

The integration of package-vc with use-package makes a bit easier what's
already possible.

One possible way to mitigate the problem of running instable code is to
check out the commit with the latest tag.  (I don't know how feasible is
to implement this.)  Alternatively, use-package cloud refuse a :vc
keyword if an additional :i-know-this-is-in-development-code keyword was
not set.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Felician Nemeth <felician.nemeth@gmail.com> writes:

>>>>>   One has to keep in mind that by default you will be checking out the tip
>>>>>   of the default development branch.  [...]
>
>>>> Maybe use-package's package-vc integration should use more sensible
>>>> defaults then?  But if people don't like this feature, I personally
>>>> won't press the issue further.
>>>
>>> I am afraid I don't know what you mean by more sensible?  My central
>>> point is that I wouldn't want to encourage people to download packages
>>> using package-vc by default.
>
> The integration of package-vc with use-package makes a bit easier what's
> already possible.
>
> One possible way to mitigate the problem of running instable code is to
> check out the commit with the latest tag.  (I don't know how feasible is
> to implement this.)  

package-vc can check out the latest revision that bumped the version tag
(which is what {Non,}GNU ELPA uses to create new versions of a package),
you just need to call `package-vc-install' with `:last-release' as REV.
So the technical means are there.

But if this is done, the question what advantage this has over just
installing a tarball this has?  You would almost always get the same
code.

>                      Alternatively, use-package cloud refuse a :vc
> keyword if an additional :i-know-this-is-in-development-code keyword was
> not set.

Just to clarify, I am not vetoing anything here.  If the functionality
is added to use-package (which is up to the use-package maintainers to
decide), then I don't think users need to be infantilized with these
kinds of little ceremonies.  All I want to understand is the motivation
behind this feature in the first place.

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Felician Nemeth]

Philip Kaludercic <philipk@posteo.net> writes:

> package-vc can check out the latest revision that bumped the version tag
> (which is what {Non,}GNU ELPA uses to create new versions of a package),
> you just need to call `package-vc-install' with `:last-release' as REV.
> So the technical means are there.
>
> But if this is done, the question what advantage this has over just
> installing a tarball this has?  You would almost always get the same
> code.

Convenience, I suppose.  You can copy your init file to a new machine,
and it will work.

> [...] All I want to understand is the motivation behind this feature
> in the first place.

One use-case is to let package developers to distribute thier packages
without a package archive.  Or more precisely, to let users conveniently
install packages that are not in a package archive.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Felician Nemeth <felician.nemeth@gmail.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> package-vc can check out the latest revision that bumped the version tag
>> (which is what {Non,}GNU ELPA uses to create new versions of a package),
>> you just need to call `package-vc-install' with `:last-release' as REV.
>> So the technical means are there.
>>
>> But if this is done, the question what advantage this has over just
>> installing a tarball this has?  You would almost always get the same
>> code.
>
> Convenience, I suppose.  You can copy your init file to a new machine,
> and it will work.

But isn't that is already given with use-package and package using
tarballs?

>> [...] All I want to understand is the motivation behind this feature
>> in the first place.
>
> One use-case is to let package developers to distribute thier packages
> without a package archive.  Or more precisely, to let users conveniently
> install packages that are not in a package archive.

Hmm, I had not considered this use-case.  I think that if we combine
this with falling back to :last-release by default, this could be a good
way to solve this problem.

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Felician Nemeth]

Philip Kaludercic <philipk@posteo.net> writes:

> Felician Nemeth <felician.nemeth@gmail.com> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> package-vc can check out the latest revision that bumped the version tag
>>> (which is what {Non,}GNU ELPA uses to create new versions of a package),
>>> you just need to call `package-vc-install' with `:last-release' as REV.
>>> So the technical means are there.
>>>
>>> But if this is done, the question what advantage this has over just
>>> installing a tarball this has?  You would almost always get the same
>>> code.
>>
>> Convenience, I suppose.  You can copy your init file to a new machine,
>> and it will work.
>
> But isn't that is already given with use-package and package using
> tarballs?

I don't know, but a package might have just a public git repository
without tarballs.

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Sat, Apr 08 2023 11:25, Felician Nemeth wrote:
>> [...] All I want to understand is the motivation behind this feature
>> in the first place.
>
> One use-case is to let package developers to distribute thier packages
> without a package archive.  Or more precisely, to let users conveniently
> install packages that are not in a package archive.

FWIW, this was also my motivation for making this change. It's just
incredibly convenient to be able to do this without installing
third-party dependencies.

Further, it also makes testing out new changes very easy. For example, I
wanted to try out the overhauled LaTeX preview system that was proposed
for Org mode [1]. Rather than having to clone the respective repo and
check out the branch manually, this was just a

    (use-package org
      :vc (:url "https://git.tecosaur.net/tec/org-mode.git" :branch "dev"))

away, which is certainly a win in my book. 

[1]: https://lists.gnu.org/archive/html/emacs-orgmode/2023-03/msg00196.html

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> On Sat, Apr 08 2023 11:25, Felician Nemeth wrote:
>>> [...] All I want to understand is the motivation behind this feature
>>> in the first place.
>>
>> One use-case is to let package developers to distribute thier packages
>> without a package archive.  Or more precisely, to let users conveniently
>> install packages that are not in a package archive.
>
> FWIW, this was also my motivation for making this change. It's just
> incredibly convenient to be able to do this without installing
> third-party dependencies.

I think that this is a legitimate point, and I would be ready to push
this change if you could adjust the patch to checkout the latest release
instead of the latest commit by default, as Felician proposed.  Does
that sound good to you?

> Further, it also makes testing out new changes very easy. For example, I
> wanted to try out the overhauled LaTeX preview system that was proposed
> for Org mode [1]. Rather than having to clone the respective repo and
> check out the branch manually, this was just a
>
>     (use-package org
>       :vc (:url "https://git.tecosaur.net/tec/org-mode.git" :branch "dev"))
>
> away, which is certainly a win in my book. 

To be fair, use-package is not necessary here.  You can also just use
package-vc-checkout to download a package to some directory and check it
out that way.  I have a command somewhere that even automates the
selection and would just "install" the package for a single session.

> [1]: https://lists.gnu.org/archive/html/emacs-orgmode/2023-03/msg00196.html

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wed, Apr 12 2023 07:34, Philip Kaludercic wrote:
> I think that this is a legitimate point, and I would be ready to push
> this change if you could adjust the patch to checkout the latest release
> instead of the latest commit by default, as Felician proposed.  Does
> that sound good to you?

Yes, sounds good to me. I should have time to prepare a revised patch
this weekend.

>> Further, it also makes testing out new changes very easy. For example, I
>> wanted to try out the overhauled LaTeX preview system that was proposed
>> for Org mode [1]. Rather than having to clone the respective repo and
>> check out the branch manually, this was just a
>>
>>     (use-package org
>>       :vc (:url "https://git.tecosaur.net/tec/org-mode.git" :branch "dev"))
>>
>> away, which is certainly a win in my book. 
>
> To be fair, use-package is not necessary here.  You can also just use
> package-vc-checkout to download a package to some directory and check it
> out that way.  I have a command somewhere that even automates the
> selection and would just "install" the package for a single session.

Fair point. I should have mentioned that I already had a `use-package`
declaration for org-mode, so trying out the branch simply required
adding a single line, which is why that felt so nice.

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Alright, attached are patches that contain the requested change: we now
default to :last-release.

  Tony


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-vc-keyword-to-use-package.patch --]
[-- Type: text/x-patch, Size: 9738 bytes --]

From dd13136ddf9a8db0ae526769ad4885085cc0b19b Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 11:05:04 +0100
Subject: [PATCH 1/2] Add :vc keyword to use-package

* lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
(use-package-handler/:load-path): Insert 'load-path' into 'state'.
(use-package-vc-install): Install the package with package-vc.el.
(use-package-handler/:vc): Handler for the :vc keyword.
(use-package-normalize--vc-arg): Normalization for more complex
arguments to 'use-package-normalize/:vc', in order to make them
compatible with the specification of 'package-vc-selected-packages'.
(use-package-normalize/:vc): Normalizer for the :vc keyword.
(use-package): Document :vc.

* lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
Do not ensure a package when :vc is used in the declaration.

* test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
(use-package-test/:vc-2):
(use-package-test/:vc-3):
(use-package-test/:vc-4):
(use-package-test/:vc-5):
(use-package-test-normalize/:vc):
Add tests for :vc.
---
 lisp/use-package/use-package-core.el       | 83 +++++++++++++++++++++-
 lisp/use-package/use-package-ensure.el     |  3 +-
 test/lisp/use-package/use-package-tests.el | 49 +++++++++++++
 3 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 7ab5bdc276..2f84c225d6 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -76,6 +76,7 @@ use-package-keywords
     :functions
     :preface
     :if :when :unless
+    :vc
     :no-require
     :catch
     :after
@@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
     #'use-package-normalize-paths))
 
 (defun use-package-handler/:load-path (name _keyword arg rest state)
-  (let ((body (use-package-process-keywords name rest state)))
+  (let ((body (use-package-process-keywords name rest
+                (plist-put state :load-path arg))))
     (use-package-concat
      (mapcar #'(lambda (path)
                  `(eval-and-compile (add-to-list 'load-path ,path)))
@@ -1577,6 +1579,82 @@ use-package-handler/:config
      (when use-package-compute-statistics
        `((use-package-statistics-gather :config ',name t))))))
 
+;;;; :vc
+
+(defun use-package-vc-install (arg &optional local-path)
+  "ARG is a list of the form (NAME OPTIONS REVISION).
+The optional LOCAL-PATH boolean decides whether
+`package-vc-install-from-checkout' or `package-vc-install' will
+end up being called."
+  (pcase-let* ((`(,name ,opts ,rev) arg)
+               (spec (if opts (cons name opts) name)))
+    (unless (package-installed-p name)
+      (if local-path
+          (package-vc-install-from-checkout local-path (symbol-name name))
+        (package-vc-install spec rev)))))
+
+(defun use-package-handler/:vc (name _keyword arg rest state)
+  "Generate code for the :vc keyword."
+  (let ((body (use-package-process-keywords name rest state))
+        (local-path (car (plist-get state :load-path))))
+    ;; See `use-package-handler/:ensure' for an explanation.
+    (if (bound-and-true-p byte-compile-current-file)
+        (funcall #'use-package-vc-install arg local-path)         ; compile time
+      (push `(use-package-vc-install ',arg ,local-path) body)))) ; run time
+
+(defun use-package-normalize--vc-arg (arg)
+  "Normalize possible arguments to the :vc keyword.
+ARG is a cons-cell of approximately the form that
+`package-vc-selected-packages' accepts, plus an additional `:rev'
+keyword.  If `:rev' is not given, it defaults to `:last-release'.
+
+Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
+with `package-vc-selected-packages' and REV is a (possibly nil)
+revision."
+  (cl-flet* ((mk-string (s)
+	       (if (and s (stringp s)) s (symbol-name s)))
+             (mk-sym (s)
+               (if (and s (stringp s)) (intern s) s))
+	     (normalize (k v)
+               (pcase k
+                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
+                             ((eq v :head) nil)
+                             (t (mk-string v))))
+                 (:vc-backend (mk-sym v))
+                 (_ (mk-string v)))))
+    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
+                (`(,name . ,opts) arg))
+      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
+          (list name opts)
+        ;; Error handling
+        (cl-loop for (k _) on opts by #'cddr
+                 if (not (member k valid-kws))
+                 do (use-package-error
+                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
+                             k valid-kws)))
+        ;; Actual normalization
+        (list name
+              (cl-loop for (k v) on opts by #'cddr
+                       if (not (eq k :rev))
+                       nconc (list k (normalize k v)))
+              (normalize :rev (plist-get opts :rev)))))))
+
+(defun use-package-normalize/:vc (name _keyword args)
+  (let ((arg (car args)))
+    (pcase arg
+      ((or 'nil 't) (list name))                 ; guess name
+      ((pred symbolp) (list arg))                ; use this name
+      ((pred stringp) (list name arg))           ; version string + guess name
+      ((pred plistp)                             ; plist + guess name
+       (use-package-normalize--vc-arg (cons name arg)))
+      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
+                                (pred stringp)))
+       (use-package-normalize--vc-arg arg))
+      (_ (use-package-error "Unrecognised argument to :vc.\
+ The keyword wants an argument of nil, t, a name of a package,\
+ or a cons-cell as accepted by `package-vc-selected-packages', where \
+ the accepted plist is augmented by a `:rev' keyword.")))))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
 ;;; The main macro
@@ -1666,7 +1744,8 @@ use-package
                  (compare with `custom-set-variables').
 :custom-face     Call `custom-set-faces' with each face definition.
 :ensure          Loads the package using package.el if necessary.
-:pin             Pin the package to an archive."
+:pin             Pin the package to an archive.
+:vc              Integration with `package-vc.el'."
   (declare (indent defun))
   (unless (memq :disabled args)
     (macroexp-progn
diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
index e0ea982594..425a3e8917 100644
--- a/lisp/use-package/use-package-ensure.el
+++ b/lisp/use-package/use-package-ensure.el
@@ -182,7 +182,8 @@ use-package-ensure-elpa
 
 ;;;###autoload
 (defun use-package-handler/:ensure (name _keyword ensure rest state)
-  (let* ((body (use-package-process-keywords name rest state)))
+  (let* ((body (use-package-process-keywords name rest state))
+         (ensure (unless (plist-member rest :vc) ensure)))
     ;; We want to avoid installing packages when the `use-package' macro is
     ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
     ;; but still install packages when byte-compiling, to avoid requiring
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index 6374a0d103..b04d087e65 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1951,6 +1951,55 @@ bind-key/845
     (should (eq (nth 1 binding) 'ignore))
     (should (eq (nth 2 binding) nil))))
 
+(ert-deftest use-package-test/:vc-1 ()
+  (match-expansion
+   (use-package foo :vc (:url "bar"))
+   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
+
+(ert-deftest use-package-test/:vc-2 ()
+  (match-expansion
+   (use-package foo
+     :vc (baz . (:url "baz" :vc-backend "Git"
+                 :main-file qux.el :rev "rev-string")))
+   `(use-package-vc-install '(baz
+                              (:url "baz" :vc-backend Git :main-file "qux.el")
+                              "rev-string")
+                            nil)))
+
+(ert-deftest use-package-test/:vc-3 ()
+  (match-expansion
+   (use-package foo :vc (bar . "baz"))
+   `(use-package-vc-install '(bar "baz") nil)))
+
+(ert-deftest use-package-test/:vc-4 ()
+  (match-expansion
+   (use-package foo :vc (bar . (:url "baz" :rev :head)))
+   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
+
+(ert-deftest use-package-test/:vc-5 ()
+  (let ((load-path? '(pred (apply-partially
+                            #'string=
+                            (expand-file-name "bar" user-emacs-directory)))))
+    (match-expansion
+     (use-package foo :vc other-name :load-path "bar")
+     `(progn (eval-and-compile
+               (add-to-list 'load-path ,load-path?))
+             (use-package-vc-install '(other-name) ,load-path?)))))
+
+(ert-deftest use-package-test-normalize/:vc ()
+  (should (equal '(foo "version-string")
+                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
+  (should (equal '(bar "version-string")
+                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
+  (should (equal '(foo (:url "bar") "baz")
+                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc '(t))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc nil)))
+  (should (equal '(bar)
+                 (use-package-normalize/:vc 'foo :vc '(bar)))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; no-update-autoloads: t
-- 
2.40.0


[-- Attachment #3: 0002-Document-use-package-s-vc-keyword.patch --]
[-- Type: text/x-patch, Size: 3734 bytes --]

From f4bc4ac5448eda45c9de938dceb280b9854c1b1b Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 12:23:56 +0100
Subject: [PATCH 2/2] ; Document use-package's :vc keyword

* doc/misc/use-package.texi (Installing packages):
(Install package):
Add documentation for :vc and link to the related chapter in the Emacs
manual.

* etc/NEWS: Mention :vc keyword
---
 doc/misc/use-package.texi | 41 +++++++++++++++++++++++++++++++++++++--
 etc/NEWS                  | 12 ++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
index 87105c4db0..6316af23ca 100644
--- a/doc/misc/use-package.texi
+++ b/doc/misc/use-package.texi
@@ -1554,8 +1554,10 @@ Installing packages
 (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
 @code{use-package} macro provides the @code{:ensure} and @code{:pin}
 keywords that interface with that package manager to automatically
-install packages.  This is particularly useful if you use your init
-file on more than one system.
+install packages.  Further, the @code{:vc} keyword may be used to
+control how package sources are fetched (@pxref{Fetching Package
+Sources,,, emacs, GNU Emacs Manual}).  This is particularly useful if
+you use your init file on more than one system.
 
 @menu
 * Install package::
@@ -1607,6 +1609,41 @@ Install package
 You can override the above setting for a single package by adding
 @w{@code{:ensure nil}} to its declaration.
 
+@findex :vc
+The @code{:vc} keyword can be used to control how packages are fetched.
+It accepts the same arguments as @code{package-vc-selected-packages},
+except that a name need not explicitly given: it is inferred from the
+declaration.  Further, the accepted property list is augmented by a
+@code{:rev} keyword, which has the same shape as the @code{REV} argument
+to @code{package-vc-install}.  Notably—even when not specified—@code{:rev}
+defaults to checking out the last release of the package.  You can use
+@code{:rev :head} to check out the latest commit.
+
+For example,
+
+@lisp
+@group
+(use-package foo
+  :vc (:url "https://bar.com/foo" :rev :head))
+@end group
+@end lisp
+
+would try—by invoking @code{package-vc-install}—to install the latest
+commit of the package @code{foo} from the specified remote.
+
+This can also be used for local packages, by combining it with the
+@code{:load-path} (@pxref{Load path}) keyword:
+
+@lisp
+@group
+(use-package foo
+  :vc t
+  :load-path "/path/to/foo/)
+@end group
+@end lisp
+
+The above dispatches to @code{package-vc-install-from-checkout}.
+
 @node Pinning packages
 @section Pinning packages using @code{:pin}
 @cindex installing package from specific archive
diff --git a/etc/NEWS b/etc/NEWS
index b121002b24..40cf44dd30 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -276,6 +276,18 @@ distracting and easily confused with actual code, or a significant
 early aid that relieves you from moving the buffer or reaching for the
 mouse to consult an error message.
 
+** use-package
+
+*** New ':vc' keyword
+This keyword enables the user to control how packages are fetched by
+utilising 'package-vc.el'.  By default, it relays its arguments to
+'package-vc-install', but—when combined with the ':load-path'
+keyword—it can also call upon 'package-vc-install-from-checkout'
+instead.  Further, if no revision is given via the ':rev' argument, we
+fall back to the last release (via 'package-vc-install's
+':last-release' argument).  To check out the last commit, use ':rev
+:head'.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
-- 
2.40.0


[-- Attachment #4: Type: text/plain, Size: 44 bytes --]


-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> Cc: 60418@debbugs.gnu.org, Felician Nemeth <felician.nemeth@gmail.com>,
>  stefankangas@gmail.com
> Date: Sun, 16 Apr 2023 17:43:02 +0200
> From:  Tony Zorman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Alright, attached are patches that contain the requested change: we now
> default to :last-release.

Thanks, please see below a few minor comments.

> +(defun use-package-vc-install (arg &optional local-path)
> +  "ARG is a list of the form (NAME OPTIONS REVISION).

This should tell what the function does, not just what the arguments
look like.

> +The optional LOCAL-PATH boolean decides whether
> +`package-vc-install-from-checkout' or `package-vc-install' will
> +end up being called."

This should tell explicitly which of the two is called when
LOCAL-PATH is nil and when it's non-nil.

> +(defun use-package-handler/:vc (name _keyword arg rest state)
> +  "Generate code for the :vc keyword."

I don't think this is an accurate description of what the function
does.  Also, we try very hard to mention at least the mandatory
arguments in the first line of the doc strings.

> @@ -1666,7 +1744,8 @@ use-package
>                   (compare with `custom-set-variables').
>  :custom-face     Call `custom-set-faces' with each face definition.
>  :ensure          Loads the package using package.el if necessary.
> -:pin             Pin the package to an archive."
> +:pin             Pin the package to an archive.
> +:vc              Integration with `package-vc.el'."

The description of other keywords say what is the effect of each one;
the description of :vc doesn't.  "Integration with package.el" is not
a useful description, it says nothing about what this keyword does.

> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are fetched.

Without saying more regarding what "fetched" is about, this
description is not as useful as it could have been.  You should give
some context which would explain how "fetching" is related to
use-package.

My suggestion, btw, is to use a more descriptive "downloading", not
"fetching".

> +It accepts the same arguments as @code{package-vc-selected-packages},

There should be a cross-reference here to the Emacs manual where it
describes package-vc-selected-packages.

> +except that a name need not explicitly given: it is inferred from the
                              ^
"be" is missing there.

> +declaration.  Further, the accepted property list is augmented by a
> +@code{:rev} keyword, which has the same shape as the @code{REV} argument
> +to @code{package-vc-install}.  Notably—even when not specified—@code{:rev}
                                         ^
Please don't use non-ASCII characters in Texinfo sources, that is
usually unnecessary.  In this case, to produce an em-dash, use two
dashes in a row -- they will be converted to em-dash on output.

> +would try—by invoking @code{package-vc-install}—to install the latest

Same here.

> +The above dispatches to @code{package-vc-install-from-checkout}.

A cross-reference here would be beneficial, again.

> +** use-package
> +
> +*** New ':vc' keyword

Heading lines in NEWS should end in a period.

Also, this entry should be marked with "+++", as the necessary changes
in the manuals are included.

> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to
> +'package-vc-install', but—when combined with the ':load-path'
> +keyword—it can also call upon 'package-vc-install-from-checkout'
> +instead.

Please also avoid non-ASCII characters in NEWS.

>           Further, if no revision is given via the ':rev' argument, we
> +fall back to the last release (via 'package-vc-install's
> +':last-release' argument).  To check out the last commit, use ':rev

You seem to like to say "Further," at the beginning of sentences, but
please be aware that this word usually adds no useful information, so
you can easily drop it in most cases.  For example, in the text above:
the sentence will be as informative without "Further" as with it.

Also, "we fall back" is not our style in documentation (who is "we"?).
We say "use-package will fall back" instead.

Thanks for working on this.

bug#60418: [PATCH] Add :vc keyword to use-package

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Tony Zorman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes:
> --- a/test/lisp/use-package/use-package-tests.el
> +++ b/test/lisp/use-package/use-package-tests.el
> @@ -1951,6 +1951,55 @@ bind-key/845
>      (should (eq (nth 1 binding) 'ignore))
>      (should (eq (nth 2 binding) nil))))
>  
> +(ert-deftest use-package-test/:vc-1 ()
> +  (match-expansion
> +   (use-package foo :vc (:url "bar"))
> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
> +
> +(ert-deftest use-package-test/:vc-2 ()
> +  (match-expansion
> +   (use-package foo
> +     :vc (baz . (:url "baz" :vc-backend "Git"
> +                 :main-file qux.el :rev "rev-string")))
> +   `(use-package-vc-install '(baz
> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
> +                              "rev-string")
> +                            nil)))

I'm curious, what is the significance to using quote versus quasiquote?
I noticed that there is no unquote inside the quasiquote at all, and am
wondering whether it is simply a typo or some sort of peculiarity in
`match-expansion'.

Admittedly I didn't look into the definition of `match-expansion' since
"git fetch" is taking its time on my end for some reason.

-- 
Best,


RY

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Sun, Apr 16 2023 19:10, Eli Zaretskii wrote:
> Thanks, please see below a few minor comments.

Thanks for the comments! Sorry, I should have mirrored existing
conventions more closely (plus, proofreading; I reckon very few of the
"further"s would have survived even the first one). I do have one
question before preparing a new patch though.

>> +(defun use-package-handler/:vc (name _keyword arg rest state)
>> +  "Generate code for the :vc keyword."
>
> I don't think this is an accurate description of what the function
> does.  Also, we try very hard to mention at least the mandatory
> arguments in the first line of the doc strings.

I think I initially copied this from the handler for ":custom" (where
it's perhaps more applicable) and then forgot to change it. Still, I'm
not totally sure what to write here without assuming that the reader
already knows what the handler for a use-package keyword does (which is,
I guess, why none of the other handlers have much in the way of
documentation). Would it be a good idea to link to a relevant entry in
the info manual ((use-package) > Keyword extensions > Creating an
extension)?

Thanks again!

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Tony Zorman <soliditsallgood@mailbox.org>
> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>  stefankangas@gmail.com
> Date: Mon, 17 Apr 2023 21:39:47 +0200
> 
> >> +(defun use-package-handler/:vc (name _keyword arg rest state)
> >> +  "Generate code for the :vc keyword."
> >
> > I don't think this is an accurate description of what the function
> > does.  Also, we try very hard to mention at least the mandatory
> > arguments in the first line of the doc strings.
> 
> I think I initially copied this from the handler for ":custom" (where
> it's perhaps more applicable) and then forgot to change it. Still, I'm
> not totally sure what to write here without assuming that the reader
> already knows what the handler for a use-package keyword does (which is,
> I guess, why none of the other handlers have much in the way of
> documentation). Would it be a good idea to link to a relevant entry in
> the info manual ((use-package) > Keyword extensions > Creating an
> extension)?

You could say something short on the first line assuming the reader
knows, then explain a bit more in the subsequent lines.  And yes,
pointing to the Info manual is also an option, but something should
still be in the doc string.

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Tue, Apr 18 2023 15:13, Eli Zaretskii wrote:
>> From: Tony Zorman <soliditsallgood@mailbox.org>
>> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>>  stefankangas@gmail.com
>> Date: Mon, 17 Apr 2023 21:39:47 +0200
>> 
>> >> +(defun use-package-handler/:vc (name _keyword arg rest state)
>> >> +  "Generate code for the :vc keyword."
>> >
>> > I don't think this is an accurate description of what the function
>> > does.  Also, we try very hard to mention at least the mandatory
>> > arguments in the first line of the doc strings.
>> 
>> I think I initially copied this from the handler for ":custom" (where
>> it's perhaps more applicable) and then forgot to change it. Still, I'm
>> not totally sure what to write here without assuming that the reader
>> already knows what the handler for a use-package keyword does (which is,
>> I guess, why none of the other handlers have much in the way of
>> documentation). Would it be a good idea to link to a relevant entry in
>> the info manual ((use-package) > Keyword extensions > Creating an
>> extension)?
>
> You could say something short on the first line assuming the reader
> knows, then explain a bit more in the subsequent lines.  And yes,
> pointing to the Info manual is also an option, but something should
> still be in the doc string.

I've now attached revised patches that should hopefully contain some
more useful documentation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-vc-keyword-to-use-package.patch --]
[-- Type: text/x-patch, Size: 10985 bytes --]

From c3c026655f372d84c9da4281d1a230629d13b55f Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 11:05:04 +0100
Subject: [PATCH 1/2] Add :vc keyword to use-package

* lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
(use-package-handler/:load-path): Insert 'load-path' into 'state'.
(use-package-vc-install): Install the package with package-vc.el.
(use-package-handler/:vc): Handler for the :vc keyword.
(use-package-normalize--vc-arg): Normalization for more complex
arguments to 'use-package-normalize/:vc', in order to make them
compatible with the specification of 'package-vc-selected-packages'.
(use-package-normalize/:vc): Normalizer for the :vc keyword.
(use-package): Document :vc.

* lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
Do not ensure a package when :vc is used in the declaration.

* test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
(use-package-test/:vc-2):
(use-package-test/:vc-3):
(use-package-test/:vc-4):
(use-package-test/:vc-5):
(use-package-test-normalize/:vc):
Add tests for :vc.
---
 lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
 lisp/use-package/use-package-ensure.el     |   3 +-
 test/lisp/use-package/use-package-tests.el |  49 +++++++++
 3 files changed, 160 insertions(+), 3 deletions(-)

diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 7ab5bdc276..0067ffc4f0 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -76,6 +76,7 @@ use-package-keywords
     :functions
     :preface
     :if :when :unless
+    :vc
     :no-require
     :catch
     :after
@@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
     #'use-package-normalize-paths))
 
 (defun use-package-handler/:load-path (name _keyword arg rest state)
-  (let ((body (use-package-process-keywords name rest state)))
+  (let ((body (use-package-process-keywords name rest
+                (plist-put state :load-path arg))))
     (use-package-concat
      (mapcar #'(lambda (path)
                  `(eval-and-compile (add-to-list 'load-path ,path)))
@@ -1577,6 +1579,109 @@ use-package-handler/:config
      (when use-package-compute-statistics
        `((use-package-statistics-gather :config ',name t))))))
 
+;;;; :vc
+
+(defun use-package-vc-install (arg &optional local-path)
+  "Install a package with `package-vc.el'.
+ARG is a list of the form (NAME OPTIONS REVISION), as returned by
+`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
+`package-vc-install-from-checkout'; otherwise, indicating a
+remote host, call `package-vc-install' instead."
+  (pcase-let* ((`(,name ,opts ,rev) arg)
+               (spec (if opts (cons name opts) name)))
+    (unless (package-installed-p name)
+      (if local-path
+          (package-vc-install-from-checkout local-path (symbol-name name))
+        (package-vc-install spec rev)))))
+
+(defun use-package-handler/:vc (name _keyword arg rest state)
+  "Generate code to install package NAME, or do so directly.
+When the use-package declaration is part of a byte-compiled file,
+install the package during compilation; otherwise, add it to the
+macro expansion and wait until runtime.  The remaining arguments
+are as follows:
+
+_KEYWORD is ignored.
+
+ARG is the normalized input to the `:vc' keyword, as returned by
+the `use-package-normalize/:vc' function.
+
+REST is a plist of other (following) keywords and their
+arguments, each having already been normalised by the respective
+function.
+
+STATE is a plist of any state that keywords processed before
+`:vc' (see `use-package-keywords') may have accumulated.
+
+Also see (info \"(use-package) Creating an extension\")."
+  (let ((body (use-package-process-keywords name rest state))
+        (local-path (car (plist-get state :load-path))))
+    ;; See `use-package-handler/:ensure' for an explanation.
+    (if (bound-and-true-p byte-compile-current-file)
+        (funcall #'use-package-vc-install arg local-path)        ; compile time
+      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
+
+(defun use-package-normalize--vc-arg (arg)
+  "Normalize possible arguments to the `:vc' keyword.
+ARG is a cons-cell of approximately the form that
+`package-vc-selected-packages' accepts, plus an additional `:rev'
+keyword.  If `:rev' is not given, it defaults to `:last-release'.
+
+Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
+with `package-vc-selected-packages' and REV is a (possibly nil,
+indicating the latest commit) revision."
+  (cl-flet* ((mk-string (s)
+	       (if (and s (stringp s)) s (symbol-name s)))
+             (mk-sym (s)
+               (if (and s (stringp s)) (intern s) s))
+	     (normalize (k v)
+               (pcase k
+                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
+                             ((eq v :head) nil)
+                             (t (mk-string v))))
+                 (:vc-backend (mk-sym v))
+                 (_ (mk-string v)))))
+    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
+                (`(,name . ,opts) arg))
+      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
+          (list name opts)
+        ;; Error handling
+        (cl-loop for (k _) on opts by #'cddr
+                 if (not (member k valid-kws))
+                 do (use-package-error
+                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
+                             k valid-kws)))
+        ;; Actual normalization
+        (list name
+              (cl-loop for (k v) on opts by #'cddr
+                       if (not (eq k :rev))
+                       nconc (list k (normalize k v)))
+              (normalize :rev (plist-get opts :rev)))))))
+
+(defun use-package-normalize/:vc (name _keyword args)
+  "Normalize possible arguments to the `:vc' keyword.
+NAME is the name of the `use-package' declaration, _KEYWORD is
+ignored, and ARGS it a list of arguments given to the `:vc'
+keyword, the cdr of which is ignored.
+
+See `use-package-normalize--vc-arg' for most of the actual
+normalization work.  Also see (info \"(use-package) Creating an
+extension\")."
+  (let ((arg (car args)))
+    (pcase arg
+      ((or 'nil 't) (list name))                 ; guess name
+      ((pred symbolp) (list arg))                ; use this name
+      ((pred stringp) (list name arg))           ; version string + guess name
+      ((pred plistp)                             ; plist + guess name
+       (use-package-normalize--vc-arg (cons name arg)))
+      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
+                                (pred stringp)))
+       (use-package-normalize--vc-arg arg))
+      (_ (use-package-error "Unrecognised argument to :vc.\
+ The keyword wants an argument of nil, t, a name of a package,\
+ or a cons-cell as accepted by `package-vc-selected-packages', where \
+ the accepted plist is augmented by a `:rev' keyword.")))))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
 ;;; The main macro
@@ -1666,7 +1771,9 @@ use-package
                  (compare with `custom-set-variables').
 :custom-face     Call `custom-set-faces' with each face definition.
 :ensure          Loads the package using package.el if necessary.
-:pin             Pin the package to an archive."
+:pin             Pin the package to an archive.
+:vc              Install the package directly from a version control system
+                 (using `package-vc.el')."
   (declare (indent defun))
   (unless (memq :disabled args)
     (macroexp-progn
diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
index e0ea982594..425a3e8917 100644
--- a/lisp/use-package/use-package-ensure.el
+++ b/lisp/use-package/use-package-ensure.el
@@ -182,7 +182,8 @@ use-package-ensure-elpa
 
 ;;;###autoload
 (defun use-package-handler/:ensure (name _keyword ensure rest state)
-  (let* ((body (use-package-process-keywords name rest state)))
+  (let* ((body (use-package-process-keywords name rest state))
+         (ensure (unless (plist-member rest :vc) ensure)))
     ;; We want to avoid installing packages when the `use-package' macro is
     ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
     ;; but still install packages when byte-compiling, to avoid requiring
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index 6374a0d103..b04d087e65 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1951,6 +1951,55 @@ bind-key/845
     (should (eq (nth 1 binding) 'ignore))
     (should (eq (nth 2 binding) nil))))
 
+(ert-deftest use-package-test/:vc-1 ()
+  (match-expansion
+   (use-package foo :vc (:url "bar"))
+   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
+
+(ert-deftest use-package-test/:vc-2 ()
+  (match-expansion
+   (use-package foo
+     :vc (baz . (:url "baz" :vc-backend "Git"
+                 :main-file qux.el :rev "rev-string")))
+   `(use-package-vc-install '(baz
+                              (:url "baz" :vc-backend Git :main-file "qux.el")
+                              "rev-string")
+                            nil)))
+
+(ert-deftest use-package-test/:vc-3 ()
+  (match-expansion
+   (use-package foo :vc (bar . "baz"))
+   `(use-package-vc-install '(bar "baz") nil)))
+
+(ert-deftest use-package-test/:vc-4 ()
+  (match-expansion
+   (use-package foo :vc (bar . (:url "baz" :rev :head)))
+   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
+
+(ert-deftest use-package-test/:vc-5 ()
+  (let ((load-path? '(pred (apply-partially
+                            #'string=
+                            (expand-file-name "bar" user-emacs-directory)))))
+    (match-expansion
+     (use-package foo :vc other-name :load-path "bar")
+     `(progn (eval-and-compile
+               (add-to-list 'load-path ,load-path?))
+             (use-package-vc-install '(other-name) ,load-path?)))))
+
+(ert-deftest use-package-test-normalize/:vc ()
+  (should (equal '(foo "version-string")
+                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
+  (should (equal '(bar "version-string")
+                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
+  (should (equal '(foo (:url "bar") "baz")
+                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc '(t))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc nil)))
+  (should (equal '(bar)
+                 (use-package-normalize/:vc 'foo :vc '(bar)))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; no-update-autoloads: t
-- 
2.40.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Document-use-package-s-vc-keyword.patch --]
[-- Type: text/x-patch, Size: 3809 bytes --]

From 652bb5b80c57f9891f74c48f66e96fba29007f44 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 12:23:56 +0100
Subject: [PATCH 2/2] ; Document use-package's :vc keyword

* doc/misc/use-package.texi (Installing packages):
(Install package):
Add documentation for :vc and link to the related chapter in the Emacs
manual.

* etc/NEWS: Mention :vc keyword
---
 doc/misc/use-package.texi | 45 +++++++++++++++++++++++++++++++++++++--
 etc/NEWS                  | 13 +++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
index 87105c4db0..d1d847c0e0 100644
--- a/doc/misc/use-package.texi
+++ b/doc/misc/use-package.texi
@@ -1554,8 +1554,11 @@ Installing packages
 (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
 @code{use-package} macro provides the @code{:ensure} and @code{:pin}
 keywords that interface with that package manager to automatically
-install packages.  This is particularly useful if you use your init
-file on more than one system.
+install packages.  The @code{:vc} keyword may be used to control how
+package sources are downloaded (e.g., from remote hosts)
+(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This
+is particularly useful if you use your init file on more than one
+system.
 
 @menu
 * Install package::
@@ -1607,6 +1610,44 @@ Install package
 You can override the above setting for a single package by adding
 @w{@code{:ensure nil}} to its declaration.
 
+@findex :vc
+The @code{:vc} keyword can be used to control how packages are
+downloaded and/or installed.  It accepts the same arguments as
+@code{package-vc-selected-packages} (@pxref{Specifying Package
+Sources,,, emacs, GNU Emacs Manual}), except that a name need not
+explicitly be given: it is inferred from the declaration.  The
+accepted property list is augmented by a @code{:rev} keyword, which
+has the same shape as the @code{REV} argument to
+@code{package-vc-install}.  Notably -- even when not specified --
+@code{:rev} defaults to checking out the last release of the package.
+You can use @code{:rev :head} to check out the latest commit.
+
+For example,
+
+@lisp
+@group
+(use-package foo
+  :vc (:url "https://bar.com/foo" :rev :head))
+@end group
+@end lisp
+
+would try -- by invoking @code{package-vc-install} -- to install the
+latest commit of the package @code{foo} from the specified remote.
+
+This can also be used for local packages, by combining it with the
+@code{:load-path} (@pxref{Load path}) keyword:
+
+@lisp
+@group
+(use-package foo
+  :vc t
+  :load-path "/path/to/foo/)
+@end group
+@end lisp
+
+The above dispatches to @code{package-vc-install-from-checkout}
+(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).
+
 @node Pinning packages
 @section Pinning packages using @code{:pin}
 @cindex installing package from specific archive
diff --git a/etc/NEWS b/etc/NEWS
index b121002b24..c63bd42fc8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -276,6 +276,19 @@ distracting and easily confused with actual code, or a significant
 early aid that relieves you from moving the buffer or reaching for the
 mouse to consult an error message.
 
+** use-package
+
++++
+*** New ':vc' keyword.
+This keyword enables the user to control how packages are fetched by
+utilising 'package-vc.el'.  By default, it relays its arguments to
+'package-vc-install', but -- when combined with the ':load-path'
+keyword -- it can also call upon 'package-vc-install-from-checkout'
+instead.  If no revision is given via the ':rev' argument, use-package
+falls back to the last release (via 'package-vc-install's
+':last-release' argument).  To check out the last commit, use ':rev
+:head'.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
-- 
2.40.0


[-- Attachment #4: Type: text/plain, Size: 44 bytes --]


-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Tony Zorman <soliditsallgood@mailbox.org>
> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>  stefankangas@gmail.com
> Date: Wed, 19 Apr 2023 19:38:10 +0200

John, any comments to this changeset?

Philip, will you install this when you think it's ready?

> On Tue, Apr 18 2023 15:13, Eli Zaretskii wrote:
> >> From: Tony Zorman <soliditsallgood@mailbox.org>
> >> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
> >>  stefankangas@gmail.com
> >> Date: Mon, 17 Apr 2023 21:39:47 +0200
> >> 
> >> >> +(defun use-package-handler/:vc (name _keyword arg rest state)
> >> >> +  "Generate code for the :vc keyword."
> >> >
> >> > I don't think this is an accurate description of what the function
> >> > does.  Also, we try very hard to mention at least the mandatory
> >> > arguments in the first line of the doc strings.
> >> 
> >> I think I initially copied this from the handler for ":custom" (where
> >> it's perhaps more applicable) and then forgot to change it. Still, I'm
> >> not totally sure what to write here without assuming that the reader
> >> already knows what the handler for a use-package keyword does (which is,
> >> I guess, why none of the other handlers have much in the way of
> >> documentation). Would it be a good idea to link to a relevant entry in
> >> the info manual ((use-package) > Keyword extensions > Creating an
> >> extension)?
> >
> > You could say something short on the first line assuming the reader
> > knows, then explain a bit more in the subsequent lines.  And yes,
> > pointing to the Info manual is also an option, but something should
> > still be in the doc string.
> 
> I've now attached revised patches that should hopefully contain some
> more useful documentation.
> 
> >From c3c026655f372d84c9da4281d1a230629d13b55f Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 11:05:04 +0100
> Subject: [PATCH 1/2] Add :vc keyword to use-package
> 
> * lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
> (use-package-handler/:load-path): Insert 'load-path' into 'state'.
> (use-package-vc-install): Install the package with package-vc.el.
> (use-package-handler/:vc): Handler for the :vc keyword.
> (use-package-normalize--vc-arg): Normalization for more complex
> arguments to 'use-package-normalize/:vc', in order to make them
> compatible with the specification of 'package-vc-selected-packages'.
> (use-package-normalize/:vc): Normalizer for the :vc keyword.
> (use-package): Document :vc.
> 
> * lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
> Do not ensure a package when :vc is used in the declaration.
> 
> * test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
> (use-package-test/:vc-2):
> (use-package-test/:vc-3):
> (use-package-test/:vc-4):
> (use-package-test/:vc-5):
> (use-package-test-normalize/:vc):
> Add tests for :vc.
> ---
>  lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
>  lisp/use-package/use-package-ensure.el     |   3 +-
>  test/lisp/use-package/use-package-tests.el |  49 +++++++++
>  3 files changed, 160 insertions(+), 3 deletions(-)
> 
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 7ab5bdc276..0067ffc4f0 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -76,6 +76,7 @@ use-package-keywords
>      :functions
>      :preface
>      :if :when :unless
> +    :vc
>      :no-require
>      :catch
>      :after
> @@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
>      #'use-package-normalize-paths))
>  
>  (defun use-package-handler/:load-path (name _keyword arg rest state)
> -  (let ((body (use-package-process-keywords name rest state)))
> +  (let ((body (use-package-process-keywords name rest
> +                (plist-put state :load-path arg))))
>      (use-package-concat
>       (mapcar #'(lambda (path)
>                   `(eval-and-compile (add-to-list 'load-path ,path)))
> @@ -1577,6 +1579,109 @@ use-package-handler/:config
>       (when use-package-compute-statistics
>         `((use-package-statistics-gather :config ',name t))))))
>  
> +;;;; :vc
> +
> +(defun use-package-vc-install (arg &optional local-path)
> +  "Install a package with `package-vc.el'.
> +ARG is a list of the form (NAME OPTIONS REVISION), as returned by
> +`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
> +`package-vc-install-from-checkout'; otherwise, indicating a
> +remote host, call `package-vc-install' instead."
> +  (pcase-let* ((`(,name ,opts ,rev) arg)
> +               (spec (if opts (cons name opts) name)))
> +    (unless (package-installed-p name)
> +      (if local-path
> +          (package-vc-install-from-checkout local-path (symbol-name name))
> +        (package-vc-install spec rev)))))
> +
> +(defun use-package-handler/:vc (name _keyword arg rest state)
> +  "Generate code to install package NAME, or do so directly.
> +When the use-package declaration is part of a byte-compiled file,
> +install the package during compilation; otherwise, add it to the
> +macro expansion and wait until runtime.  The remaining arguments
> +are as follows:
> +
> +_KEYWORD is ignored.
> +
> +ARG is the normalized input to the `:vc' keyword, as returned by
> +the `use-package-normalize/:vc' function.
> +
> +REST is a plist of other (following) keywords and their
> +arguments, each having already been normalised by the respective
> +function.
> +
> +STATE is a plist of any state that keywords processed before
> +`:vc' (see `use-package-keywords') may have accumulated.
> +
> +Also see (info \"(use-package) Creating an extension\")."
> +  (let ((body (use-package-process-keywords name rest state))
> +        (local-path (car (plist-get state :load-path))))
> +    ;; See `use-package-handler/:ensure' for an explanation.
> +    (if (bound-and-true-p byte-compile-current-file)
> +        (funcall #'use-package-vc-install arg local-path)        ; compile time
> +      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
> +
> +(defun use-package-normalize--vc-arg (arg)
> +  "Normalize possible arguments to the `:vc' keyword.
> +ARG is a cons-cell of approximately the form that
> +`package-vc-selected-packages' accepts, plus an additional `:rev'
> +keyword.  If `:rev' is not given, it defaults to `:last-release'.
> +
> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
> +with `package-vc-selected-packages' and REV is a (possibly nil,
> +indicating the latest commit) revision."
> +  (cl-flet* ((mk-string (s)
> +	       (if (and s (stringp s)) s (symbol-name s)))
> +             (mk-sym (s)
> +               (if (and s (stringp s)) (intern s) s))
> +	     (normalize (k v)
> +               (pcase k
> +                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
> +                             ((eq v :head) nil)
> +                             (t (mk-string v))))
> +                 (:vc-backend (mk-sym v))
> +                 (_ (mk-string v)))))
> +    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
> +                (`(,name . ,opts) arg))
> +      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
> +          (list name opts)
> +        ;; Error handling
> +        (cl-loop for (k _) on opts by #'cddr
> +                 if (not (member k valid-kws))
> +                 do (use-package-error
> +                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
> +                             k valid-kws)))
> +        ;; Actual normalization
> +        (list name
> +              (cl-loop for (k v) on opts by #'cddr
> +                       if (not (eq k :rev))
> +                       nconc (list k (normalize k v)))
> +              (normalize :rev (plist-get opts :rev)))))))
> +
> +(defun use-package-normalize/:vc (name _keyword args)
> +  "Normalize possible arguments to the `:vc' keyword.
> +NAME is the name of the `use-package' declaration, _KEYWORD is
> +ignored, and ARGS it a list of arguments given to the `:vc'
> +keyword, the cdr of which is ignored.
> +
> +See `use-package-normalize--vc-arg' for most of the actual
> +normalization work.  Also see (info \"(use-package) Creating an
> +extension\")."
> +  (let ((arg (car args)))
> +    (pcase arg
> +      ((or 'nil 't) (list name))                 ; guess name
> +      ((pred symbolp) (list arg))                ; use this name
> +      ((pred stringp) (list name arg))           ; version string + guess name
> +      ((pred plistp)                             ; plist + guess name
> +       (use-package-normalize--vc-arg (cons name arg)))
> +      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
> +                                (pred stringp)))
> +       (use-package-normalize--vc-arg arg))
> +      (_ (use-package-error "Unrecognised argument to :vc.\
> + The keyword wants an argument of nil, t, a name of a package,\
> + or a cons-cell as accepted by `package-vc-selected-packages', where \
> + the accepted plist is augmented by a `:rev' keyword.")))))
> +
>  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  ;;
>  ;;; The main macro
> @@ -1666,7 +1771,9 @@ use-package
>                   (compare with `custom-set-variables').
>  :custom-face     Call `custom-set-faces' with each face definition.
>  :ensure          Loads the package using package.el if necessary.
> -:pin             Pin the package to an archive."
> +:pin             Pin the package to an archive.
> +:vc              Install the package directly from a version control system
> +                 (using `package-vc.el')."
>    (declare (indent defun))
>    (unless (memq :disabled args)
>      (macroexp-progn
> diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
> index e0ea982594..425a3e8917 100644
> --- a/lisp/use-package/use-package-ensure.el
> +++ b/lisp/use-package/use-package-ensure.el
> @@ -182,7 +182,8 @@ use-package-ensure-elpa
>  
>  ;;;###autoload
>  (defun use-package-handler/:ensure (name _keyword ensure rest state)
> -  (let* ((body (use-package-process-keywords name rest state)))
> +  (let* ((body (use-package-process-keywords name rest state))
> +         (ensure (unless (plist-member rest :vc) ensure)))
>      ;; We want to avoid installing packages when the `use-package' macro is
>      ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
>      ;; but still install packages when byte-compiling, to avoid requiring
> diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
> index 6374a0d103..b04d087e65 100644
> --- a/test/lisp/use-package/use-package-tests.el
> +++ b/test/lisp/use-package/use-package-tests.el
> @@ -1951,6 +1951,55 @@ bind-key/845
>      (should (eq (nth 1 binding) 'ignore))
>      (should (eq (nth 2 binding) nil))))
>  
> +(ert-deftest use-package-test/:vc-1 ()
> +  (match-expansion
> +   (use-package foo :vc (:url "bar"))
> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
> +
> +(ert-deftest use-package-test/:vc-2 ()
> +  (match-expansion
> +   (use-package foo
> +     :vc (baz . (:url "baz" :vc-backend "Git"
> +                 :main-file qux.el :rev "rev-string")))
> +   `(use-package-vc-install '(baz
> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
> +                              "rev-string")
> +                            nil)))
> +
> +(ert-deftest use-package-test/:vc-3 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . "baz"))
> +   `(use-package-vc-install '(bar "baz") nil)))
> +
> +(ert-deftest use-package-test/:vc-4 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . (:url "baz" :rev :head)))
> +   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
> +
> +(ert-deftest use-package-test/:vc-5 ()
> +  (let ((load-path? '(pred (apply-partially
> +                            #'string=
> +                            (expand-file-name "bar" user-emacs-directory)))))
> +    (match-expansion
> +     (use-package foo :vc other-name :load-path "bar")
> +     `(progn (eval-and-compile
> +               (add-to-list 'load-path ,load-path?))
> +             (use-package-vc-install '(other-name) ,load-path?)))))
> +
> +(ert-deftest use-package-test-normalize/:vc ()
> +  (should (equal '(foo "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
> +  (should (equal '(bar "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
> +  (should (equal '(foo (:url "bar") "baz")
> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc '(t))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc nil)))
> +  (should (equal '(bar)
> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
> +
>  ;; Local Variables:
>  ;; no-byte-compile: t
>  ;; no-update-autoloads: t
> -- 
> 2.40.0
> 
> 
> >From 652bb5b80c57f9891f74c48f66e96fba29007f44 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 12:23:56 +0100
> Subject: [PATCH 2/2] ; Document use-package's :vc keyword
> 
> * doc/misc/use-package.texi (Installing packages):
> (Install package):
> Add documentation for :vc and link to the related chapter in the Emacs
> manual.
> 
> * etc/NEWS: Mention :vc keyword
> ---
>  doc/misc/use-package.texi | 45 +++++++++++++++++++++++++++++++++++++--
>  etc/NEWS                  | 13 +++++++++++
>  2 files changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
> index 87105c4db0..d1d847c0e0 100644
> --- a/doc/misc/use-package.texi
> +++ b/doc/misc/use-package.texi
> @@ -1554,8 +1554,11 @@ Installing packages
>  (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
>  @code{use-package} macro provides the @code{:ensure} and @code{:pin}
>  keywords that interface with that package manager to automatically
> -install packages.  This is particularly useful if you use your init
> -file on more than one system.
> +install packages.  The @code{:vc} keyword may be used to control how
> +package sources are downloaded (e.g., from remote hosts)
> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This
> +is particularly useful if you use your init file on more than one
> +system.
>  
>  @menu
>  * Install package::
> @@ -1607,6 +1610,44 @@ Install package
>  You can override the above setting for a single package by adding
>  @w{@code{:ensure nil}} to its declaration.
>  
> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are
> +downloaded and/or installed.  It accepts the same arguments as
> +@code{package-vc-selected-packages} (@pxref{Specifying Package
> +Sources,,, emacs, GNU Emacs Manual}), except that a name need not
> +explicitly be given: it is inferred from the declaration.  The
> +accepted property list is augmented by a @code{:rev} keyword, which
> +has the same shape as the @code{REV} argument to
> +@code{package-vc-install}.  Notably -- even when not specified --
> +@code{:rev} defaults to checking out the last release of the package.
> +You can use @code{:rev :head} to check out the latest commit.
> +
> +For example,
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc (:url "https://bar.com/foo" :rev :head))
> +@end group
> +@end lisp
> +
> +would try -- by invoking @code{package-vc-install} -- to install the
> +latest commit of the package @code{foo} from the specified remote.
> +
> +This can also be used for local packages, by combining it with the
> +@code{:load-path} (@pxref{Load path}) keyword:
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc t
> +  :load-path "/path/to/foo/)
> +@end group
> +@end lisp
> +
> +The above dispatches to @code{package-vc-install-from-checkout}
> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).
> +
>  @node Pinning packages
>  @section Pinning packages using @code{:pin}
>  @cindex installing package from specific archive
> diff --git a/etc/NEWS b/etc/NEWS
> index b121002b24..c63bd42fc8 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -276,6 +276,19 @@ distracting and easily confused with actual code, or a significant
>  early aid that relieves you from moving the buffer or reaching for the
>  mouse to consult an error message.
>  
> +** use-package
> +
> ++++
> +*** New ':vc' keyword.
> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to
> +'package-vc-install', but -- when combined with the ':load-path'
> +keyword -- it can also call upon 'package-vc-install-from-checkout'
> +instead.  If no revision is given via the ':rev' argument, use-package
> +falls back to the last release (via 'package-vc-install's
> +':last-release' argument).  To check out the last commit, use ':rev
> +:head'.
> +
>  \f
>  * New Modes and Packages in Emacs 30.1
>  
> -- 
> 2.40.0

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> On Tue, Apr 18 2023 15:13, Eli Zaretskii wrote:
>>> From: Tony Zorman <soliditsallgood@mailbox.org>
>>> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>>>  stefankangas@gmail.com
>>> Date: Mon, 17 Apr 2023 21:39:47 +0200
>>> 
>>> >> +(defun use-package-handler/:vc (name _keyword arg rest state)
>>> >> +  "Generate code for the :vc keyword."
>>> >
>>> > I don't think this is an accurate description of what the function
>>> > does.  Also, we try very hard to mention at least the mandatory
>>> > arguments in the first line of the doc strings.
>>> 
>>> I think I initially copied this from the handler for ":custom" (where
>>> it's perhaps more applicable) and then forgot to change it. Still, I'm
>>> not totally sure what to write here without assuming that the reader
>>> already knows what the handler for a use-package keyword does (which is,
>>> I guess, why none of the other handlers have much in the way of
>>> documentation). Would it be a good idea to link to a relevant entry in
>>> the info manual ((use-package) > Keyword extensions > Creating an
>>> extension)?
>>
>> You could say something short on the first line assuming the reader
>> knows, then explain a bit more in the subsequent lines.  And yes,
>> pointing to the Info manual is also an option, but something should
>> still be in the doc string.
>
> I've now attached revised patches that should hopefully contain some
> more useful documentation.
>
> From c3c026655f372d84c9da4281d1a230629d13b55f Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 11:05:04 +0100
> Subject: [PATCH 1/2] Add :vc keyword to use-package
>
> * lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
> (use-package-handler/:load-path): Insert 'load-path' into 'state'.
> (use-package-vc-install): Install the package with package-vc.el.
> (use-package-handler/:vc): Handler for the :vc keyword.
> (use-package-normalize--vc-arg): Normalization for more complex
> arguments to 'use-package-normalize/:vc', in order to make them
> compatible with the specification of 'package-vc-selected-packages'.
> (use-package-normalize/:vc): Normalizer for the :vc keyword.
> (use-package): Document :vc.
>
> * lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
> Do not ensure a package when :vc is used in the declaration.
>
> * test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
> (use-package-test/:vc-2):
> (use-package-test/:vc-3):
> (use-package-test/:vc-4):
> (use-package-test/:vc-5):
> (use-package-test-normalize/:vc):
> Add tests for :vc.
> ---
>  lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
>  lisp/use-package/use-package-ensure.el     |   3 +-
>  test/lisp/use-package/use-package-tests.el |  49 +++++++++
>  3 files changed, 160 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 7ab5bdc276..0067ffc4f0 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -76,6 +76,7 @@ use-package-keywords
>      :functions
>      :preface
>      :if :when :unless
> +    :vc
>      :no-require
>      :catch
>      :after
> @@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
>      #'use-package-normalize-paths))
>  
>  (defun use-package-handler/:load-path (name _keyword arg rest state)
> -  (let ((body (use-package-process-keywords name rest state)))
> +  (let ((body (use-package-process-keywords name rest
> +                (plist-put state :load-path arg))))
>      (use-package-concat
>       (mapcar #'(lambda (path)
>                   `(eval-and-compile (add-to-list 'load-path ,path)))
> @@ -1577,6 +1579,109 @@ use-package-handler/:config
>       (when use-package-compute-statistics
>         `((use-package-statistics-gather :config ',name t))))))
>  
> +;;;; :vc
> +
> +(defun use-package-vc-install (arg &optional local-path)
> +  "Install a package with `package-vc.el'.
> +ARG is a list of the form (NAME OPTIONS REVISION), as returned by
> +`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
> +`package-vc-install-from-checkout'; otherwise, indicating a
> +remote host, call `package-vc-install' instead."
> +  (pcase-let* ((`(,name ,opts ,rev) arg)
> +               (spec (if opts (cons name opts) name)))
> +    (unless (package-installed-p name)
> +      (if local-path
> +          (package-vc-install-from-checkout local-path (symbol-name name))
> +        (package-vc-install spec rev)))))
> +
> +(defun use-package-handler/:vc (name _keyword arg rest state)
> +  "Generate code to install package NAME, or do so directly.
> +When the use-package declaration is part of a byte-compiled file,
> +install the package during compilation; otherwise, add it to the
> +macro expansion and wait until runtime.  The remaining arguments
> +are as follows:
> +
> +_KEYWORD is ignored.
> +
> +ARG is the normalized input to the `:vc' keyword, as returned by
> +the `use-package-normalize/:vc' function.
> +
> +REST is a plist of other (following) keywords and their
> +arguments, each having already been normalised by the respective
> +function.
> +
> +STATE is a plist of any state that keywords processed before
> +`:vc' (see `use-package-keywords') may have accumulated.
> +
> +Also see (info \"(use-package) Creating an extension\")."
> +  (let ((body (use-package-process-keywords name rest state))
> +        (local-path (car (plist-get state :load-path))))
> +    ;; See `use-package-handler/:ensure' for an explanation.
> +    (if (bound-and-true-p byte-compile-current-file)
> +        (funcall #'use-package-vc-install arg local-path)        ; compile time
> +      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
> +
> +(defun use-package-normalize--vc-arg (arg)
> +  "Normalize possible arguments to the `:vc' keyword.
> +ARG is a cons-cell of approximately the form that
> +`package-vc-selected-packages' accepts, plus an additional `:rev'
> +keyword.  If `:rev' is not given, it defaults to `:last-release'.
> +
> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
> +with `package-vc-selected-packages' and REV is a (possibly nil,
> +indicating the latest commit) revision."
> +  (cl-flet* ((mk-string (s)
> +	       (if (and s (stringp s)) s (symbol-name s)))
> +             (mk-sym (s)
> +               (if (and s (stringp s)) (intern s) s))
> +	     (normalize (k v)
> +               (pcase k
> +                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
> +                             ((eq v :head) nil)
> +                             (t (mk-string v))))
> +                 (:vc-backend (mk-sym v))
> +                 (_ (mk-string v)))))
> +    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
> +                (`(,name . ,opts) arg))
> +      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
> +          (list name opts)
> +        ;; Error handling
> +        (cl-loop for (k _) on opts by #'cddr
> +                 if (not (member k valid-kws))
> +                 do (use-package-error
> +                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
> +                             k valid-kws)))
> +        ;; Actual normalization
> +        (list name
> +              (cl-loop for (k v) on opts by #'cddr
> +                       if (not (eq k :rev))
> +                       nconc (list k (normalize k v)))
> +              (normalize :rev (plist-get opts :rev)))))))
> +
> +(defun use-package-normalize/:vc (name _keyword args)
> +  "Normalize possible arguments to the `:vc' keyword.
> +NAME is the name of the `use-package' declaration, _KEYWORD is
> +ignored, and ARGS it a list of arguments given to the `:vc'
> +keyword, the cdr of which is ignored.
> +
> +See `use-package-normalize--vc-arg' for most of the actual
> +normalization work.  Also see (info \"(use-package) Creating an
> +extension\")."
> +  (let ((arg (car args)))
> +    (pcase arg
> +      ((or 'nil 't) (list name))                 ; guess name
> +      ((pred symbolp) (list arg))                ; use this name
> +      ((pred stringp) (list name arg))           ; version string + guess name
> +      ((pred plistp)                             ; plist + guess name
> +       (use-package-normalize--vc-arg (cons name arg)))
> +      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
> +                                (pred stringp)))
> +       (use-package-normalize--vc-arg arg))
> +      (_ (use-package-error "Unrecognised argument to :vc.\
> + The keyword wants an argument of nil, t, a name of a package,\
> + or a cons-cell as accepted by `package-vc-selected-packages', where \
> + the accepted plist is augmented by a `:rev' keyword.")))))
> +
>  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  ;;
>  ;;; The main macro
> @@ -1666,7 +1771,9 @@ use-package
>                   (compare with `custom-set-variables').
>  :custom-face     Call `custom-set-faces' with each face definition.
>  :ensure          Loads the package using package.el if necessary.
> -:pin             Pin the package to an archive."
> +:pin             Pin the package to an archive.
> +:vc              Install the package directly from a version control system
> +                 (using `package-vc.el')."
>    (declare (indent defun))
>    (unless (memq :disabled args)
>      (macroexp-progn
> diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
> index e0ea982594..425a3e8917 100644
> --- a/lisp/use-package/use-package-ensure.el
> +++ b/lisp/use-package/use-package-ensure.el
> @@ -182,7 +182,8 @@ use-package-ensure-elpa
>  
>  ;;;###autoload
>  (defun use-package-handler/:ensure (name _keyword ensure rest state)
> -  (let* ((body (use-package-process-keywords name rest state)))
> +  (let* ((body (use-package-process-keywords name rest state))
> +         (ensure (unless (plist-member rest :vc) ensure)))

I think it is better to write this as (and (not (plist-member rest :vc))
ensure).

>      ;; We want to avoid installing packages when the `use-package' macro is
>      ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
>      ;; but still install packages when byte-compiling, to avoid requiring
> diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
> index 6374a0d103..b04d087e65 100644
> --- a/test/lisp/use-package/use-package-tests.el
> +++ b/test/lisp/use-package/use-package-tests.el
> @@ -1951,6 +1951,55 @@ bind-key/845
>      (should (eq (nth 1 binding) 'ignore))
>      (should (eq (nth 2 binding) nil))))
>  
> +(ert-deftest use-package-test/:vc-1 ()
> +  (match-expansion
> +   (use-package foo :vc (:url "bar"))
> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
> +
> +(ert-deftest use-package-test/:vc-2 ()
> +  (match-expansion
> +   (use-package foo
> +     :vc (baz . (:url "baz" :vc-backend "Git"
> +                 :main-file qux.el :rev "rev-string")))
> +   `(use-package-vc-install '(baz
> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
> +                              "rev-string")
> +                            nil)))
> +
> +(ert-deftest use-package-test/:vc-3 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . "baz"))
> +   `(use-package-vc-install '(bar "baz") nil)))
> +
> +(ert-deftest use-package-test/:vc-4 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . (:url "baz" :rev :head)))
> +   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
> +
> +(ert-deftest use-package-test/:vc-5 ()
> +  (let ((load-path? '(pred (apply-partially
> +                            #'string=
> +                            (expand-file-name "bar" user-emacs-directory)))))
> +    (match-expansion
> +     (use-package foo :vc other-name :load-path "bar")
> +     `(progn (eval-and-compile
> +               (add-to-list 'load-path ,load-path?))
> +             (use-package-vc-install '(other-name) ,load-path?)))))
> +
> +(ert-deftest use-package-test-normalize/:vc ()
> +  (should (equal '(foo "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
> +  (should (equal '(bar "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
> +  (should (equal '(foo (:url "bar") "baz")
> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc '(t))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc nil)))
> +  (should (equal '(bar)
> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))

On my machine, these tests appear to fail?  Also, it would probably be
useful to document them (at leat the ...-1 to ...-5 ones) so that a
failure can be more easily understood.

>  ;; Local Variables:
>  ;; no-byte-compile: t
>  ;; no-update-autoloads: t
> -- 
> 2.40.0
>
> From 652bb5b80c57f9891f74c48f66e96fba29007f44 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 12:23:56 +0100
> Subject: [PATCH 2/2] ; Document use-package's :vc keyword
>
> * doc/misc/use-package.texi (Installing packages):
> (Install package):
> Add documentation for :vc and link to the related chapter in the Emacs
> manual.
>
> * etc/NEWS: Mention :vc keyword
> ---
>  doc/misc/use-package.texi | 45 +++++++++++++++++++++++++++++++++++++--
>  etc/NEWS                  | 13 +++++++++++
>  2 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
> index 87105c4db0..d1d847c0e0 100644
> --- a/doc/misc/use-package.texi
> +++ b/doc/misc/use-package.texi
> @@ -1554,8 +1554,11 @@ Installing packages
>  (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
>  @code{use-package} macro provides the @code{:ensure} and @code{:pin}
>  keywords that interface with that package manager to automatically
> -install packages.  This is particularly useful if you use your init
> -file on more than one system.
> +install packages.  The @code{:vc} keyword may be used to control how
> +package sources are downloaded (e.g., from remote hosts)
> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This

The double-parentheses here look weird IMO.

> +is particularly useful if you use your init file on more than one
> +system.
>  
>  @menu
>  * Install package::
> @@ -1607,6 +1610,44 @@ Install package
>  You can override the above setting for a single package by adding
>  @w{@code{:ensure nil}} to its declaration.
>  
> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are
> +downloaded and/or installed.  

This sounds vague, I think it makes sense to start with some context
motivating why/when :vc should be used.  

>                                It accepts the same arguments as
> +@code{package-vc-selected-packages} (@pxref{Specifying Package
> +Sources,,, emacs, GNU Emacs Manual}), except that a name need not

This link does not appear to work.  I believe you can only link to a
node, and this is just a section.  Also, referring to the variable has
become an unnecessary redirection, since the variable documentation now
refers to the manual.

> +explicitly be given: it is inferred from the declaration.  The
> +accepted property list is augmented by a @code{:rev} keyword, which
> +has the same shape as the @code{REV} argument to
> +@code{package-vc-install}.  Notably -- even when not specified --
> +@code{:rev} defaults to checking out the last release of the package.
> +You can use @code{:rev :head} to check out the latest commit.

The term "head" is a bit Git-specific, how about something like "tip" or
"newest"?

> +For example,
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc (:url "https://bar.com/foo" :rev :head))
> +@end group
> +@end lisp

I think it would make sense to use the same examples as the Emacs manual
has been updated to use (BBDB) so that users can directly try out the
code.

> +would try -- by invoking @code{package-vc-install} -- to install the
> +latest commit of the package @code{foo} from the specified remote.
> +
> +This can also be used for local packages, by combining it with the
> +@code{:load-path} (@pxref{Load path}) keyword:
> +
> +@lisp
> +@group
> +(use-package foo
> +  :vc t
> +  :load-path "/path/to/foo/)
> +@end group
> +@end lisp
> +
> +The above dispatches to @code{package-vc-install-from-checkout}
> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).

Does this reference here really help?

>  @node Pinning packages
>  @section Pinning packages using @code{:pin}
>  @cindex installing package from specific archive
> diff --git a/etc/NEWS b/etc/NEWS
> index b121002b24..c63bd42fc8 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -276,6 +276,19 @@ distracting and easily confused with actual code, or a significant
>  early aid that relieves you from moving the buffer or reaching for the
>  mouse to consult an error message.
>  
> +** use-package
> +
> ++++
> +*** New ':vc' keyword.
> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to
> +'package-vc-install', but -- when combined with the ':load-path'
> +keyword -- it can also call upon 'package-vc-install-from-checkout'
> +instead.  If no revision is given via the ':rev' argument, use-package
> +falls back to the last release (via 'package-vc-install's
> +':last-release' argument).  To check out the last commit, use ':rev
> +:head'.
> +
>  \f
>  * New Modes and Packages in Emacs 30.1
>  
> -- 
> 2.40.0

(Sorry for all the back and forth, I hope you understand that this is
just to get stuff right and not for my own sake of being pedantic.  If
you don't have the time, I can also make the changes I'd just like to
hear your opinions.)

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tony Zorman <soliditsallgood@mailbox.org>
>> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>>  stefankangas@gmail.com
>> Date: Wed, 19 Apr 2023 19:38:10 +0200
>
> John, any comments to this changeset?
>
> Philip, will you install this when you think it's ready?

Can do, I just had a few more comments in my other message.

>> On Tue, Apr 18 2023 15:13, Eli Zaretskii wrote:
>> >> From: Tony Zorman <soliditsallgood@mailbox.org>
>> >> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>> >>  stefankangas@gmail.com
>> >> Date: Mon, 17 Apr 2023 21:39:47 +0200
>> >> 
>> >> >> +(defun use-package-handler/:vc (name _keyword arg rest state)
>> >> >> +  "Generate code for the :vc keyword."
>> >> >
>> >> > I don't think this is an accurate description of what the function
>> >> > does.  Also, we try very hard to mention at least the mandatory
>> >> > arguments in the first line of the doc strings.
>> >> 
>> >> I think I initially copied this from the handler for ":custom" (where
>> >> it's perhaps more applicable) and then forgot to change it. Still, I'm
>> >> not totally sure what to write here without assuming that the reader
>> >> already knows what the handler for a use-package keyword does (which is,
>> >> I guess, why none of the other handlers have much in the way of
>> >> documentation). Would it be a good idea to link to a relevant entry in
>> >> the info manual ((use-package) > Keyword extensions > Creating an
>> >> extension)?
>> >
>> > You could say something short on the first line assuming the reader
>> > knows, then explain a bit more in the subsequent lines.  And yes,
>> > pointing to the Info manual is also an option, but something should
>> > still be in the doc string.
>> 
>> I've now attached revised patches that should hopefully contain some
>> more useful documentation.
>> 
>> >From c3c026655f372d84c9da4281d1a230629d13b55f Mon Sep 17 00:00:00 2001
>> From: Tony Zorman <soliditsallgood@mailbox.org>
>> Date: Thu, 29 Dec 2022 11:05:04 +0100
>> Subject: [PATCH 1/2] Add :vc keyword to use-package
>> 
>> * lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
>> (use-package-handler/:load-path): Insert 'load-path' into 'state'.
>> (use-package-vc-install): Install the package with package-vc.el.
>> (use-package-handler/:vc): Handler for the :vc keyword.
>> (use-package-normalize--vc-arg): Normalization for more complex
>> arguments to 'use-package-normalize/:vc', in order to make them
>> compatible with the specification of 'package-vc-selected-packages'.
>> (use-package-normalize/:vc): Normalizer for the :vc keyword.
>> (use-package): Document :vc.
>> 
>> * lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
>> Do not ensure a package when :vc is used in the declaration.
>> 
>> * test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
>> (use-package-test/:vc-2):
>> (use-package-test/:vc-3):
>> (use-package-test/:vc-4):
>> (use-package-test/:vc-5):
>> (use-package-test-normalize/:vc):
>> Add tests for :vc.
>> ---
>>  lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
>>  lisp/use-package/use-package-ensure.el     |   3 +-
>>  test/lisp/use-package/use-package-tests.el |  49 +++++++++
>>  3 files changed, 160 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
>> index 7ab5bdc276..0067ffc4f0 100644
>> --- a/lisp/use-package/use-package-core.el
>> +++ b/lisp/use-package/use-package-core.el
>> @@ -76,6 +76,7 @@ use-package-keywords
>>      :functions
>>      :preface
>>      :if :when :unless
>> +    :vc
>>      :no-require
>>      :catch
>>      :after
>> @@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
>>      #'use-package-normalize-paths))
>>  
>>  (defun use-package-handler/:load-path (name _keyword arg rest state)
>> -  (let ((body (use-package-process-keywords name rest state)))
>> +  (let ((body (use-package-process-keywords name rest
>> +                (plist-put state :load-path arg))))
>>      (use-package-concat
>>       (mapcar #'(lambda (path)
>>                   `(eval-and-compile (add-to-list 'load-path ,path)))
>> @@ -1577,6 +1579,109 @@ use-package-handler/:config
>>       (when use-package-compute-statistics
>>         `((use-package-statistics-gather :config ',name t))))))
>>  
>> +;;;; :vc
>> +
>> +(defun use-package-vc-install (arg &optional local-path)
>> +  "Install a package with `package-vc.el'.
>> +ARG is a list of the form (NAME OPTIONS REVISION), as returned by
>> +`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
>> +`package-vc-install-from-checkout'; otherwise, indicating a
>> +remote host, call `package-vc-install' instead."
>> +  (pcase-let* ((`(,name ,opts ,rev) arg)
>> +               (spec (if opts (cons name opts) name)))
>> +    (unless (package-installed-p name)
>> +      (if local-path
>> +          (package-vc-install-from-checkout local-path (symbol-name name))
>> +        (package-vc-install spec rev)))))
>> +
>> +(defun use-package-handler/:vc (name _keyword arg rest state)
>> +  "Generate code to install package NAME, or do so directly.
>> +When the use-package declaration is part of a byte-compiled file,
>> +install the package during compilation; otherwise, add it to the
>> +macro expansion and wait until runtime.  The remaining arguments
>> +are as follows:
>> +
>> +_KEYWORD is ignored.
>> +
>> +ARG is the normalized input to the `:vc' keyword, as returned by
>> +the `use-package-normalize/:vc' function.
>> +
>> +REST is a plist of other (following) keywords and their
>> +arguments, each having already been normalised by the respective
>> +function.
>> +
>> +STATE is a plist of any state that keywords processed before
>> +`:vc' (see `use-package-keywords') may have accumulated.
>> +
>> +Also see (info \"(use-package) Creating an extension\")."
>> +  (let ((body (use-package-process-keywords name rest state))
>> +        (local-path (car (plist-get state :load-path))))
>> +    ;; See `use-package-handler/:ensure' for an explanation.
>> +    (if (bound-and-true-p byte-compile-current-file)
>> +        (funcall #'use-package-vc-install arg local-path)        ; compile time
>> +      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
>> +
>> +(defun use-package-normalize--vc-arg (arg)
>> +  "Normalize possible arguments to the `:vc' keyword.
>> +ARG is a cons-cell of approximately the form that
>> +`package-vc-selected-packages' accepts, plus an additional `:rev'
>> +keyword.  If `:rev' is not given, it defaults to `:last-release'.
>> +
>> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
>> +with `package-vc-selected-packages' and REV is a (possibly nil,
>> +indicating the latest commit) revision."
>> +  (cl-flet* ((mk-string (s)
>> +	       (if (and s (stringp s)) s (symbol-name s)))
>> +             (mk-sym (s)
>> +               (if (and s (stringp s)) (intern s) s))
>> +	     (normalize (k v)
>> +               (pcase k
>> +                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
>> +                             ((eq v :head) nil)
>> +                             (t (mk-string v))))
>> +                 (:vc-backend (mk-sym v))
>> +                 (_ (mk-string v)))))
>> +    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
>> +                (`(,name . ,opts) arg))
>> +      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
>> +          (list name opts)
>> +        ;; Error handling
>> +        (cl-loop for (k _) on opts by #'cddr
>> +                 if (not (member k valid-kws))
>> +                 do (use-package-error
>> +                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
>> +                             k valid-kws)))
>> +        ;; Actual normalization
>> +        (list name
>> +              (cl-loop for (k v) on opts by #'cddr
>> +                       if (not (eq k :rev))
>> +                       nconc (list k (normalize k v)))
>> +              (normalize :rev (plist-get opts :rev)))))))
>> +
>> +(defun use-package-normalize/:vc (name _keyword args)
>> +  "Normalize possible arguments to the `:vc' keyword.
>> +NAME is the name of the `use-package' declaration, _KEYWORD is
>> +ignored, and ARGS it a list of arguments given to the `:vc'
>> +keyword, the cdr of which is ignored.
>> +
>> +See `use-package-normalize--vc-arg' for most of the actual
>> +normalization work.  Also see (info \"(use-package) Creating an
>> +extension\")."
>> +  (let ((arg (car args)))
>> +    (pcase arg
>> +      ((or 'nil 't) (list name))                 ; guess name
>> +      ((pred symbolp) (list arg))                ; use this name
>> +      ((pred stringp) (list name arg))           ; version string + guess name
>> +      ((pred plistp)                             ; plist + guess name
>> +       (use-package-normalize--vc-arg (cons name arg)))
>> +      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
>> +                                (pred stringp)))
>> +       (use-package-normalize--vc-arg arg))
>> +      (_ (use-package-error "Unrecognised argument to :vc.\
>> + The keyword wants an argument of nil, t, a name of a package,\
>> + or a cons-cell as accepted by `package-vc-selected-packages', where \
>> + the accepted plist is augmented by a `:rev' keyword.")))))
>> +
>>  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>  ;;
>>  ;;; The main macro
>> @@ -1666,7 +1771,9 @@ use-package
>>                   (compare with `custom-set-variables').
>>  :custom-face     Call `custom-set-faces' with each face definition.
>>  :ensure          Loads the package using package.el if necessary.
>> -:pin             Pin the package to an archive."
>> +:pin             Pin the package to an archive.
>> +:vc              Install the package directly from a version control system
>> +                 (using `package-vc.el')."
>>    (declare (indent defun))
>>    (unless (memq :disabled args)
>>      (macroexp-progn
>> diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
>> index e0ea982594..425a3e8917 100644
>> --- a/lisp/use-package/use-package-ensure.el
>> +++ b/lisp/use-package/use-package-ensure.el
>> @@ -182,7 +182,8 @@ use-package-ensure-elpa
>>  
>>  ;;;###autoload
>>  (defun use-package-handler/:ensure (name _keyword ensure rest state)
>> -  (let* ((body (use-package-process-keywords name rest state)))
>> +  (let* ((body (use-package-process-keywords name rest state))
>> +         (ensure (unless (plist-member rest :vc) ensure)))
>>      ;; We want to avoid installing packages when the `use-package' macro is
>>      ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
>>      ;; but still install packages when byte-compiling, to avoid requiring
>> diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
>> index 6374a0d103..b04d087e65 100644
>> --- a/test/lisp/use-package/use-package-tests.el
>> +++ b/test/lisp/use-package/use-package-tests.el
>> @@ -1951,6 +1951,55 @@ bind-key/845
>>      (should (eq (nth 1 binding) 'ignore))
>>      (should (eq (nth 2 binding) nil))))
>>  
>> +(ert-deftest use-package-test/:vc-1 ()
>> +  (match-expansion
>> +   (use-package foo :vc (:url "bar"))
>> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
>> +
>> +(ert-deftest use-package-test/:vc-2 ()
>> +  (match-expansion
>> +   (use-package foo
>> +     :vc (baz . (:url "baz" :vc-backend "Git"
>> +                 :main-file qux.el :rev "rev-string")))
>> +   `(use-package-vc-install '(baz
>> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
>> +                              "rev-string")
>> +                            nil)))
>> +
>> +(ert-deftest use-package-test/:vc-3 ()
>> +  (match-expansion
>> +   (use-package foo :vc (bar . "baz"))
>> +   `(use-package-vc-install '(bar "baz") nil)))
>> +
>> +(ert-deftest use-package-test/:vc-4 ()
>> +  (match-expansion
>> +   (use-package foo :vc (bar . (:url "baz" :rev :head)))
>> +   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
>> +
>> +(ert-deftest use-package-test/:vc-5 ()
>> +  (let ((load-path? '(pred (apply-partially
>> +                            #'string=
>> +                            (expand-file-name "bar" user-emacs-directory)))))
>> +    (match-expansion
>> +     (use-package foo :vc other-name :load-path "bar")
>> +     `(progn (eval-and-compile
>> +               (add-to-list 'load-path ,load-path?))
>> +             (use-package-vc-install '(other-name) ,load-path?)))))
>> +
>> +(ert-deftest use-package-test-normalize/:vc ()
>> +  (should (equal '(foo "version-string")
>> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
>> +  (should (equal '(bar "version-string")
>> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
>> +  (should (equal '(foo (:url "bar") "baz")
>> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
>> +  (should (equal '(foo)
>> +                 (use-package-normalize/:vc 'foo :vc '(t))))
>> +  (should (equal '(foo)
>> +                 (use-package-normalize/:vc 'foo :vc nil)))
>> +  (should (equal '(bar)
>> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
>> +
>>  ;; Local Variables:
>>  ;; no-byte-compile: t
>>  ;; no-update-autoloads: t
>> -- 
>> 2.40.0
>> 
>> 
>> >From 652bb5b80c57f9891f74c48f66e96fba29007f44 Mon Sep 17 00:00:00 2001
>> From: Tony Zorman <soliditsallgood@mailbox.org>
>> Date: Thu, 29 Dec 2022 12:23:56 +0100
>> Subject: [PATCH 2/2] ; Document use-package's :vc keyword
>> 
>> * doc/misc/use-package.texi (Installing packages):
>> (Install package):
>> Add documentation for :vc and link to the related chapter in the Emacs
>> manual.
>> 
>> * etc/NEWS: Mention :vc keyword
>> ---
>>  doc/misc/use-package.texi | 45 +++++++++++++++++++++++++++++++++++++--
>>  etc/NEWS                  | 13 +++++++++++
>>  2 files changed, 56 insertions(+), 2 deletions(-)
>> 
>> diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
>> index 87105c4db0..d1d847c0e0 100644
>> --- a/doc/misc/use-package.texi
>> +++ b/doc/misc/use-package.texi
>> @@ -1554,8 +1554,11 @@ Installing packages
>>  (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
>>  @code{use-package} macro provides the @code{:ensure} and @code{:pin}
>>  keywords that interface with that package manager to automatically
>> -install packages.  This is particularly useful if you use your init
>> -file on more than one system.
>> +install packages.  The @code{:vc} keyword may be used to control how
>> +package sources are downloaded (e.g., from remote hosts)
>> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This
>> +is particularly useful if you use your init file on more than one
>> +system.
>>  
>>  @menu
>>  * Install package::
>> @@ -1607,6 +1610,44 @@ Install package
>>  You can override the above setting for a single package by adding
>>  @w{@code{:ensure nil}} to its declaration.
>>  
>> +@findex :vc
>> +The @code{:vc} keyword can be used to control how packages are
>> +downloaded and/or installed.  It accepts the same arguments as
>> +@code{package-vc-selected-packages} (@pxref{Specifying Package
>> +Sources,,, emacs, GNU Emacs Manual}), except that a name need not
>> +explicitly be given: it is inferred from the declaration.  The
>> +accepted property list is augmented by a @code{:rev} keyword, which
>> +has the same shape as the @code{REV} argument to
>> +@code{package-vc-install}.  Notably -- even when not specified --
>> +@code{:rev} defaults to checking out the last release of the package.
>> +You can use @code{:rev :head} to check out the latest commit.
>> +
>> +For example,
>> +
>> +@lisp
>> +@group
>> +(use-package foo
>> +  :vc (:url "https://bar.com/foo" :rev :head))
>> +@end group
>> +@end lisp
>> +
>> +would try -- by invoking @code{package-vc-install} -- to install the
>> +latest commit of the package @code{foo} from the specified remote.
>> +
>> +This can also be used for local packages, by combining it with the
>> +@code{:load-path} (@pxref{Load path}) keyword:
>> +
>> +@lisp
>> +@group
>> +(use-package foo
>> +  :vc t
>> +  :load-path "/path/to/foo/)
>> +@end group
>> +@end lisp
>> +
>> +The above dispatches to @code{package-vc-install-from-checkout}
>> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).
>> +
>>  @node Pinning packages
>>  @section Pinning packages using @code{:pin}
>>  @cindex installing package from specific archive
>> diff --git a/etc/NEWS b/etc/NEWS
>> index b121002b24..c63bd42fc8 100644
>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -276,6 +276,19 @@ distracting and easily confused with actual code, or a significant
>>  early aid that relieves you from moving the buffer or reaching for the
>>  mouse to consult an error message.
>>  
>> +** use-package
>> +
>> ++++
>> +*** New ':vc' keyword.
>> +This keyword enables the user to control how packages are fetched by
>> +utilising 'package-vc.el'.  By default, it relays its arguments to
>> +'package-vc-install', but -- when combined with the ':load-path'
>> +keyword -- it can also call upon 'package-vc-install-from-checkout'
>> +instead.  If no revision is given via the ':rev' argument, use-package
>> +falls back to the last release (via 'package-vc-install's
>> +':last-release' argument).  To check out the last commit, use ':rev
>> +:head'.
>> +
>>  \f
>>  * New Modes and Packages in Emacs 30.1
>>  
>> -- 
>> 2.40.0

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[John Wiegley]

>>>>> Philip Kaludercic <philipk@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>> John, any comments to this changeset?
>> 
>> Philip, will you install this when you think it's ready?

> Can do, I just had a few more comments in my other message.

These changes look fine to me.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Sat, Apr 22 2023 11:32, Philip Kaludercic wrote:
> (Sorry for all the back and forth, I hope you understand that this is
> just to get stuff right and not for my own sake of being pedantic.  If
> you don't have the time, I can also make the changes I'd just like to
> hear your opinions.)

No, I totally get it. Getting this stuff right the first time is much
easier than going back and trying to fix it afterwards.

I agree with most of your suggestions, so I'm just going to answer those
where I have questions/comments.

>> +(ert-deftest use-package-test/:vc-1 ()
>> +  (match-expansion
>> +   (use-package foo :vc (:url "bar"))
>> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
>> +
>> +(ert-deftest use-package-test/:vc-2 ()
>> +  (match-expansion
>> +   (use-package foo
>> +     :vc (baz . (:url "baz" :vc-backend "Git"
>> +                 :main-file qux.el :rev "rev-string")))
>> +   `(use-package-vc-install '(baz
>> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
>> +                              "rev-string")
>> +                            nil)))
>> +
>> +(ert-deftest use-package-test/:vc-3 ()
>> +  (match-expansion
>> +   (use-package foo :vc (bar . "baz"))
>> +   `(use-package-vc-install '(bar "baz") nil)))
>> +
>> +(ert-deftest use-package-test/:vc-4 ()
>> +  (match-expansion
>> +   (use-package foo :vc (bar . (:url "baz" :rev :head)))
>> +   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
>> +
>> +(ert-deftest use-package-test/:vc-5 ()
>> +  (let ((load-path? '(pred (apply-partially
>> +                            #'string=
>> +                            (expand-file-name "bar" user-emacs-directory)))))
>> +    (match-expansion
>> +     (use-package foo :vc other-name :load-path "bar")
>> +     `(progn (eval-and-compile
>> +               (add-to-list 'load-path ,load-path?))
>> +             (use-package-vc-install '(other-name) ,load-path?)))))
>> +
>> +(ert-deftest use-package-test-normalize/:vc ()
>> +  (should (equal '(foo "version-string")
>> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
>> +  (should (equal '(bar "version-string")
>> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
>> +  (should (equal '(foo (:url "bar") "baz")
>> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
>> +  (should (equal '(foo)
>> +                 (use-package-normalize/:vc 'foo :vc '(t))))
>> +  (should (equal '(foo)
>> +                 (use-package-normalize/:vc 'foo :vc nil)))
>> +  (should (equal '(bar)
>> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
>
> On my machine, these tests appear to fail?  Also, it would probably be
> useful to document them (at leat the ...-1 to ...-5 ones) so that a
> failure can be more easily understood.

They don't fail for me—maybe I'm running the tests wrong? So far, I've
done so interactively by evaluating the whole buffer and then running
`M-x ert'. The -1…-5 tests just test the macro expansion, so e.g.

    (ert-deftest use-package-test/:vc-1 ()
      (match-expansion
       (use-package foo :vc (:url "bar"))
       '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))

would test if the first argument to `match-expansion' actually expands
into the second one or not (which I guess explains why the error
messages are a bit hard to understand).   

>>                                It accepts the same arguments as
>> +@code{package-vc-selected-packages} (@pxref{Specifying Package
>> +Sources,,, emacs, GNU Emacs Manual}), except that a name need not
>
> This link does not appear to work.  I believe you can only link to a
> node, and this is just a section.  Also, referring to the variable has
> become an unnecessary redirection, since the variable documentation now
> refers to the manual.

Oh, I see. I have to admit that info links are still a bit of a mystery
to me; sorry. So should I just link to the closest node, which would be
"Fetching Package Sources" again, here?

>> +For example,
>> +
>> +@lisp
>> +@group
>> +(use-package foo
>> +  :vc (:url "https://bar.com/foo" :rev :head))
>> +@end group
>> +@end lisp
>
> I think it would make sense to use the same examples as the Emacs manual
> has been updated to use (BBDB) so that users can directly try out the
> code.

Sorry, I'm not sure I follow: the same examples as what exactly? Or do
you just mean an actual package that users could try out when executing
this code, instead of a dummy one?

>> +would try -- by invoking @code{package-vc-install} -- to install the
>> +latest commit of the package @code{foo} from the specified remote.
>> +
>> +This can also be used for local packages, by combining it with the
>> +@code{:load-path} (@pxref{Load path}) keyword:
>> +
>> +@lisp
>> +@group
>> +(use-package foo
>> +  :vc t
>> +  :load-path "/path/to/foo/)
>> +@end group
>> +@end lisp
>> +
>> +The above dispatches to @code{package-vc-install-from-checkout}
>> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).
>
> Does this reference here really help?

Perhaps not. I don't have any strong opinions about this, so if you
prefer I can remove this.

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> On Sat, Apr 22 2023 11:32, Philip Kaludercic wrote:
>> (Sorry for all the back and forth, I hope you understand that this is
>> just to get stuff right and not for my own sake of being pedantic.  If
>> you don't have the time, I can also make the changes I'd just like to
>> hear your opinions.)
>
> No, I totally get it. Getting this stuff right the first time is much
> easier than going back and trying to fix it afterwards.

OK, great.  I just didn't want to make this seem like a ritual GNU thing ^^.

> I agree with most of your suggestions, so I'm just going to answer those
> where I have questions/comments.
>
>>> +(ert-deftest use-package-test/:vc-1 ()
>>> +  (match-expansion
>>> +   (use-package foo :vc (:url "bar"))
>>> +   '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
>>> +
>>> +(ert-deftest use-package-test/:vc-2 ()
>>> +  (match-expansion
>>> +   (use-package foo
>>> +     :vc (baz . (:url "baz" :vc-backend "Git"
>>> +                 :main-file qux.el :rev "rev-string")))
>>> +   `(use-package-vc-install '(baz
>>> +                              (:url "baz" :vc-backend Git :main-file "qux.el")
>>> +                              "rev-string")
>>> +                            nil)))
>>> +
>>> +(ert-deftest use-package-test/:vc-3 ()
>>> +  (match-expansion
>>> +   (use-package foo :vc (bar . "baz"))
>>> +   `(use-package-vc-install '(bar "baz") nil)))
>>> +
>>> +(ert-deftest use-package-test/:vc-4 ()
>>> +  (match-expansion
>>> +   (use-package foo :vc (bar . (:url "baz" :rev :head)))
>>> +   '(use-package-vc-install '(bar (:url "baz") nil) nil)))
>>> +
>>> +(ert-deftest use-package-test/:vc-5 ()
>>> +  (let ((load-path? '(pred (apply-partially
>>> +                            #'string=
>>> +                            (expand-file-name "bar" user-emacs-directory)))))
>>> +    (match-expansion
>>> +     (use-package foo :vc other-name :load-path "bar")
>>> +     `(progn (eval-and-compile
>>> +               (add-to-list 'load-path ,load-path?))
>>> +             (use-package-vc-install '(other-name) ,load-path?)))))
>>> +
>>> +(ert-deftest use-package-test-normalize/:vc ()
>>> +  (should (equal '(foo "version-string")
>>> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
>>> +  (should (equal '(bar "version-string")
>>> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
>>> +  (should (equal '(foo (:url "bar") "baz")
>>> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
>>> +  (should (equal '(foo)
>>> +                 (use-package-normalize/:vc 'foo :vc '(t))))
>>> +  (should (equal '(foo)
>>> +                 (use-package-normalize/:vc 'foo :vc nil)))
>>> +  (should (equal '(bar)
>>> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
>>
>> On my machine, these tests appear to fail?  Also, it would probably be
>> useful to document them (at leat the ...-1 to ...-5 ones) so that a
>> failure can be more easily understood.
>
> They don't fail for me—maybe I'm running the tests wrong? So far, I've
> done so interactively by evaluating the whole buffer and then running
> `M-x ert'. The -1…-5 tests just test the macro expansion, so e.g.
>
>     (ert-deftest use-package-test/:vc-1 ()
>       (match-expansion
>        (use-package foo :vc (:url "bar"))
>        '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
>
> would test if the first argument to `match-expansion' actually expands
> into the second one or not (which I guess explains why the error
> messages are a bit hard to understand).   

But that isn't the case, or am I mistaken?  The use-package form expands
to a lot mroe than just (use-package-vc-install ...)?

>>>                                It accepts the same arguments as
>>> +@code{package-vc-selected-packages} (@pxref{Specifying Package
>>> +Sources,,, emacs, GNU Emacs Manual}), except that a name need not
>>
>> This link does not appear to work.  I believe you can only link to a
>> node, and this is just a section.  Also, referring to the variable has
>> become an unnecessary redirection, since the variable documentation now
>> refers to the manual.
>
> Oh, I see. I have to admit that info links are still a bit of a mystery
> to me; sorry. So should I just link to the closest node, which would be
> "Fetching Package Sources" again, here?

You should be able to test this yourself by compiling Emacs and then
opening the manual to see how it was compiled.

>>> +For example,
>>> +
>>> +@lisp
>>> +@group
>>> +(use-package foo
>>> +  :vc (:url "https://bar.com/foo" :rev :head))
>>> +@end group
>>> +@end lisp
>>
>> I think it would make sense to use the same examples as the Emacs manual
>> has been updated to use (BBDB) so that users can directly try out the
>> code.
>
> Sorry, I'm not sure I follow: the same examples as what exactly? Or do
> you just mean an actual package that users could try out when executing
> this code, instead of a dummy one?

Sorry, I should have been more explicit here.  I am referring to the
changes in 6e6e8b5c974202d2691c1971be66c1cb3788b7c1, where we moved some
of the documentation from package-vc.el to the Emacs manual.  Part of
the change included the addition of some package specification examples,
that I think would be a good fit here as well.

>>> +would try -- by invoking @code{package-vc-install} -- to install the
>>> +latest commit of the package @code{foo} from the specified remote.
>>> +
>>> +This can also be used for local packages, by combining it with the
>>> +@code{:load-path} (@pxref{Load path}) keyword:
>>> +
>>> +@lisp
>>> +@group
>>> +(use-package foo
>>> +  :vc t
>>> +  :load-path "/path/to/foo/)
>>> +@end group
>>> +@end lisp
>>> +
>>> +The above dispatches to @code{package-vc-install-from-checkout}
>>> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).
>>
>> Does this reference here really help?
>
> Perhaps not. I don't have any strong opinions about this, so if you
> prefer I can remove this.

I think it would read better.

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Sun, Apr 23 2023 12:35, Philip Kaludercic wrote:
> Tony Zorman <soliditsallgood@mailbox.org> writes:
>
>> On Sat, Apr 22 2023 11:32, Philip Kaludercic wrote:
>>> (Sorry for all the back and forth, I hope you understand that this is
>>> just to get stuff right and not for my own sake of being pedantic.  If
>>> you don't have the time, I can also make the changes I'd just like to
>>> hear your opinions.)
>>
>> No, I totally get it. Getting this stuff right the first time is much
>> easier than going back and trying to fix it afterwards.
>
> OK, great.  I just didn't want to make this seem like a ritual GNU thing ^^.

Well, maybe a little bit of that as well :)

>>> On my machine, these tests appear to fail?  Also, it would probably be
>>> useful to document them (at leat the ...-1 to ...-5 ones) so that a
>>> failure can be more easily understood.
>>
>> They don't fail for me—maybe I'm running the tests wrong? So far, I've
>> done so interactively by evaluating the whole buffer and then running
>> `M-x ert'. The -1…-5 tests just test the macro expansion, so e.g.
>>
>>     (ert-deftest use-package-test/:vc-1 ()
>>       (match-expansion
>>        (use-package foo :vc (:url "bar"))
>>        '(use-package-vc-install '(foo (:url "bar") :last-release) nil)))
>>
>> would test if the first argument to `match-expansion' actually expands
>> into the second one or not (which I guess explains why the error
>> messages are a bit hard to understand).   
>
> But that isn't the case, or am I mistaken?  The use-package form expands
> to a lot mroe than just (use-package-vc-install ...)?

I think a combination of 'use-package-expand-minimally' and
'macroexpand-1' does shrink it down to essentially just that.

Oh, but while writing this I think I found the issue—global variables,
as always. I have 'use-package-always-ensure' and
'use-package-always-defer' set, which obviously influences macro
expansion. The bottom line is that most examples need an extra '(require
'foo nil nil)' added, nothing too major. I'll fix that, thanks for
helping me figure it out!

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi,

attached are the updated patches; sorry that this took so long.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-vc-keyword-to-use-package.patch --]
[-- Type: text/x-patch, Size: 11233 bytes --]

From 2b5f99fcaec6c8f985c60ed3ff5e119a7fb58e39 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 11:05:04 +0100
Subject: [PATCH 1/2] Add :vc keyword to use-package

* lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
(use-package-handler/:load-path): Insert 'load-path' into 'state'.
(use-package-vc-install): Install the package with package-vc.el.
(use-package-handler/:vc): Handler for the :vc keyword.
(use-package-normalize--vc-arg): Normalization for more complex
arguments to 'use-package-normalize/:vc', in order to make them
compatible with the specification of 'package-vc-selected-packages'.
(use-package-normalize/:vc): Normalizer for the :vc keyword.
(use-package): Document :vc.

* lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
Do not ensure a package when :vc is used in the declaration.

* test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
(use-package-test/:vc-2):
(use-package-test/:vc-3):
(use-package-test/:vc-4):
(use-package-test/:vc-5):
(use-package-test-normalize/:vc):
Add tests for :vc.
---
 lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
 lisp/use-package/use-package-ensure.el     |   3 +-
 test/lisp/use-package/use-package-tests.el |  54 ++++++++++
 3 files changed, 165 insertions(+), 3 deletions(-)

diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 7ab5bdc276..e7e7ecc710 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -76,6 +76,7 @@ use-package-keywords
     :functions
     :preface
     :if :when :unless
+    :vc
     :no-require
     :catch
     :after
@@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
     #'use-package-normalize-paths))
 
 (defun use-package-handler/:load-path (name _keyword arg rest state)
-  (let ((body (use-package-process-keywords name rest state)))
+  (let ((body (use-package-process-keywords name rest
+                (plist-put state :load-path arg))))
     (use-package-concat
      (mapcar #'(lambda (path)
                  `(eval-and-compile (add-to-list 'load-path ,path)))
@@ -1577,6 +1579,109 @@ use-package-handler/:config
      (when use-package-compute-statistics
        `((use-package-statistics-gather :config ',name t))))))
 
+;;;; :vc
+
+(defun use-package-vc-install (arg &optional local-path)
+  "Install a package with `package-vc.el'.
+ARG is a list of the form (NAME OPTIONS REVISION), as returned by
+`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
+`package-vc-install-from-checkout'; otherwise, indicating a
+remote host, call `package-vc-install' instead."
+  (pcase-let* ((`(,name ,opts ,rev) arg)
+               (spec (if opts (cons name opts) name)))
+    (unless (package-installed-p name)
+      (if local-path
+          (package-vc-install-from-checkout local-path (symbol-name name))
+        (package-vc-install spec rev)))))
+
+(defun use-package-handler/:vc (name _keyword arg rest state)
+  "Generate code to install package NAME, or do so directly.
+When the use-package declaration is part of a byte-compiled file,
+install the package during compilation; otherwise, add it to the
+macro expansion and wait until runtime.  The remaining arguments
+are as follows:
+
+_KEYWORD is ignored.
+
+ARG is the normalized input to the `:vc' keyword, as returned by
+the `use-package-normalize/:vc' function.
+
+REST is a plist of other (following) keywords and their
+arguments, each having already been normalised by the respective
+function.
+
+STATE is a plist of any state that keywords processed before
+`:vc' (see `use-package-keywords') may have accumulated.
+
+Also see the Info node `(use-package) Creating an extension'."
+  (let ((body (use-package-process-keywords name rest state))
+        (local-path (car (plist-get state :load-path))))
+    ;; See `use-package-handler/:ensure' for an explanation.
+    (if (bound-and-true-p byte-compile-current-file)
+        (funcall #'use-package-vc-install arg local-path)        ; compile time
+      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
+
+(defun use-package-normalize--vc-arg (arg)
+  "Normalize possible arguments to the `:vc' keyword.
+ARG is a cons-cell of approximately the form that
+`package-vc-selected-packages' accepts, plus an additional `:rev'
+keyword.  If `:rev' is not given, it defaults to `:last-release'.
+
+Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
+with `package-vc-selected-packages' and REV is a (possibly nil,
+indicating the latest commit) revision."
+  (cl-flet* ((mk-string (s)
+	       (if (and s (stringp s)) s (symbol-name s)))
+             (mk-sym (s)
+               (if (and s (stringp s)) (intern s) s))
+	     (normalize (k v)
+               (pcase k
+                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
+                             ((eq v :newest) nil)
+                             (t (mk-string v))))
+                 (:vc-backend (mk-sym v))
+                 (_ (mk-string v)))))
+    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
+                (`(,name . ,opts) arg))
+      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
+          (list name opts)
+        ;; Error handling
+        (cl-loop for (k _) on opts by #'cddr
+                 if (not (member k valid-kws))
+                 do (use-package-error
+                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
+                             k valid-kws)))
+        ;; Actual normalization
+        (list name
+              (cl-loop for (k v) on opts by #'cddr
+                       if (not (eq k :rev))
+                       nconc (list k (normalize k v)))
+              (normalize :rev (plist-get opts :rev)))))))
+
+(defun use-package-normalize/:vc (name _keyword args)
+  "Normalize possible arguments to the `:vc' keyword.
+NAME is the name of the `use-package' declaration, _KEYWORD is
+ignored, and ARGS it a list of arguments given to the `:vc'
+keyword, the cdr of which is ignored.
+
+See `use-package-normalize--vc-arg' for most of the actual
+normalization work.  Also see the Info
+node `(use-package) Creating an extension'."
+  (let ((arg (car args)))
+    (pcase arg
+      ((or 'nil 't) (list name))                 ; guess name
+      ((pred symbolp) (list arg))                ; use this name
+      ((pred stringp) (list name arg))           ; version string + guess name
+      ((pred plistp)                             ; plist + guess name
+       (use-package-normalize--vc-arg (cons name arg)))
+      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
+                                (pred stringp)))
+       (use-package-normalize--vc-arg arg))
+      (_ (use-package-error "Unrecognised argument to :vc.\
+ The keyword wants an argument of nil, t, a name of a package,\
+ or a cons-cell as accepted by `package-vc-selected-packages', where \
+ the accepted plist is augmented by a `:rev' keyword.")))))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
 ;;; The main macro
@@ -1666,7 +1771,9 @@ use-package
                  (compare with `custom-set-variables').
 :custom-face     Call `custom-set-faces' with each face definition.
 :ensure          Loads the package using package.el if necessary.
-:pin             Pin the package to an archive."
+:pin             Pin the package to an archive.
+:vc              Install the package directly from a version control system
+                 (using `package-vc.el')."
   (declare (indent defun))
   (unless (memq :disabled args)
     (macroexp-progn
diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
index e0ea982594..395a0bbda0 100644
--- a/lisp/use-package/use-package-ensure.el
+++ b/lisp/use-package/use-package-ensure.el
@@ -182,7 +182,8 @@ use-package-ensure-elpa
 
 ;;;###autoload
 (defun use-package-handler/:ensure (name _keyword ensure rest state)
-  (let* ((body (use-package-process-keywords name rest state)))
+  (let* ((body (use-package-process-keywords name rest state))
+         (ensure (and (not (plist-member rest :vc)) ensure)))
     ;; We want to avoid installing packages when the `use-package' macro is
     ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
     ;; but still install packages when byte-compiling, to avoid requiring
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index 6374a0d103..c8c20fc51c 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1951,6 +1951,60 @@ bind-key/845
     (should (eq (nth 1 binding) 'ignore))
     (should (eq (nth 2 binding) nil))))
 
+(ert-deftest use-package-test/:vc-1 ()
+  (match-expansion
+   (use-package foo :vc (:url "bar"))
+   '(progn (use-package-vc-install '(foo (:url "bar") :last-release) nil)
+           (require 'foo nil nil))))
+
+(ert-deftest use-package-test/:vc-2 ()
+  (match-expansion
+   (use-package foo
+     :vc (baz . (:url "baz" :vc-backend "Git"
+                 :main-file qux.el :rev "rev-string")))
+   '(progn (use-package-vc-install '(baz
+                                     (:url "baz" :vc-backend Git :main-file "qux.el")
+                                     "rev-string")
+                                   nil)
+           (require 'foo nil nil))))
+
+(ert-deftest use-package-test/:vc-3 ()
+  (match-expansion
+   (use-package foo :vc (bar . "baz"))
+   '(progn (use-package-vc-install '(bar "baz") nil)
+           (require 'foo nil nil))))
+
+(ert-deftest use-package-test/:vc-4 ()
+  (match-expansion
+   (use-package foo :vc (bar . (:url "baz" :rev :newest)))
+   '(progn (use-package-vc-install '(bar (:url "baz") nil) nil)
+           (require 'foo nil nil))))
+
+(ert-deftest use-package-test/:vc-5 ()
+  (let ((load-path? '(pred (apply-partially
+                            #'string=
+                            (expand-file-name "bar" user-emacs-directory)))))
+    (match-expansion
+     (use-package foo :vc other-name :load-path "bar")
+     `(progn (eval-and-compile
+               (add-to-list 'load-path ,load-path?))
+             (use-package-vc-install '(other-name) ,load-path?)
+             (require 'foo nil nil)))))
+
+(ert-deftest use-package-test-normalize/:vc ()
+  (should (equal '(foo "version-string")
+                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
+  (should (equal '(bar "version-string")
+                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
+  (should (equal '(foo (:url "bar") "baz")
+                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc '(t))))
+  (should (equal '(foo)
+                 (use-package-normalize/:vc 'foo :vc nil)))
+  (should (equal '(bar)
+                 (use-package-normalize/:vc 'foo :vc '(bar)))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; no-update-autoloads: t
-- 
2.40.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Document-use-package-s-vc-keyword.patch --]
[-- Type: text/x-patch, Size: 3949 bytes --]

From fba961176cd6ffb9998413f25956f7f6e044e064 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Thu, 29 Dec 2022 12:23:56 +0100
Subject: [PATCH 2/2] ; Document use-package's :vc keyword

* doc/misc/use-package.texi (Installing packages):
(Install package):
Add documentation for :vc and link to the related chapter in the Emacs
manual.

* etc/NEWS: Mention :vc keyword
---
 doc/misc/use-package.texi | 50 +++++++++++++++++++++++++++++++++++++--
 etc/NEWS                  | 13 ++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
index 87105c4db0..d75cb67e08 100644
--- a/doc/misc/use-package.texi
+++ b/doc/misc/use-package.texi
@@ -1554,8 +1554,11 @@ Installing packages
 (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
 @code{use-package} macro provides the @code{:ensure} and @code{:pin}
 keywords that interface with that package manager to automatically
-install packages.  This is particularly useful if you use your init
-file on more than one system.
+install packages.  The @code{:vc} keyword may be used to control how
+package sources are downloaded; e.g., from remote hosts
+(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This
+is particularly useful if you use your init file on more than one
+system.
 
 @menu
 * Install package::
@@ -1607,6 +1610,49 @@ Install package
 You can override the above setting for a single package by adding
 @w{@code{:ensure nil}} to its declaration.
 
+@findex :vc
+The @code{:vc} keyword can be used to control how packages are
+downloaded and/or installed. More specifically, it allows one to fetch
+and update packages directly from a version control system. This is
+especially convenient when wanting to install a package that is not on
+any package archive.
+
+The keyword accepts the same arguments as specified in
+@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}, except
+that a name need not explicitly be given: it is inferred from the
+declaration.  The accepted property list is augmented by a @code{:rev}
+keyword, which has the same shape as the @code{REV} argument to
+@code{package-vc-install}.  Notably -- even when not specified --
+@code{:rev} defaults to checking out the last release of the package.
+You can use @code{:rev :newest} to check out the latest commit.
+
+For example,
+
+@example
+@group
+(use-package bbdb
+  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
+       :rev :newest))
+@end group
+@end example
+
+would try -- by invoking @code{package-vc-install} -- to install the
+latest commit of the package @code{foo} from the specified remote.
+
+This can also be used for local packages, by combining it with the
+@code{:load-path} (@pxref{Load path}) keyword:
+
+@example
+@group
+;; Use a local copy of BBDB instead of the one from GNU ELPA.
+(use-package bbdb
+  :vc t
+  :load-path "/path/to/bbdb/dir/")
+@end group
+@end example
+
+The above dispatches to @code{package-vc-install-from-checkout}.
+
 @node Pinning packages
 @section Pinning packages using @code{:pin}
 @cindex installing package from specific archive
diff --git a/etc/NEWS b/etc/NEWS
index b989f80f3c..20f6751121 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -322,6 +322,19 @@ instead of:
         and another_expression):
         do_something()
 
+** use-package
+
++++
+*** New ':vc' keyword.
+This keyword enables the user to control how packages are fetched by
+utilising 'package-vc.el'.  By default, it relays its arguments to
+'package-vc-install', but -- when combined with the ':load-path'
+keyword -- it can also call upon 'package-vc-install-from-checkout'
+instead.  If no revision is given via the ':rev' argument, use-package
+falls back to the last release (via 'package-vc-install's
+':last-release' argument).  To check out the last commit, use ':rev
+:newest'.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
-- 
2.40.1


[-- Attachment #4: Type: text/plain, Size: 44 bytes --]


-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> Hi,
>
> attached are the updated patches; sorry that this took so long.

No need to worry about that.

> From 2b5f99fcaec6c8f985c60ed3ff5e119a7fb58e39 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 11:05:04 +0100
> Subject: [PATCH 1/2] Add :vc keyword to use-package
>
> * lisp/use-package/use-package-core.el (use-package-keywords): Add :vc.
> (use-package-handler/:load-path): Insert 'load-path' into 'state'.
> (use-package-vc-install): Install the package with package-vc.el.
> (use-package-handler/:vc): Handler for the :vc keyword.
> (use-package-normalize--vc-arg): Normalization for more complex
> arguments to 'use-package-normalize/:vc', in order to make them
> compatible with the specification of 'package-vc-selected-packages'.
> (use-package-normalize/:vc): Normalizer for the :vc keyword.
> (use-package): Document :vc.
>
> * lisp/use-package/use-package-ensure.el (use-package-handler/:ensure):
> Do not ensure a package when :vc is used in the declaration.
>
> * test/lisp/use-package/use-package-tests.el (use-package-test/:vc-1):
> (use-package-test/:vc-2):
> (use-package-test/:vc-3):
> (use-package-test/:vc-4):
> (use-package-test/:vc-5):
> (use-package-test-normalize/:vc):
> Add tests for :vc.
> ---
>  lisp/use-package/use-package-core.el       | 111 ++++++++++++++++++++-
>  lisp/use-package/use-package-ensure.el     |   3 +-
>  test/lisp/use-package/use-package-tests.el |  54 ++++++++++
>  3 files changed, 165 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 7ab5bdc276..e7e7ecc710 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -76,6 +76,7 @@ use-package-keywords
>      :functions
>      :preface
>      :if :when :unless
> +    :vc
>      :no-require
>      :catch
>      :after
> @@ -1151,7 +1152,8 @@ use-package-normalize/:load-path
>      #'use-package-normalize-paths))
>  
>  (defun use-package-handler/:load-path (name _keyword arg rest state)
> -  (let ((body (use-package-process-keywords name rest state)))
> +  (let ((body (use-package-process-keywords name rest
> +                (plist-put state :load-path arg))))
>      (use-package-concat
>       (mapcar #'(lambda (path)
>                   `(eval-and-compile (add-to-list 'load-path ,path)))
> @@ -1577,6 +1579,109 @@ use-package-handler/:config
>       (when use-package-compute-statistics
>         `((use-package-statistics-gather :config ',name t))))))
>  
> +;;;; :vc
> +
> +(defun use-package-vc-install (arg &optional local-path)
> +  "Install a package with `package-vc.el'.
> +ARG is a list of the form (NAME OPTIONS REVISION), as returned by
> +`use-package-normalize--vc-arg'.  If LOCAL-PATH is non-nil, call
> +`package-vc-install-from-checkout'; otherwise, indicating a
> +remote host, call `package-vc-install' instead."
> +  (pcase-let* ((`(,name ,opts ,rev) arg)
> +               (spec (if opts (cons name opts) name)))
> +    (unless (package-installed-p name)
> +      (if local-path
> +          (package-vc-install-from-checkout local-path (symbol-name name))
> +        (package-vc-install spec rev)))))
> +
> +(defun use-package-handler/:vc (name _keyword arg rest state)
> +  "Generate code to install package NAME, or do so directly.
> +When the use-package declaration is part of a byte-compiled file,
> +install the package during compilation; otherwise, add it to the
> +macro expansion and wait until runtime.  The remaining arguments
> +are as follows:
> +
> +_KEYWORD is ignored.

I think we can remove this, the argument is explicitly marked as unused.

> +ARG is the normalized input to the `:vc' keyword, as returned by
> +the `use-package-normalize/:vc' function.
> +
> +REST is a plist of other (following) keywords and their
> +arguments, each having already been normalised by the respective
> +function.
> +
> +STATE is a plist of any state that keywords processed before
> +`:vc' (see `use-package-keywords') may have accumulated.
> +
> +Also see the Info node `(use-package) Creating an extension'."
> +  (let ((body (use-package-process-keywords name rest state))
> +        (local-path (car (plist-get state :load-path))))
> +    ;; See `use-package-handler/:ensure' for an explanation.
> +    (if (bound-and-true-p byte-compile-current-file)
> +        (funcall #'use-package-vc-install arg local-path)        ; compile time
> +      (push `(use-package-vc-install ',arg ,local-path) body)))) ; runtime
> +
> +(defun use-package-normalize--vc-arg (arg)
> +  "Normalize possible arguments to the `:vc' keyword.
> +ARG is a cons-cell of approximately the form that
> +`package-vc-selected-packages' accepts, plus an additional `:rev'
> +keyword.  If `:rev' is not given, it defaults to `:last-release'.
> +
> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
> +with `package-vc-selected-packages' and REV is a (possibly nil,
> +indicating the latest commit) revision."
> +  (cl-flet* ((mk-string (s)
> +	       (if (and s (stringp s)) s (symbol-name s)))
> +             (mk-sym (s)
> +               (if (and s (stringp s)) (intern s) s))
> +	     (normalize (k v)

It seems there is inconsistent spacing here.

> +               (pcase k
> +                 (:rev (cond ((or (eq v :last-release) (not v)) :last-release)
> +                             ((eq v :newest) nil)
> +                             (t (mk-string v))))
> +                 (:vc-backend (mk-sym v))
> +                 (_ (mk-string v)))))
> +    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend :rev))
> +                (`(,name . ,opts) arg))
> +      (if (stringp opts)                ; (NAME . VERSION-STRING) ?
> +          (list name opts)
> +        ;; Error handling
> +        (cl-loop for (k _) on opts by #'cddr
> +                 if (not (member k valid-kws))
> +                 do (use-package-error
> +                     (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
> +                             k valid-kws)))
> +        ;; Actual normalization
> +        (list name
> +              (cl-loop for (k v) on opts by #'cddr
> +                       if (not (eq k :rev))
> +                       nconc (list k (normalize k v)))
> +              (normalize :rev (plist-get opts :rev)))))))
> +
> +(defun use-package-normalize/:vc (name _keyword args)
> +  "Normalize possible arguments to the `:vc' keyword.
> +NAME is the name of the `use-package' declaration, _KEYWORD is
> +ignored, and ARGS it a list of arguments given to the `:vc'
> +keyword, the cdr of which is ignored.
> +
> +See `use-package-normalize--vc-arg' for most of the actual
> +normalization work.  Also see the Info
> +node `(use-package) Creating an extension'."
> +  (let ((arg (car args)))
> +    (pcase arg
> +      ((or 'nil 't) (list name))                 ; guess name
> +      ((pred symbolp) (list arg))                ; use this name
> +      ((pred stringp) (list name arg))           ; version string + guess name
> +      ((pred plistp)                             ; plist + guess name
> +       (use-package-normalize--vc-arg (cons name arg)))
> +      (`(,(pred symbolp) . ,(or (pred plistp)    ; plist/version string + name
> +                                (pred stringp)))
> +       (use-package-normalize--vc-arg arg))
> +      (_ (use-package-error "Unrecognised argument to :vc.\
> + The keyword wants an argument of nil, t, a name of a package,\
> + or a cons-cell as accepted by `package-vc-selected-packages', where \
> + the accepted plist is augmented by a `:rev' keyword.")))))
> +
>  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  ;;
>  ;;; The main macro
> @@ -1666,7 +1771,9 @@ use-package
>                   (compare with `custom-set-variables').
>  :custom-face     Call `custom-set-faces' with each face definition.
>  :ensure          Loads the package using package.el if necessary.
> -:pin             Pin the package to an archive."
> +:pin             Pin the package to an archive.
> +:vc              Install the package directly from a version control system
> +                 (using `package-vc.el')."
>    (declare (indent defun))
>    (unless (memq :disabled args)
>      (macroexp-progn
> diff --git a/lisp/use-package/use-package-ensure.el b/lisp/use-package/use-package-ensure.el
> index e0ea982594..395a0bbda0 100644
> --- a/lisp/use-package/use-package-ensure.el
> +++ b/lisp/use-package/use-package-ensure.el
> @@ -182,7 +182,8 @@ use-package-ensure-elpa
>  
>  ;;;###autoload
>  (defun use-package-handler/:ensure (name _keyword ensure rest state)
> -  (let* ((body (use-package-process-keywords name rest state)))
> +  (let* ((body (use-package-process-keywords name rest state))
> +         (ensure (and (not (plist-member rest :vc)) ensure)))
>      ;; We want to avoid installing packages when the `use-package' macro is
>      ;; being macro-expanded by elisp completion (see `lisp--local-variables'),
>      ;; but still install packages when byte-compiling, to avoid requiring
> diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
> index 6374a0d103..c8c20fc51c 100644
> --- a/test/lisp/use-package/use-package-tests.el
> +++ b/test/lisp/use-package/use-package-tests.el
> @@ -1951,6 +1951,60 @@ bind-key/845
>      (should (eq (nth 1 binding) 'ignore))
>      (should (eq (nth 2 binding) nil))))
>  
> +(ert-deftest use-package-test/:vc-1 ()
> +  (match-expansion
> +   (use-package foo :vc (:url "bar"))
> +   '(progn (use-package-vc-install '(foo (:url "bar") :last-release) nil)
> +           (require 'foo nil nil))))
> +
> +(ert-deftest use-package-test/:vc-2 ()
> +  (match-expansion
> +   (use-package foo
> +     :vc (baz . (:url "baz" :vc-backend "Git"
> +                 :main-file qux.el :rev "rev-string")))
> +   '(progn (use-package-vc-install '(baz
> +                                     (:url "baz" :vc-backend Git :main-file "qux.el")
> +                                     "rev-string")
> +                                   nil)
> +           (require 'foo nil nil))))
> +
> +(ert-deftest use-package-test/:vc-3 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . "baz"))
> +   '(progn (use-package-vc-install '(bar "baz") nil)
> +           (require 'foo nil nil))))
> +
> +(ert-deftest use-package-test/:vc-4 ()
> +  (match-expansion
> +   (use-package foo :vc (bar . (:url "baz" :rev :newest)))
> +   '(progn (use-package-vc-install '(bar (:url "baz") nil) nil)
> +           (require 'foo nil nil))))
> +
> +(ert-deftest use-package-test/:vc-5 ()
> +  (let ((load-path? '(pred (apply-partially
> +                            #'string=
> +                            (expand-file-name "bar" user-emacs-directory)))))
> +    (match-expansion
> +     (use-package foo :vc other-name :load-path "bar")
> +     `(progn (eval-and-compile
> +               (add-to-list 'load-path ,load-path?))
> +             (use-package-vc-install '(other-name) ,load-path?)
> +             (require 'foo nil nil)))))
> +
> +(ert-deftest use-package-test-normalize/:vc ()
> +  (should (equal '(foo "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '("version-string"))))
> +  (should (equal '(bar "version-string")
> +                 (use-package-normalize/:vc 'foo :vc '((bar . "version-string")))))
> +  (should (equal '(foo (:url "bar") "baz")
> +                 (use-package-normalize/:vc 'foo :vc '((:url "bar" :rev "baz")))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc '(t))))
> +  (should (equal '(foo)
> +                 (use-package-normalize/:vc 'foo :vc nil)))
> +  (should (equal '(bar)
> +                 (use-package-normalize/:vc 'foo :vc '(bar)))))
> +
>  ;; Local Variables:
>  ;; no-byte-compile: t
>  ;; no-update-autoloads: t
> -- 
> 2.40.1
>
> From fba961176cd6ffb9998413f25956f7f6e044e064 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Thu, 29 Dec 2022 12:23:56 +0100
> Subject: [PATCH 2/2] ; Document use-package's :vc keyword
>
> * doc/misc/use-package.texi (Installing packages):
> (Install package):
> Add documentation for :vc and link to the related chapter in the Emacs
> manual.
>
> * etc/NEWS: Mention :vc keyword
> ---
>  doc/misc/use-package.texi | 50 +++++++++++++++++++++++++++++++++++++--
>  etc/NEWS                  | 13 ++++++++++
>  2 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/doc/misc/use-package.texi b/doc/misc/use-package.texi
> index 87105c4db0..d75cb67e08 100644
> --- a/doc/misc/use-package.texi
> +++ b/doc/misc/use-package.texi
> @@ -1554,8 +1554,11 @@ Installing packages
>  (@pxref{Package Installation,,, emacs, GNU Emacs Manual}).  The
>  @code{use-package} macro provides the @code{:ensure} and @code{:pin}
>  keywords that interface with that package manager to automatically
> -install packages.  This is particularly useful if you use your init
> -file on more than one system.
> +install packages.  The @code{:vc} keyword may be used to control how
> +package sources are downloaded; e.g., from remote hosts
> +(@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}).  This
> +is particularly useful if you use your init file on more than one
> +system.
>  
>  @menu
>  * Install package::
> @@ -1607,6 +1610,49 @@ Install package
>  You can override the above setting for a single package by adding
>  @w{@code{:ensure nil}} to its declaration.
>  
> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are
> +downloaded and/or installed. More specifically, it allows one to fetch
> +and update packages directly from a version control system. This is
> +especially convenient when wanting to install a package that is not on
> +any package archive.
>
> +The keyword accepts the same arguments as specified in
> +@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}, except
> +that a name need not explicitly be given: it is inferred from the
> +declaration.  The accepted property list is augmented by a @code{:rev}
> +keyword, which has the same shape as the @code{REV} argument to
> +@code{package-vc-install}.  Notably -- even when not specified --
> +@code{:rev} defaults to checking out the last release of the package.
> +You can use @code{:rev :newest} to check out the latest commit.
> +
> +For example,
> +
> +@example
> +@group
> +(use-package bbdb
> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
> +       :rev :newest))
> +@end group
> +@end example
> +
> +would try -- by invoking @code{package-vc-install} -- to install the
> +latest commit of the package @code{foo} from the specified remote.
> +
> +This can also be used for local packages, by combining it with the
> +@code{:load-path} (@pxref{Load path}) keyword:
> +
> +@example
> +@group
> +;; Use a local copy of BBDB instead of the one from GNU ELPA.
> +(use-package bbdb
> +  :vc t
> +  :load-path "/path/to/bbdb/dir/")
> +@end group
> +@end example
> +
> +The above dispatches to @code{package-vc-install-from-checkout}.
> +
>  @node Pinning packages
>  @section Pinning packages using @code{:pin}
>  @cindex installing package from specific archive
> diff --git a/etc/NEWS b/etc/NEWS
> index b989f80f3c..20f6751121 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -322,6 +322,19 @@ instead of:
>          and another_expression):
>          do_something()
>  
> +** use-package
> +
> ++++
> +*** New ':vc' keyword.
> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to

I would not add the .el at the end.

> +'package-vc-install', but -- when combined with the ':load-path'
> +keyword -- it can also call upon 'package-vc-install-from-checkout'
> +instead.  If no revision is given via the ':rev' argument, use-package
> +falls back to the last release (via 'package-vc-install's
> +':last-release' argument).  To check out the last commit, use ':rev
> +:newest'.

I feel like this is too much information for the NEWS file, the first
sentence should be enough.

If you want to, I can make all these changes in your patch if you ack
them.  I'll try and test the code in the coming days to see if it all
works, and with John's and Eli's blessing will apply them to master.

>  \f
>  * New Modes and Packages in Emacs 30.1
>  
> -- 
> 2.40.1

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Tony Zorman <soliditsallgood@mailbox.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 60418@debbugs.gnu.org,
>  felician.nemeth@gmail.com, stefankangas@gmail.com
> Date: Mon, 01 May 2023 21:43:22 +0200
> 
> +@findex :vc
> +The @code{:vc} keyword can be used to control how packages are
> +downloaded and/or installed. More specifically, it allows one to fetch
> +and update packages directly from a version control system. This is
> +especially convenient when wanting to install a package that is not on
> +any package archive.

This paragraph uses just one space between sentences; please use two
of them, per our conventions.

> +The keyword accepts the same arguments as specified in
> +@pxref{Fetching Package Sources,,, emacs, GNU Emacs Manual}, except

Please don't use @pxref this way: it only looks well in HTML output,
but in all other outputs it looks awkward, if not incorrect.  Instead,
please use the more traditional and less "sexy" way:

  The keyword accepts the same form of specifications as
  @code{package-vc-install} (@pxref{Fetching Package Sources,,, emacs,
  GNU Emacs Manual}), except that...

> +declaration.  The accepted property list is augmented by a @code{:rev}
> +keyword, which has the same shape as the @code{REV} argument to
> +@code{package-vc-install}.

Instead of @code{REV}, please use @var{rev}: @var markup and
lower-case "rev".  This is the markup suitable for formal arguments to
functions in Texinfo sources.

> +@code{package-vc-install}.  Notably -- even when not specified --
> +@code{:rev} defaults to checking out the last release of the package.
> +You can use @code{:rev :newest} to check out the latest commit.
> +
> +For example,
> +
> +@example
> +@group
> +(use-package bbdb
> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
> +       :rev :newest))
> +@end group
> +@end example
> +
> +would try -- by invoking @code{package-vc-install} -- to install the
> +latest commit of the package @code{foo} from the specified remote.
                                      ^^^
A typo there.

Also, you say above "the latest release", but then "the latest
commit".  These two are not the same, and in fact I think talking
about "release" here is misleading, since you actually mean "commit".
For the same reason, I think the text should explain how to indicate a
commit that is not the latest one, because that is also not
self-evident, especially since the upstream VCS is not necessarily
Git.

> +*** New ':vc' keyword.

"*** New keyword ':vc'."

> +This keyword enables the user to control how packages are fetched by
> +utilising 'package-vc.el'.  By default, it relays its arguments to

We use US English spelling: "utilizing".  But I would actually suggest
to say "using" instead: there's no need to use overly-complicated
words when simpler ones will do.

> +'package-vc-install', but -- when combined with the ':load-path'
> +keyword -- it can also call upon 'package-vc-install-from-checkout'
> +instead.  If no revision is given via the ':rev' argument, use-package
> +falls back to the last release (via 'package-vc-install's
> +':last-release' argument).  To check out the last commit, use ':rev
> +:newest'.

Here, again, you mix up "release", "revision", and "commit".  It is
always better to use consistent terminology, and in this case I think
only "commit" is accurate.

Thanks.

bug#60418: [PATCH] Add :vc keyword to use-package

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Philip Kaludercic <philipk@posteo.net> writes:

> Tony Zorman <soliditsallgood@mailbox.org> writes:

>> +** use-package
>> +
>> ++++
>> +*** New ':vc' keyword.
>> +This keyword enables the user to control how packages are fetched by
>> +utilising 'package-vc.el'.  By default, it relays its arguments to

If I'm not mistaken, don't we prefer quoting like `this' in Emacs?  (Or
maybe NEWS has a different style to which I'm not familiar.)  If that is
correct, then all quotings in the patch would need to be adjusted
accordingly.

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]

bug#60418: [PATCH] Add :vc keyword to use-package

[Robert Pluim]

>>>>> On Tue, 02 May 2023 21:18:22 +0800, Ruijie Yu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> said:

    Ruijie> Philip Kaludercic <philipk@posteo.net> writes:

    >> Tony Zorman <soliditsallgood@mailbox.org> writes:

    >>> +** use-package
    >>> +
    >>> ++++
    >>> +*** New ':vc' keyword.
    >>> +This keyword enables the user to control how packages are fetched by
    >>> +utilising 'package-vc.el'.  By default, it relays its arguments to

    Ruijie> If I'm not mistaken, don't we prefer quoting like `this' in Emacs?  (Or
    Ruijie> maybe NEWS has a different style to which I'm not familiar.)  If that is
    Ruijie> correct, then all quotings in the patch would need to be adjusted
    Ruijie> accordingly.

In docstrings we use `this', in NEWS we use 'that' (and in commit
messages, although I think thereʼs less consistency there).

Robert
-- 

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Tue, May 02 2023 15:40, Eli Zaretskii wrote:
>> +@code{package-vc-install}.  Notably -- even when not specified --
>> +@code{:rev} defaults to checking out the last release of the package.
>> +You can use @code{:rev :newest} to check out the latest commit.
>> +
>> +For example,
>> +
>> +@example
>> +@group
>> +(use-package bbdb
>> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
>> +       :rev :newest))
>> +@end group
>> +@end example
>> +
>> +would try -- by invoking @code{package-vc-install} -- to install the
>> +latest commit of the package @code{foo} from the specified remote.
>                                       ^^^
> A typo there.
>
> Also, you say above "the latest release", but then "the latest
> commit".  These two are not the same, and in fact I think talking
> about "release" here is misleading, since you actually mean "commit".
> For the same reason, I think the text should explain how to indicate a
> commit that is not the latest one, because that is also not
> self-evident, especially since the upstream VCS is not necessarily
> Git.

I think the terminology of commit and release I use here are consistent,
though maybe the wording is perhaps not entirely clear.

If :rev is not explicitly given, then :vc falls back to calling
package-vc-install (in the case of a non-local upstream) with the
:last-release keyword in place of its REV argument (which is called a
revision in the docs). Since package-vc.el freely calls :last-release a
release of a package, I figured this terminology is appropriate here. It
is only when :rev :newest is given that I talk about commits, which
should also be accurate. Or perhaps you mean that I mistakenly talk
about the latest release in some other place that I've overlooked just
now?

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Mon, May 01 2023 20:01, Philip Kaludercic wrote:
> Tony Zorman <soliditsallgood@mailbox.org> writes:
>> +(defun use-package-normalize--vc-arg (arg)
>> +  "Normalize possible arguments to the `:vc' keyword.
>> +ARG is a cons-cell of approximately the form that
>> +`package-vc-selected-packages' accepts, plus an additional `:rev'
>> +keyword.  If `:rev' is not given, it defaults to `:last-release'.
>> +
>> +Returns a list (NAME SPEC REV), where (NAME . SPEC) is compliant
>> +with `package-vc-selected-packages' and REV is a (possibly nil,
>> +indicating the latest commit) revision."
>> +  (cl-flet* ((mk-string (s)
>> +	       (if (and s (stringp s)) s (symbol-name s)))
>> +             (mk-sym (s)
>> +               (if (and s (stringp s)) (intern s) s))
>> +	     (normalize (k v)
>
> It seems there is inconsistent spacing here.

Whoops, some tabs must have snuck into the code. 

>> +** use-package
>> +
>> ++++
>> +*** New ':vc' keyword.
>> +This keyword enables the user to control how packages are fetched by
>> +utilising 'package-vc.el'.  By default, it relays its arguments to
>
> I would not add the .el at the end.

Is there a standard way to refer to packages? I feel like `package-vc'
would be too easy to mistake for a function/variable, but maybe that is
just paranoia.

>> +'package-vc-install', but -- when combined with the ':load-path'
>> +keyword -- it can also call upon 'package-vc-install-from-checkout'
>> +instead.  If no revision is given via the ':rev' argument, use-package
>> +falls back to the last release (via 'package-vc-install's
>> +':last-release' argument).  To check out the last commit, use ':rev
>> +:newest'.
>
> I feel like this is too much information for the NEWS file, the first
> sentence should be enough.

I think it's not totally unprecedented in terms of word count, but sure,
if you think it's too long I wouldn't be opposed to shortening it. 

> If you want to, I can make all these changes in your patch if you ack
> them.  I'll try and test the code in the coming days to see if it all
> works, and with John's and Eli's blessing will apply them to master.

Given my schedule for the next few weeks, this would be absolutely
fantastic—thank you!

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Ruijie Yu <ruijie@netyu.xyz>
> Cc: Philip Kaludercic <philipk@posteo.net>, Eli Zaretskii <eliz@gnu.org>,
>  felician.nemeth@gmail.com, 60418@debbugs.gnu.org, stefankangas@gmail.com
> Date: Tue, 02 May 2023 21:18:22 +0800
> 
> 
> >> +** use-package
> >> +
> >> ++++
> >> +*** New ':vc' keyword.
> >> +This keyword enables the user to control how packages are fetched by
> >> +utilising 'package-vc.el'.  By default, it relays its arguments to
> 
> If I'm not mistaken, don't we prefer quoting like `this' in Emacs?

Not in NEWS, no.  In NEWS, we quote 'like this'.

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Tony Zorman <soliditsallgood@mailbox.org>
> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>  stefankangas@gmail.com
> Date: Tue, 02 May 2023 16:22:17 +0200
> 
> On Tue, May 02 2023 15:40, Eli Zaretskii wrote:
> >> +@code{package-vc-install}.  Notably -- even when not specified --
> >> +@code{:rev} defaults to checking out the last release of the package.
> >> +You can use @code{:rev :newest} to check out the latest commit.
> >> +
> >> +For example,
> >> +
> >> +@example
> >> +@group
> >> +(use-package bbdb
> >> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
> >> +       :rev :newest))
> >> +@end group
> >> +@end example
> >> +
> >> +would try -- by invoking @code{package-vc-install} -- to install the
> >> +latest commit of the package @code{foo} from the specified remote.
> >                                       ^^^
> > A typo there.
> >
> > Also, you say above "the latest release", but then "the latest
> > commit".  These two are not the same, and in fact I think talking
> > about "release" here is misleading, since you actually mean "commit".
> > For the same reason, I think the text should explain how to indicate a
> > commit that is not the latest one, because that is also not
> > self-evident, especially since the upstream VCS is not necessarily
> > Git.
> 
> I think the terminology of commit and release I use here are consistent,
> though maybe the wording is perhaps not entirely clear.

Not in my eyes, it isn't.  E.g., look at any GitHub repository: there
are "commits" there, and there are "releases", and they are not the
same.

> If :rev is not explicitly given, then :vc falls back to calling
> package-vc-install (in the case of a non-local upstream) with the
> :last-release keyword in place of its REV argument (which is called a
> revision in the docs). Since package-vc.el freely calls :last-release a
> release of a package, I figured this terminology is appropriate here. It
> is only when :rev :newest is given that I talk about commits, which
> should also be accurate. Or perhaps you mean that I mistakenly talk
> about the latest release in some other place that I've overlooked just
> now?

All I know is that when I've read the documentation you wrote, I asked
myself "what is meant by 'release' here?"  I found the answer when you
later wrote "last commit".

Are you talking about commits?  More generally, what kind of "release
IDs" does :rev accept as its valid value?

I understand that the same confusion could exist elsewhere, but that
doesn't mean we should proliferate it or even live with what we have.
We should instead clarify this in every place where we use this
terminology.

So let's figure out what are these "releases", and then let's examine
the existing and the new documentation and see if we need to get our
terminology right there.

Thanks.

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

I suppose Philip would be more qualified than me to answer this, but
I'll try.

On Tue, May 02 2023 18:16, Eli Zaretskii wrote:
>> From: Tony Zorman <soliditsallgood@mailbox.org>
>> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>>  stefankangas@gmail.com
>> Date: Tue, 02 May 2023 16:22:17 +0200
>> 
>> On Tue, May 02 2023 15:40, Eli Zaretskii wrote:
>> >> +@code{package-vc-install}.  Notably -- even when not specified --
>> >> +@code{:rev} defaults to checking out the last release of the package.
>> >> +You can use @code{:rev :newest} to check out the latest commit.
>> >> +
>> >> +For example,
>> >> +
>> >> +@example
>> >> +@group
>> >> +(use-package bbdb
>> >> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
>> >> +       :rev :newest))
>> >> +@end group
>> >> +@end example
>> >> +
>> >> +would try -- by invoking @code{package-vc-install} -- to install the
>> >> +latest commit of the package @code{foo} from the specified remote.
>> >                                       ^^^
>> > A typo there.
>> >
>> > Also, you say above "the latest release", but then "the latest
>> > commit".  These two are not the same, and in fact I think talking
>> > about "release" here is misleading, since you actually mean "commit".
>> > For the same reason, I think the text should explain how to indicate a
>> > commit that is not the latest one, because that is also not
>> > self-evident, especially since the upstream VCS is not necessarily
>> > Git.
>> 
>> I think the terminology of commit and release I use here are consistent,
>> though maybe the wording is perhaps not entirely clear.
>
> Not in my eyes, it isn't.  E.g., look at any GitHub repository: there
> are "commits" there, and there are "releases", and they are not the
> same.

You're right, but I don't think I'm using them as synonyms. There is a
big difference between

    (package-vc-install "foo")

which installs the latest "commit" (or revision, which I'm actually
using as a synonym to commit) of a package, and

    (package-vc-install "foo" :last-release)

which installs the latest "release". A release, as defined by
package-vc.el seems to be 'the latest revision that bumps the "Version"
tag.' It is queried in the package-vc--release-rev function, and
actually retrieved in vc-retrieve-tag. The "Version" tag is, I think,
just the "Version: XXX" string that's specified in the top comment of
the main elisp file of the respective repository.

>> If :rev is not explicitly given, then :vc falls back to calling
>> package-vc-install (in the case of a non-local upstream) with the
>> :last-release keyword in place of its REV argument (which is called a
>> revision in the docs). Since package-vc.el freely calls :last-release a
>> release of a package, I figured this terminology is appropriate here. It
>> is only when :rev :newest is given that I talk about commits, which
>> should also be accurate. Or perhaps you mean that I mistakenly talk
>> about the latest release in some other place that I've overlooked just
>> now?
>
> All I know is that when I've read the documentation you wrote, I asked
> myself "what is meant by 'release' here?"  I found the answer when you
> later wrote "last commit".
>
> Are you talking about commits?  More generally, what kind of "release
> IDs" does :rev accept as its valid value?

The :rev keyword accepts the same as REV of package-vc-install, which is
either

  - nil, signaling that the latest commit should be installed,

  - :last-release, signaling that the last release should be installed,
    or

  - a "version string" appropriate for the respective version control
    system, specifying that version (e.g., a specific commit hash).

> I understand that the same confusion could exist elsewhere, but that
> doesn't mean we should proliferate it or even live with what we have.
> We should instead clarify this in every place where we use this
> terminology.
>
> So let's figure out what are these "releases", and then let's examine
> the existing and the new documentation and see if we need to get our
> terminology right there.

I totally agree, and I think the fact that "release" means "when has the
release version as specified in the main .el file changed" should be
documented somewhere (if it is I didn't see it). Sorry that this has
caused so much confusion.

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Tony Zorman <soliditsallgood@mailbox.org>
> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>  stefankangas@gmail.com
> Date: Thu, 04 May 2023 10:13:49 +0200
> 
> > Not in my eyes, it isn't.  E.g., look at any GitHub repository: there
> > are "commits" there, and there are "releases", and they are not the
> > same.
> 
> You're right, but I don't think I'm using them as synonyms. There is a
> big difference between
> 
>     (package-vc-install "foo")
> 
> which installs the latest "commit" (or revision, which I'm actually
> using as a synonym to commit) of a package, and
> 
>     (package-vc-install "foo" :last-release)
> 
> which installs the latest "release". A release, as defined by
> package-vc.el seems to be 'the latest revision that bumps the "Version"
> tag.' It is queried in the package-vc--release-rev function, and
> actually retrieved in vc-retrieve-tag. The "Version" tag is, I think,
> just the "Version: XXX" string that's specified in the top comment of
> the main elisp file of the respective repository.

OK, then this should be documented.  Specifically, it is important to
explain that the default is the latest _commit_ (a.k.a. HEAD) of the
default branch, whereas specific releases are those that bump the
Version: tag in the main ELisp file.

> > Are you talking about commits?  More generally, what kind of "release
> > IDs" does :rev accept as its valid value?
> 
> The :rev keyword accepts the same as REV of package-vc-install, which is
> either
> 
>   - nil, signaling that the latest commit should be installed,
> 
>   - :last-release, signaling that the last release should be installed,
>     or
> 
>   - a "version string" appropriate for the respective version control
>     system, specifying that version (e.g., a specific commit hash).

Any value other than a commit hash?  E.g., are tags supported?

And how will the user know which commit hash or tag to specify given
the value of a Version: tag in the main file?  We should document that
as well.

Thanks.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> I suppose Philip would be more qualified than me to answer this, but
> I'll try.

Sorry, I forgot the respond to this exchange.

> On Tue, May 02 2023 18:16, Eli Zaretskii wrote:
>>> From: Tony Zorman <soliditsallgood@mailbox.org>
>>> Cc: philipk@posteo.net, 60418@debbugs.gnu.org, felician.nemeth@gmail.com,
>>>  stefankangas@gmail.com
>>> Date: Tue, 02 May 2023 16:22:17 +0200
>>> 
>>> On Tue, May 02 2023 15:40, Eli Zaretskii wrote:
>>> >> +@code{package-vc-install}.  Notably -- even when not specified --
>>> >> +@code{:rev} defaults to checking out the last release of the package.
>>> >> +You can use @code{:rev :newest} to check out the latest commit.
>>> >> +
>>> >> +For example,
>>> >> +
>>> >> +@example
>>> >> +@group
>>> >> +(use-package bbdb
>>> >> +  :vc (:url "https://git.savannah.nongnu.org/git/bbdb.git"
>>> >> +       :rev :newest))
>>> >> +@end group
>>> >> +@end example
>>> >> +
>>> >> +would try -- by invoking @code{package-vc-install} -- to install the
>>> >> +latest commit of the package @code{foo} from the specified remote.
>>> >                                       ^^^
>>> > A typo there.
>>> >
>>> > Also, you say above "the latest release", but then "the latest
>>> > commit".  These two are not the same, and in fact I think talking
>>> > about "release" here is misleading, since you actually mean "commit".
>>> > For the same reason, I think the text should explain how to indicate a
>>> > commit that is not the latest one, because that is also not
>>> > self-evident, especially since the upstream VCS is not necessarily
>>> > Git.
>>> 
>>> I think the terminology of commit and release I use here are consistent,
>>> though maybe the wording is perhaps not entirely clear.
>>
>> Not in my eyes, it isn't.  E.g., look at any GitHub repository: there
>> are "commits" there, and there are "releases", and they are not the
>> same.
>
> You're right, but I don't think I'm using them as synonyms. There is a
> big difference between
>
>     (package-vc-install "foo")
>
> which installs the latest "commit" (or revision, which I'm actually
> using as a synonym to commit) of a package, and
>
>     (package-vc-install "foo" :last-release)
>
> which installs the latest "release". A release, as defined by
> package-vc.el seems to be 'the latest revision that bumps the "Version"
> tag.' It is queried in the package-vc--release-rev function, and
> actually retrieved in vc-retrieve-tag. The "Version" tag is, I think,
> just the "Version: XXX" string that's specified in the top comment of
> the main elisp file of the respective repository.

How about we just say "the commit of the latest release"?  That would
avoid confusing the reader into believing that we are fetching the code
via some "official" release-channel or anything like that, while still
indicating that this is not just any commit.

>>> If :rev is not explicitly given, then :vc falls back to calling
>>> package-vc-install (in the case of a non-local upstream) with the
>>> :last-release keyword in place of its REV argument (which is called a
>>> revision in the docs). Since package-vc.el freely calls :last-release a
>>> release of a package, I figured this terminology is appropriate here. It
>>> is only when :rev :newest is given that I talk about commits, which
>>> should also be accurate. Or perhaps you mean that I mistakenly talk
>>> about the latest release in some other place that I've overlooked just
>>> now?
>>
>> All I know is that when I've read the documentation you wrote, I asked
>> myself "what is meant by 'release' here?"  I found the answer when you
>> later wrote "last commit".
>>
>> Are you talking about commits?  More generally, what kind of "release
>> IDs" does :rev accept as its valid value?
>
> The :rev keyword accepts the same as REV of package-vc-install, which is
> either
>
>   - nil, signaling that the latest commit should be installed,
>
>   - :last-release, signaling that the last release should be installed,
>     or
>
>   - a "version string" appropriate for the respective version control
>     system, specifying that version (e.g., a specific commit hash).

All correct.

>> I understand that the same confusion could exist elsewhere, but that
>> doesn't mean we should proliferate it or even live with what we have.
>> We should instead clarify this in every place where we use this
>> terminology.
>>
>> So let's figure out what are these "releases", and then let's examine
>> the existing and the new documentation and see if we need to get our
>> terminology right there.
>
> I totally agree, and I think the fact that "release" means "when has the
> release version as specified in the main .el file changed" should be
> documented somewhere (if it is I didn't see it). Sorry that this has
> caused so much confusion.

Are there any other places where we can fix this confusion?

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  60418@debbugs.gnu.org,
>   felician.nemeth@gmail.com,  stefankangas@gmail.com
> Date: Fri, 05 May 2023 05:04:59 +0000
> 
> Tony Zorman <soliditsallgood@mailbox.org> writes:
> 
> > You're right, but I don't think I'm using them as synonyms. There is a
> > big difference between
> >
> >     (package-vc-install "foo")
> >
> > which installs the latest "commit" (or revision, which I'm actually
> > using as a synonym to commit) of a package, and
> >
> >     (package-vc-install "foo" :last-release)
> >
> > which installs the latest "release". A release, as defined by
> > package-vc.el seems to be 'the latest revision that bumps the "Version"
> > tag.' It is queried in the package-vc--release-rev function, and
> > actually retrieved in vc-retrieve-tag. The "Version" tag is, I think,
> > just the "Version: XXX" string that's specified in the top comment of
> > the main elisp file of the respective repository.
> 
> How about we just say "the commit of the latest release"?

When package-vc-install is used, what is "the latest release"? isn't
that the HEAD of the default branch?  IOW, what about packages that
make no releases at all?

> >> All I know is that when I've read the documentation you wrote, I asked
> >> myself "what is meant by 'release' here?"  I found the answer when you
> >> later wrote "last commit".
> >>
> >> Are you talking about commits?  More generally, what kind of "release
> >> IDs" does :rev accept as its valid value?
> >
> > The :rev keyword accepts the same as REV of package-vc-install, which is
> > either
> >
> >   - nil, signaling that the latest commit should be installed,
> >
> >   - :last-release, signaling that the last release should be installed,
> >     or
> >
> >   - a "version string" appropriate for the respective version control
> >     system, specifying that version (e.g., a specific commit hash).
> 
> All correct.
> 
> >> I understand that the same confusion could exist elsewhere, but that
> >> doesn't mean we should proliferate it or even live with what we have.
> >> We should instead clarify this in every place where we use this
> >> terminology.
> >>
> >> So let's figure out what are these "releases", and then let's examine
> >> the existing and the new documentation and see if we need to get our
> >> terminology right there.
> >
> > I totally agree, and I think the fact that "release" means "when has the
> > release version as specified in the main .el file changed" should be
> > documented somewhere (if it is I didn't see it). Sorry that this has
> > caused so much confusion.
> 
> Are there any other places where we can fix this confusion?

I guess the documentation of package-vc-install shares these issues?

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  60418@debbugs.gnu.org,
>>   felician.nemeth@gmail.com,  stefankangas@gmail.com
>> Date: Fri, 05 May 2023 05:04:59 +0000
>> 
>> Tony Zorman <soliditsallgood@mailbox.org> writes:
>> 
>> > You're right, but I don't think I'm using them as synonyms. There is a
>> > big difference between
>> >
>> >     (package-vc-install "foo")
>> >
>> > which installs the latest "commit" (or revision, which I'm actually
>> > using as a synonym to commit) of a package, and
>> >
>> >     (package-vc-install "foo" :last-release)
>> >
>> > which installs the latest "release". A release, as defined by
>> > package-vc.el seems to be 'the latest revision that bumps the "Version"
>> > tag.' It is queried in the package-vc--release-rev function, and
>> > actually retrieved in vc-retrieve-tag. The "Version" tag is, I think,
>> > just the "Version: XXX" string that's specified in the top comment of
>> > the main elisp file of the respective repository.
>> 
>> How about we just say "the commit of the latest release"?
>
> When package-vc-install is used, what is "the latest release"? isn't
> that the HEAD of the default branch?  IOW, what about packages that
> make no releases at all?

No, the commit of the latest release is interpreted the same way as
elpa-admin.el does, namely the last revision that modified the "Version"
header.  If no such commit can be found, then a message is printed out
and the installation continues under the assumption that the package is
using a rolling-release model.

>> >> All I know is that when I've read the documentation you wrote, I asked
>> >> myself "what is meant by 'release' here?"  I found the answer when you
>> >> later wrote "last commit".
>> >>
>> >> Are you talking about commits?  More generally, what kind of "release
>> >> IDs" does :rev accept as its valid value?
>> >
>> > The :rev keyword accepts the same as REV of package-vc-install, which is
>> > either
>> >
>> >   - nil, signaling that the latest commit should be installed,
>> >
>> >   - :last-release, signaling that the last release should be installed,
>> >     or
>> >
>> >   - a "version string" appropriate for the respective version control
>> >     system, specifying that version (e.g., a specific commit hash).
>> 
>> All correct.
>> 
>> >> I understand that the same confusion could exist elsewhere, but that
>> >> doesn't mean we should proliferate it or even live with what we have.
>> >> We should instead clarify this in every place where we use this
>> >> terminology.
>> >>
>> >> So let's figure out what are these "releases", and then let's examine
>> >> the existing and the new documentation and see if we need to get our
>> >> terminology right there.
>> >
>> > I totally agree, and I think the fact that "release" means "when has the
>> > release version as specified in the main .el file changed" should be
>> > documented somewhere (if it is I didn't see it). Sorry that this has
>> > caused so much confusion.
>> 
>> Are there any other places where we can fix this confusion?
>
> I guess the documentation of package-vc-install shares these issues?

How does this sound like to you:


[-- Attachment #2: Type: text/plain, Size: 1269 bytes --]

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 8f62e7d65f3..b28e33b3b89 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -747,11 +747,14 @@ package-vc-install
 symbol whose name is the package name, and the URL for the
 package will be taken from the package's metadata.
 
-By default, this function installs the last version of the package
-available from its repository, but if REV is given and non-nil, it
-specifies the revision to install.  If REV has the special value
-`:last-release' (interactively, the prefix argument), that stands
-for the last released version of the package.
+By default, this function installs the last revision of the
+package available from its repository, but if REV is given and
+non-nil, it specifies the revision to install.  If REV has the
+special value `:last-release' (interactively, the prefix
+argument), an attempt is made to find the revision of the latest
+release.  This is done by looking up the last revision that
+modified the \"Version\" header, as described in the Info
+node `(elisp) Library Headers'.
 
 Optional argument BACKEND specifies the VC backend to use for cloning
 the package's repository; this is only possible if NAME-OR-URL is a URL,

[-- Attachment #3: Type: text/plain, Size: 56 bytes --]


I can apply this or any variation thereof to emacs-29.

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>   felician.nemeth@gmail.com,  stefankangas@gmail.com
> Date: Fri, 05 May 2023 05:49:26 +0000
> 
> >> How about we just say "the commit of the latest release"?
> >
> > When package-vc-install is used, what is "the latest release"? isn't
> > that the HEAD of the default branch?  IOW, what about packages that
> > make no releases at all?
> 
> No, the commit of the latest release is interpreted the same way as
> elpa-admin.el does, namely the last revision that modified the "Version"
> header.  If no such commit can be found, then a message is printed out
> and the installation continues under the assumption that the package is
> using a rolling-release model.

I thought package-vc-install is used (or at least can be used) to
fetch the latest HEAD from the upstream repository?  I even thought
this was its main raison d'être?

If that's not true, does it mean we have no means for package users to
track the latest development code of a package?

> >> Are there any other places where we can fix this confusion?
> >
> > I guess the documentation of package-vc-install shares these issues?
> 
> How does this sound like to you:
> 
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index 8f62e7d65f3..b28e33b3b89 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -747,11 +747,14 @@ package-vc-install
>  symbol whose name is the package name, and the URL for the
>  package will be taken from the package's metadata.
>  
> -By default, this function installs the last version of the package
> -available from its repository, but if REV is given and non-nil, it
> -specifies the revision to install.  If REV has the special value
> -`:last-release' (interactively, the prefix argument), that stands
> -for the last released version of the package.
> +By default, this function installs the last revision of the
> +package available from its repository, but if REV is given and
> +non-nil, it specifies the revision to install.  If REV has the
> +special value `:last-release' (interactively, the prefix
> +argument), an attempt is made to find the revision of the latest
> +release.  This is done by looking up the last revision that
> +modified the \"Version\" header, as described in the Info
> +node `(elisp) Library Headers'.

First, too much of passive voice.
More importantly, this still doesn't tell:

  . what is "the last revision", the one installed if REV is omitted?
  . what are possible values of REV, in addition to :last-release, and
    how are those values interpreted, in VCS terms?

> I can apply this or any variation thereof to emacs-29.

emacs-29 is fine for documentation changes.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>>   felician.nemeth@gmail.com,  stefankangas@gmail.com
>> Date: Fri, 05 May 2023 05:49:26 +0000
>> 
>> >> How about we just say "the commit of the latest release"?
>> >
>> > When package-vc-install is used, what is "the latest release"? isn't
>> > that the HEAD of the default branch?  IOW, what about packages that
>> > make no releases at all?
>> 
>> No, the commit of the latest release is interpreted the same way as
>> elpa-admin.el does, namely the last revision that modified the "Version"
>> header.  If no such commit can be found, then a message is printed out
>> and the installation continues under the assumption that the package is
>> using a rolling-release model.
>
> I thought package-vc-install is used (or at least can be used) to
> fetch the latest HEAD from the upstream repository?  I even thought
> this was its main raison d'être?
>
> If that's not true, does it mean we have no means for package users to
> track the latest development code of a package?

This is true, for package-vc-install, but the idea was not do this for
the :vc keyword to use-package.  My understanding is that the main
interest here is to install packages that are not available via package
archives.  And as use-package is a popular means of bootstrapping a
configuration, it seems the right approach to use the commit of the
latest revision, instead of just any commit to avoid instability.

>> >> Are there any other places where we can fix this confusion?
>> >
>> > I guess the documentation of package-vc-install shares these issues?
>> 
>> How does this sound like to you:
>> 
>> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
>> index 8f62e7d65f3..b28e33b3b89 100644
>> --- a/lisp/emacs-lisp/package-vc.el
>> +++ b/lisp/emacs-lisp/package-vc.el
>> @@ -747,11 +747,14 @@ package-vc-install
>>  symbol whose name is the package name, and the URL for the
>>  package will be taken from the package's metadata.
>>  
>> -By default, this function installs the last version of the package
>> -available from its repository, but if REV is given and non-nil, it
>> -specifies the revision to install.  If REV has the special value
>> -`:last-release' (interactively, the prefix argument), that stands
>> -for the last released version of the package.
>> +By default, this function installs the last revision of the
>> +package available from its repository, but if REV is given and
>> +non-nil, it specifies the revision to install.  If REV has the
>> +special value `:last-release' (interactively, the prefix
>> +argument), an attempt is made to find the revision of the latest
>> +release.  This is done by looking up the last revision that
>> +modified the \"Version\" header, as described in the Info
>> +node `(elisp) Library Headers'.
>
> First, too much of passive voice.
> More importantly, this still doesn't tell:
>
>   . what is "the last revision", the one installed if REV is omitted?
>   . what are possible values of REV, in addition to :last-release, and
>     how are those values interpreted, in VCS terms?

I have tried to address these issues here:

   By default, this function installs the last revision of the
   package available from its repository.  If REV is a string, it
   describes the revision to install, as interpreted by the VC
   backend.  The special value `:last-release' (interactively, the
   prefix argument), will use the commit of the latest release, if
   it exists.  The latest revision is determined by the latest
   revision to modify the \"Version\" header of the main file.

But I couldn't come up with an elegant way to avoid the passive voice in
the last sentence.

>> I can apply this or any variation thereof to emacs-29.
>
> emacs-29 is fine for documentation changes.

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>   felician.nemeth@gmail.com,  stefankangas@gmail.com
> Date: Fri, 05 May 2023 17:15:42 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> No, the commit of the latest release is interpreted the same way as
> >> elpa-admin.el does, namely the last revision that modified the "Version"
> >> header.  If no such commit can be found, then a message is printed out
> >> and the installation continues under the assumption that the package is
> >> using a rolling-release model.
> >
> > I thought package-vc-install is used (or at least can be used) to
> > fetch the latest HEAD from the upstream repository?  I even thought
> > this was its main raison d'être?
> >
> > If that's not true, does it mean we have no means for package users to
> > track the latest development code of a package?
> 
> This is true, for package-vc-install, but the idea was not do this for
> the :vc keyword to use-package.

But I wrote the above as a comment to a patch to package-vc.el, not to
use-package.  So why use-package is relevant here?

> My understanding is that the main
> interest here is to install packages that are not available via package
> archives.  And as use-package is a popular means of bootstrapping a
> configuration, it seems the right approach to use the commit of the
> latest revision, instead of just any commit to avoid instability.

My understanding is that the :rev keyword allows to use any value that
is acceptable to package-vc-install.  I understand that in most cases
users will want to install the latest, but once we decided to support
:rev, we must allow any valid value there.  Right?

>    By default, this function installs the last revision of the
>    package available from its repository.  If REV is a string, it
>    describes the revision to install, as interpreted by the VC
>    backend.  The special value `:last-release' (interactively, the
>    prefix argument), will use the commit of the latest release, if
>    it exists.  The latest revision is determined by the latest
                     ^^^^^^^^^^^^^^^
"last release", not "latest revision": you are explaining what the
previous sentence said.

>    revision to modify the \"Version\" header of the main file.
> 
> But I couldn't come up with an elegant way to avoid the passive voice in
> the last sentence.

Here's one way:

  The last release is the latest revision which changed the
  \"Version:\" header of the package's main Lisp file.

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>>   felician.nemeth@gmail.com,  stefankangas@gmail.com
>> Date: Fri, 05 May 2023 17:15:42 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> No, the commit of the latest release is interpreted the same way as
>> >> elpa-admin.el does, namely the last revision that modified the "Version"
>> >> header.  If no such commit can be found, then a message is printed out
>> >> and the installation continues under the assumption that the package is
>> >> using a rolling-release model.
>> >
>> > I thought package-vc-install is used (or at least can be used) to
>> > fetch the latest HEAD from the upstream repository?  I even thought
>> > this was its main raison d'être?
>> >
>> > If that's not true, does it mean we have no means for package users to
>> > track the latest development code of a package?
>> 
>> This is true, for package-vc-install, but the idea was not do this for
>> the :vc keyword to use-package.
>
> But I wrote the above as a comment to a patch to package-vc.el, not to
> use-package.  So why use-package is relevant here?

Because this patch is related to use-package, and a keyword that would
allow for use-package to invoke package-vc-install?

>> My understanding is that the main
>> interest here is to install packages that are not available via package
>> archives.  And as use-package is a popular means of bootstrapping a
>> configuration, it seems the right approach to use the commit of the
>> latest revision, instead of just any commit to avoid instability.
>
> My understanding is that the :rev keyword allows to use any value that
> is acceptable to package-vc-install.  

Right, and what is acceptable to package-vc-install is what is
transitively acceptable to `vc-clone'/`vc-retrieve-tag'.

>                                       I understand that in most cases
> users will want to install the latest, 

I don't know if that is the case.  I might be wrong that the revision of
the latest release is a good default?  Tony, do you think we should add
a user-option to regulate this behaviour.

>                                        but once we decided to support
> :rev, we must allow any valid value there.  Right?

Yes.

>>    By default, this function installs the last revision of the
>>    package available from its repository.  If REV is a string, it
>>    describes the revision to install, as interpreted by the VC
>>    backend.  The special value `:last-release' (interactively, the
>>    prefix argument), will use the commit of the latest release, if
>>    it exists.  The latest revision is determined by the latest
>                      ^^^^^^^^^^^^^^^
> "last release", not "latest revision": you are explaining what the
> previous sentence said.

Whoops, of course.

>>    revision to modify the \"Version\" header of the main file.
>> 
>> But I couldn't come up with an elegant way to avoid the passive voice in
>> the last sentence.
>
> Here's one way:
>
>   The last release is the latest revision which changed the
>   \"Version:\" header of the package's main Lisp file.

Yes, sounds good.  Will apply this change.

bug#60418: [PATCH] Add :vc keyword to use-package

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>   felician.nemeth@gmail.com,  stefankangas@gmail.com
> Date: Sat, 06 May 2023 18:50:02 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> This is true, for package-vc-install, but the idea was not do this for
> >> the :vc keyword to use-package.
> >
> > But I wrote the above as a comment to a patch to package-vc.el, not to
> > use-package.  So why use-package is relevant here?
> 
> Because this patch is related to use-package, and a keyword that would
> allow for use-package to invoke package-vc-install?

Yes, but the doc string in package-vc-install should describe
everything that package-vc supports, not just the subset we think will
be useful in use-package via :rev.

Anyway, I think we are in agreement, as your last doc string is
comprehensive enough to make me happy.

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Sat, May 06 2023 18:50, Philip Kaludercic wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>>                                       I understand that in most cases
>> users will want to install the latest, 
>
> I don't know if that is the case.  I might be wrong that the revision of
> the latest release is a good default?  Tony, do you think we should add
> a user-option to regulate this behaviour.

It's certainly my default to install the latest commit of a package
instead of the latest release. I don't know how general this desire is
among other people, but an option for controlling the specific behaviour
sounds great in either case.

-- 
Tony Zorman | https://tony-zorman.com/

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: soliditsallgood@mailbox.org,  60418@debbugs.gnu.org,
>>   felician.nemeth@gmail.com,  stefankangas@gmail.com
>> Date: Sat, 06 May 2023 18:50:02 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> This is true, for package-vc-install, but the idea was not do this for
>> >> the :vc keyword to use-package.
>> >
>> > But I wrote the above as a comment to a patch to package-vc.el, not to
>> > use-package.  So why use-package is relevant here?
>> 
>> Because this patch is related to use-package, and a keyword that would
>> allow for use-package to invoke package-vc-install?
>
> Yes, but the doc string in package-vc-install should describe
> everything that package-vc supports, not just the subset we think will
> be useful in use-package via :rev.

In that case there was some other unrelated confusion.

> Anyway, I think we are in agreement, as your last doc string is
> comprehensive enough to make me happy.

OK, great!  Thank you for your input.

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> On Sat, May 06 2023 18:50, Philip Kaludercic wrote:
>> Eli Zaretskii <eliz@gnu.org> writes:
>>>                                       I understand that in most cases
>>> users will want to install the latest, 
>>
>> I don't know if that is the case.  I might be wrong that the revision of
>> the latest release is a good default?  Tony, do you think we should add
>> a user-option to regulate this behaviour.
>
> It's certainly my default to install the latest commit of a package
> instead of the latest release. I don't know how general this desire is
> among other people, but an option for controlling the specific behaviour
> sounds great in either case.

OK, so it might make sense to address this at a later point with a
another patch.

For now, what issues remain to be resolved before these patches can be
applied?

-- 
Philip Kaludercic

bug#60418: [PATCH] Add :vc keyword to use-package

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

> Tony Zorman <soliditsallgood@mailbox.org> writes:
>
>> On Sat, May 06 2023 18:50, Philip Kaludercic wrote:
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>                                       I understand that in most cases
>>>> users will want to install the latest, 
>>>
>>> I don't know if that is the case.  I might be wrong that the revision of
>>> the latest release is a good default?  Tony, do you think we should add
>>> a user-option to regulate this behaviour.
>>
>> It's certainly my default to install the latest commit of a package
>> instead of the latest release. I don't know how general this desire is
>> among other people, but an option for controlling the specific behaviour
>> sounds great in either case.
>
> OK, so it might make sense to address this at a later point with a
> another patch.
>
> For now, what issues remain to be resolved before these patches can be
> applied?

Reviewing the thread, I think everything was ready (aside from a few
things that I promised to take care of), so I've pushed the change to
master.  Thank you for your contribution!

bug#60418: [PATCH] Add :vc keyword to use-package

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Tue, May 16 2023 19:30, Philip Kaludercic wrote:
> Reviewing the thread, I think everything was ready (aside from a few
> things that I promised to take care of), so I've pushed the change to
> master.  Thank you for your contribution!

Fantastic; thanks to you and Eli for guiding this along!

-- 
Tony Zorman | https://tony-zorman.com/