Re: feature/package+vc 04c4c578c7 3/4: Allow for packages to be installed directly from VCS

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> I'm thinking of things like the code that handles `:doc` or `:make`.
>>> These don't care about Git.
>> For something like :make to work, we would also require :renames, right?
>
> No, it's done before renames.
>
>> But if that is added, then the version control could break.
>
> In the part of `elpa-admin.el` designed for "install from Git"
> (i.e. the part that matches package+vc's goal), `:rename` is not taken
> into account.  I don't think we can reasonably do so, as you point out,
> so I think `:renames` should be labeled as something we can't support
> (and eventually deprecated for that reason).

OK, so I won't add support for that right now.

>> How does this look like:
>
> I'm a bit confused: it seems to do for `:core` what I expected to happen
> for packages with a nil `:url` (and it doesn't drop `:core` packages,
> like I'd expect).

That was my mistake, I misremembered that core packages aren't mirrored
in their own branches in elpa.git.  I've removed it here:

>From fe8530cde40d921b96ec29513d16d91114468e75 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Fri, 21 Oct 2022 23:18:30 +0200
Subject: [PATCH] Generate elpa-packages.eld with package data

* elpa-admin.el
(elpaa-publish-package-specs): Process specs and write file.
(elpaa-batch-make-all-packages): Call 'elpaa-publish-package-specs'.
---
 elpa-admin.el | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/elpa-admin.el b/elpa-admin.el
index 054c9bda86..00c3393499 100644
--- a/elpa-admin.el
+++ b/elpa-admin.el
@@ -762,13 +762,36 @@ of the current `process-environment'.  Return the 
modified copy."
          (list pkgname))
       spec)))
 
+(defun elpaa-publish-package-specs (specs)
+  "Process and publish SPECS in elpa-packages.eld files."
+  (with-temp-buffer
+    ;; Remove :core packages, handle :url nil and and compress the
+    ;; contents of the "elpa-packages"
+    (prin1
+     (mapcan
+      (lambda (spec)
+        (cond
+         ((and (plist-member (cdr spec) :url)
+               (null (plist-get (cdr spec) :url)))
+          `((,(car spec)
+             :url ,(concat "https://git.sv.gnu.org/git/"; elpaa--gitrepo)
+             :branch ,(concat elpaa--branch-prefix (car spec))
+             ,@(cdddr spec))))
+         ((plist-member (cdr spec) :core) nil)
+         ((list spec))))
+      specs)
+     (current-buffer))
+    (write-region nil nil (expand-file-name "elpa-packages.eld" 
elpaa--release-subdir))
+    (write-region nil nil (expand-file-name "elpa-packages.eld" 
elpaa--devel-subdir))))
+
 (defun elpaa-batch-make-all-packages (&rest _)
   "Check all the packages and build the relevant new tarballs."
-  (let* ((specs (elpaa--get-specs)))
+  (let ((specs (elpaa--get-specs)))
     (dolist (spec specs)
       (condition-case err
           (elpaa--make-one-package spec)
-        (error (message "Build error for %s: %S" (car spec) err))))))
+        (error (message "Build error for %s: %S" (car spec) err))))
+    (elpaa-publish-package-specs specs)))
 
 (defun elpaa-batch-make-one-package (&rest _)
   "Build the new tarballs (if needed) for one particular package."
-- 
2.38.0

> Other than that, it looks OK (in the nitpick area, I'd move the code to
> its own function and then call it from `elpaa-batch-make-all-packages`).

Done.

> I like the `.eld` extension, and I guess "elpa-packages.eld" is as good
> a name as any.
>
>> It currently makes the assumption that :core is the first element in the
>> property list for each package specification.
>
> That's right: that element should be either `:core` or `:url`.

Great.