GNU bug report logs - #65666
Lockfiles break package-vc-install-from-checkout

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65666 in the body.
You can then email your comments to 65666 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Emacs Bugs Mailing List <bug-gnu-emacs <at> gnu.org>
Subject: Lockfiles break package-vc-install-from-checkout
Date: Thu, 31 Aug 2023 14:47:48 -0700

To reproduce: clone a repo, ensure that create-lockfiles is non-nil,
edit a source file in the repo but do not save it, run
package-vc-install-from-checkout on the repo.

Backtrace:

Debugger entered--Lisp error: (file-missing "/home/joseph/.emacs.d/elpa/hyperdrive/.#hyperdrive.el")
  comp-el-to-eln-filename("/home/joseph/.emacs.d/elpa/hyperdrive/.#hyperdrive.el")
  (comp-clean-up-stale-eln (comp-el-to-eln-filename file))
  (while (consp --cl-var--) (setq file (car --cl-var--)) (comp-clean-up-stale-eln (comp-el-to-eln-filename file)) (setq --cl-var-- (cdr --cl-var--)))
  (let* ((--cl-var-- (directory-files-recursively dir "\\.el\\'")) (file nil)) (while (consp --cl-var--) (setq file (car --cl-var--)) (comp-clean-up-stale-eln (comp-el-to-eln-filename file)) (setq --cl-var-- (cdr --cl-var--))) nil)
  (progn (let* ((--cl-var-- (directory-files-recursively dir "\\.el\\'")) (file nil)) (while (consp --cl-var--) (setq file (car --cl-var--)) (comp-clean-up-stale-eln (comp-el-to-eln-filename file)) (setq --cl-var-- (cdr --cl-var--))) nil))
  (if (featurep 'native-compile) (progn (let* ((--cl-var-- (directory-files-recursively dir "\\.el\\'")) (file nil)) (while (consp --cl-var--) (setq file (car --cl-var--)) (comp-clean-up-stale-eln (comp-el-to-eln-filename file)) (setq --cl-var-- (cdr --cl-var--))) nil)))
  package--delete-directory("/home/joseph/.emacs.d/elpa/hyperdrive")
  package-vc-install-from-checkout("~/.local/src/hyperdrive.el/" "hyperdrive")
  funcall-interactively(package-vc-install-from-checkout "~/.local/src/hyperdrive.el/" "hyperdrive")
  command-execute(package-vc-install-from-checkout record)
  execute-extended-command(nil "package-vc-install-from-checkout" nil)
  funcall-interactively(execute-extended-command nil "package-vc-install-from-checkout" nil)
  command-execute(execute-extended-command)

Message #8 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam.porter <at> 47ap.net, philipk <at> posteo.net, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Fri, 01 Sep 2023 08:50:07 +0300

> Cc: Philip Kaludercic <philipk <at> posteo.net>, Adam Porter <adam.porter <at> 47ap.net>
> Date: Thu, 31 Aug 2023 14:47:48 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> To reproduce: clone a repo, ensure that create-lockfiles is non-nil,
> edit a source file in the repo but do not save it, run
> package-vc-install-from-checkout on the repo.
> 
> Backtrace:
> 
> Debugger entered--Lisp error: (file-missing "/home/joseph/.emacs.d/elpa/hyperdrive/.#hyperdrive.el")
>   comp-el-to-eln-filename("/home/joseph/.emacs.d/elpa/hyperdrive/.#hyperdrive.el")

We should not try to native-compile lock files, obviously.  Some code
naïvely uses "*.el" to find all the Lisp files; it should filter out
lock files.

P.S. Please always say in what version of Emacs you see the problem
you report.  Bonus points for including all of the information
collected by report-emacs-bug about your system and Emacs
configurations.

Message #11 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: joseph <at> breatheoutbreathe.in
To: "Eli Zaretskii" <eliz <at> gnu.org>
Cc: adam.porter <at> 47ap.net, philipk <at> posteo.net, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Fri, 01 Sep 2023 06:18:39 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
> We should not try to native-compile lock files, obviously.  Some code
> naïvely uses "*.el" to find all the Lisp files; it should filter out
> lock files.

Please see attached patch.

> P.S. Please always say in what version of Emacs you see the problem
> you report.  Bonus points for including all of the information
> collected by report-emacs-bug about your system and Emacs
> configurations.

Thanks - I'm on 29.0.92. I'll include that information next time.

[0001-Don-t-native-compile-lock-files.patch (text/x-diff, attachment)]

Message #14 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: joseph <at> breatheoutbreathe.in
Cc: adam.porter <at> 47ap.net, Eli Zaretskii <eliz <at> gnu.org>, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Fri, 01 Sep 2023 12:52:24 +0000

joseph <at> breatheoutbreathe.in writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>> We should not try to native-compile lock files, obviously.  Some code
>> naïvely uses "*.el" to find all the Lisp files; it should filter out
>> lock files.
>
> Please see attached patch.
>
>> P.S. Please always say in what version of Emacs you see the problem
>> you report.  Bonus points for including all of the information
>> collected by report-emacs-bug about your system and Emacs
>> configurations.
>
> Thanks - I'm on 29.0.92. I'll include that information next time.
>
> From 7b38b0bfd8c9da08daf734f6d0062d31dd54b947 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Thu, 31 Aug 2023 23:11:53 -0700
> Subject: [PATCH] Don't native compile lock files
>
> * lisp/emacs-lisp/package.el (package--delete-directory):
> Check that each file exists before compiling.
> ---
>  lisp/emacs-lisp/package.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index e1172d69bf0..52a538e0627 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -2485,6 +2485,7 @@ compiled."
>    (when (featurep 'native-compile)
>      (cl-loop
>       for file in (directory-files-recursively dir "\\.el\\'")
> +     when (file-exists-p file)
>       do (comp-clean-up-stale-eln (comp-el-to-eln-filename file))))
>    (if (file-symlink-p (directory-file-name dir))
>        (delete-file (directory-file-name dir))

LGTM, but I wonder if there is a better way to detect lockfiles
specifically?  If not, I can imagine that just using `file-exists-p'
would a too broad check, in the sense that it could make it difficult to
find other issues?

Message #17 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: adam.porter <at> 47ap.net, Eli Zaretskii <eliz <at> gnu.org>, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Fri, 01 Sep 2023 16:43:27 -0700

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

> LGTM, but I wonder if there is a better way to detect lockfiles
> specifically?  If not, I can imagine that just using `file-exists-p'
> would a too broad check, in the sense that it could make it difficult to
> find other issues?

We could use a regex like

(unless (string-match-p (rx string-start ".#") file))

Message #20 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam.porter <at> 47ap.net, philipk <at> posteo.net, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Sat, 02 Sep 2023 10:49:38 +0300

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 65666 <at> debbugs.gnu.org, adam.porter <at> 47ap.net
> Date: Fri, 01 Sep 2023 16:43:27 -0700
> 
> 
> Philip Kaludercic <philipk <at> posteo.net> writes:
> 
> > LGTM, but I wonder if there is a better way to detect lockfiles
> > specifically?  If not, I can imagine that just using `file-exists-p'
> > would a too broad check, in the sense that it could make it difficult to
> > find other issues?
> 
> We could use a regex like
> 
> (unless (string-match-p (rx string-start ".#") file))

Or make the REGEXP argument to directory-files-recursively more
specific, to reject lock files.

But yes, the use of file-exists-p is not TRT, IMO.

Message #23 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam.porter <at> 47ap.net, philipk <at> posteo.net, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Sat, 02 Sep 2023 10:15:48 -0700

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
> Or make the REGEXP argument to directory-files-recursively more
> specific, to reject lock files.
>
> But yes, the use of file-exists-p is not TRT, IMO.

See patch.

[0001-Don-t-native-compile-lock-files.patch (text/x-diff, attachment)]

Message #26 received at 65666 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam.porter <at> 47ap.net, philipk <at> posteo.net, 65666 <at> debbugs.gnu.org
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Sat, 02 Sep 2023 20:21:56 +0300

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: philipk <at> posteo.net, 65666 <at> debbugs.gnu.org, adam.porter <at> 47ap.net
> Date: Sat, 02 Sep 2023 10:15:48 -0700
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> > Or make the REGEXP argument to directory-files-recursively more
> > specific, to reject lock files.
> >
> > But yes, the use of file-exists-p is not TRT, IMO.
> 
> See patch.

Thanks, this LGTM.

Message #31 received at 65666-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam.porter <at> 47ap.net, 65666-done <at> debbugs.gnu.org,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#65666: Lockfiles break package-vc-install-from-checkout
Date: Sun, 03 Sep 2023 07:03:09 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc: philipk <at> posteo.net, 65666 <at> debbugs.gnu.org, adam.porter <at> 47ap.net
>> Date: Sat, 02 Sep 2023 10:15:48 -0700
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> > Or make the REGEXP argument to directory-files-recursively more
>> > specific, to reject lock files.
>> >
>> > But yes, the use of file-exists-p is not TRT, IMO.
>> 
>> See patch.
>
> Thanks, this LGTM.

Agree, I have pushed the change to master.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.