bug#64985: 29.1; [PATCH] Byte-compilation of packages via package-vc doesn't correctly handle .elpaignore wildcards

[Philip Kaludercic]

Jim Porter <jporterbugs@gmail.com> writes:

> When using the various package-vc installation functions, Emacs
> byte-compiles the source (good). However, it doesn't ignore sources
> that match wildcards in ".elpaignore" (bad). That's because, even
> though 'byte-compile-ignore-files' is documented to be a list of
> regexps, 'byte-recompile-directory' treats it as a list of strings.

I am a bit confused about this point.  Why do you think that
`byte-recompile-directory' treads `byte-compile-ignore-files' as a list
of non-regexp strings?

> To reproduce this, install a package via VC, for example via M-x
> package-vc-install RET https://github.com/jimporter/urgrep.git
> RET. Then notice how "urgrep-tests.el" got compiled, even though it
> matches a wildcard in ".elpaignore".
>
> Patch attached to fix this. Eli, would this be safe enough for 29.2?
> 'byte-compile-ignore-files' is a new feature in 29.1, so hopefully no
> one else is relying on its current semantics just yet.
>
> From ec4e198fb9c87653d1e65288969bed1ff548683c Mon Sep 17 00:00:00 2001
> From: Jim Porter <jporterbugs@gmail.com>
> Date: Mon, 31 Jul 2023 23:10:03 -0700
> Subject: [PATCH] Fix handling of ".elpaignore" file when compiling packages
>
> * lisp/emacs-lisp/bytecomp.el (byte-recompile-directory): Treat
> 'byte-compile-ignore-files' as a list of regexps per its docstring.
> ---
>  lisp/emacs-lisp/bytecomp.el | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index 5b1d958e6c2..d123e68a088 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -1995,9 +1995,8 @@ byte-recompile-directory
>                     (or (null arg) (eq 0 arg)
>                         (y-or-n-p (concat "Check " source "? ")))
>                        ;; Directory is requested to be ignored
> -                      (not (string-match-p
> -                            (regexp-opt byte-compile-ignore-files)
> -                            source))
> +                      (not (seq-some (lambda (ex) (string-match-p ex source))
> +                                     byte-compile-ignore-files))

Here also, instead of merging a list of regular expressions into a
disjunctive one, you match each individually.  Am I missing something,
or what does this change?

(string-match-p (regexp-opt '("foo" "bar" "baz")) "foo") ;=> 0
(seq-some (lambda (ex) (string-match-p ex "foo")) '("foo" "bar" "baz")) ;=> 0

(string-match-p (regexp-opt '("foo" "bar" "baz")) "quux") ;=> nil
(seq-some (lambda (ex) (string-match-p ex "quux")) '("foo" "bar" "baz")) ;=> nil

>                        (setq directories (nconc directories (list source))))
>                 ;; It is an ordinary file.  Decide whether to compile it.
>                 (if (and (string-match emacs-lisp-file-regexp source)
> @@ -2007,9 +2006,8 @@ byte-recompile-directory
>                          (not (auto-save-file-name-p source))
>                          (not (member source (dir-locals--all-files 
> directory)))
>                          ;; File is requested to be ignored
> -                        (not (string-match-p
> -                              (regexp-opt byte-compile-ignore-files)
> -                              source)))
> +                        (not (seq-some (lambda (ex) (string-match-p ex 
> source))
> +                                       byte-compile-ignore-files)))
>                     (progn (cl-incf
>                             (pcase (byte-recompile-file source force arg)
>                               ('no-byte-compile skip-count)