bug#63916: 30.0.50; use-package: changes do not propagate to elpa-devel

[Philip Kaludercic]

To: Benjamin Orthen <benjamin@orthen.net>
Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  63916@debbugs.gnu.org
Subject: Re: bug#63916: 30.0.50; use-package: changes do not propagate to 
elpa-devel
From: Philip Kaludercic <philipk@posteo.net>
Gcc: nnimap+posteo.de:Sent
--text follows this line--
Benjamin Orthen <benjamin@orthen.net> writes:

> Related to this,
>
> I have a patch for elpa-admin.el (in the elpa repository) which would
> improve devel versioning for core packages:
>
> Instead of looking only at the git log of the main package file, we
> look instead at the git log of all files of the core package.
> This way, a new devel version is created when any other package file is
> changed, not just the main file.

This seems to make sense.

> Is this the right mailing list to send the patch? If not, where could I
> send it to?

This is fine, the main thing is that someone has a patch that can be
applied.

>
> Best,
> Benjamin
>
> On Sun, 2023-06-11 at 19:32 +0200, Benjamin Orthen wrote:
>> Hi,
>> 
>> I think I figured out why elpa does not build a new version of use-
>> package.
>> 
>> To determine the devel version, it looks only at the git log of
>> `(elpa-
>> -main-file pkg-spec)`, which in this case turns out to be use-
>> package.el. However, use-package.el has not been changed since
>> 20230115, I suppose this is why no new version is built.
>> 
>> Best, Benjamin
>> 
>> 
>> On Sun, 2023-06-11 at 12:55 -0400, Stefan Monnier wrote:
>> > > > I agree that these kinds of problems are annoying to debug.  It
>> > > > would be
>> > > > nice to offer a "debug mode" where the user can see what's
>> > > > going
>> > > > on
>> > > > within the sandbox.  Maybe we could do that by opening an
>> > > > interactive
>> > > > shell with a message saying "this is the command that we want
>> > > > to
>> > > > run"
>> > > > and then let the user run that command?
>> > > Shouldn't just invoking bash do that?
>> > 
>> > Yes, it should be fairly easy to do.
>> > 
>> > > But returning to the initial issue (hoping I did not miss
>> > > anything), if
>> > > the issue is not what I mentioned, then why is use-package not
>> > > building?
>> > 
>> > You did not miss anything: it's not building simply because the
>> > process
>> > run within the sandbox cannot read the target of the `use-
>> > package.texi`
>> > symlink because the sandbox does not expose this target.
>> > 
>> > 
>> >         Stefan
>
> From d29163e00313690435b2baacc770a734598dd956 Mon Sep 17 00:00:00 2001
> From: Benjamin Orthen <git@orthen.net>
> Date: Fri, 8 Sep 2023 12:05:14 +0200
> Subject: [PATCH] Add elpa--core-files to get more exact devel-versions for
>  core packages
>
> ---
>  elpa-admin.el | 51 ++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/elpa-admin.el b/elpa-admin.el
> index 2c2d2aeab7..57dfee8f18 100644
> --- a/elpa-admin.el
> +++ b/elpa-admin.el
> @@ -908,20 +908,49 @@ of the current `process-environment'.  Return the 
> modified copy."
>                 0)))
>      (encode-time (list s mi h d mo y nil nil zs))))
>  
> +(defun elpaa--core-files (pkg-spec)
> +  "Get a list of core files (and only files) for PKG-SPEC.
> +Core folders are recursively searched, excluded files are ignored."
> +  (when-let
> +      ((core (elpaa--spec-get pkg-spec :core)))

There is no need to fold this.

> +    (let*

Or this.

> +        ((excludes (elpaa--spec-get pkg-spec :excludes))
> +         (emacs-repo-root (expand-file-name "emacs"))
> +         (default-directory emacs-repo-root)
> +         (file-patterns
> +          (if (listp core)
> +              core
> +            (list core)))
> +         (core-files nil))

You can use list-ensure here.

> +      ;; we look at each file or files in folder and add them
> +      ;; to core-files if they are in the excludes
> +      (cl-labels ((process-item (item)
> +                    (unless (member item excludes)
> +                      (if (file-directory-p item)
> +                          (dolist (file (directory-files item nil 
> directory-files-no-dot-files-regexp))
> +                            (process-item (concat item file)))
> +                        (push item core-files)))))
> +        (dolist (item file-patterns)
> +          (process-item item)))

Couldn't `directory-files-recursively' be useful here?

> +      core-files)))
> +
>  (defun elpaa--get-devel-version (dir pkg-spec)
>    "Compute the date-based pseudo-version used for devel builds."
> -  (let* ((ftn (file-truename      ;; Follow symlinks!
> -              (expand-file-name (elpaa--main-file pkg-spec) dir)))
> -         (default-directory (file-name-directory ftn))
> -         (gitdate
> +  (let* ((gitdate
>            (with-temp-buffer
> -           (if (plist-get (cdr pkg-spec) :core)
> -               ;; For core packages, don't use the date of the last
> -               ;; commit to the branch, but that of the last commit
> -               ;; to the main file.
> -               (elpaa--call t "git" "log" "--pretty=format:%cI" "--no-patch"
> -                            "-1" "--" (file-name-nondirectory ftn))
> -             (elpaa--call t "git" "show" "--pretty=format:%cI" "--no-patch"))
> +            (if (plist-get (cdr pkg-spec) :core)
> +                (let
> +                    ((core-files (elpaa--core-files pkg-spec))
> +                     (default-directory (expand-file-name "emacs")))
> +                  ;; For core packages, don't use the date of the last
> +                  ;; commit to the branch, but that of the last commit
> +                  ;; to the core files.
> +                  (apply 'elpaa--call t "git" "log" "--pretty=format:%cI" 
> "--no-patch"

While we are at it, you might as well sharp-quote the `elpaa-call' here.

> +                         "-1" "--" core-files))
> +              (let* ((ftn (file-truename      ;; Follow symlinks!
> +                           (expand-file-name (elpaa--main-file pkg-spec) 
> dir)))
> +                     (default-directory (file-name-directory ftn)))
> +                (elpaa--call t "git" "show" "--pretty=format:%cI" 
> "--no-patch")))
>              (buffer-string)))
>           (verdate
>            ;; Convert Git's date into something that looks like a version 
> number.