* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
@ 2023-05-16 23:12 Steven Allen
2023-05-18 19:54 ` Philip Kaludercic
0 siblings, 1 reply; 9+ messages in thread
From: Steven Allen @ 2023-05-16 23:12 UTC (permalink / raw)
To: 63541
* lisp/use-package/use-package-core.el (use-package-handler/:vc):
Return the new 'body'. Otherwise, the entire package declaration gets
dropped when byte-compiling.
---
lisp/use-package/use-package-core.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 0d99e270a3f..e0e16134ed3 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -1619,7 +1619,8 @@ defvar use-package--form)
;; 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
+ (push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
+ body))
(defun use-package-normalize--vc-arg (arg)
"Normalize possible arguments to the `:vc' keyword.
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-16 23:12 bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled Steven Allen
@ 2023-05-18 19:54 ` Philip Kaludercic
2023-05-19 15:16 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 9+ messages in thread
From: Philip Kaludercic @ 2023-05-18 19:54 UTC (permalink / raw)
To: Tony Zorman; +Cc: 63541, Steven Allen
Steven Allen <steven@stebalien.com> writes:
> * lisp/use-package/use-package-core.el (use-package-handler/:vc):
> Return the new 'body'. Otherwise, the entire package declaration gets
> dropped when byte-compiling.
> ---
> lisp/use-package/use-package-core.el | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
> index 0d99e270a3f..e0e16134ed3 100644
> --- a/lisp/use-package/use-package-core.el
> +++ b/lisp/use-package/use-package-core.el
> @@ -1619,7 +1619,8 @@ defvar use-package--form)
> ;; 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
> + (push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
> + body))
>
> (defun use-package-normalize--vc-arg (arg)
> "Normalize possible arguments to the `:vc' keyword.
CC'ing Tony, since he did most of the work here. Do you have any
comments on this change?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-18 19:54 ` Philip Kaludercic
@ 2023-05-19 15:16 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-20 16:50 ` Philip Kaludercic
0 siblings, 1 reply; 9+ messages in thread
From: Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-19 15:16 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: 63541, Steven Allen
On Thu, May 18 2023 19:54, Philip Kaludercic wrote:
> Steven Allen <steven@stebalien.com> writes:
>
>> * lisp/use-package/use-package-core.el (use-package-handler/:vc):
>> Return the new 'body'. Otherwise, the entire package declaration gets
>> dropped when byte-compiling.
>> ---
>> lisp/use-package/use-package-core.el | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
>> index 0d99e270a3f..e0e16134ed3 100644
>> --- a/lisp/use-package/use-package-core.el
>> +++ b/lisp/use-package/use-package-core.el
>> @@ -1619,7 +1619,8 @@ defvar use-package--form)
>> ;; 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
>> + (push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
>> + body))
>>
>> (defun use-package-normalize--vc-arg (arg)
>> "Normalize possible arguments to the `:vc' keyword.
>
> CC'ing Tony, since he did most of the work here. Do you have any
> comments on this change?
Looks fine to me; @Steven thanks for catching this! Seems like I wasn't
careful enough when testing with a byte compiled file.
I'm not super familiar with the testing infrastructure here, but I
wonder whether there is an easy way to write a quick regression test for
this?
--
Tony Zorman | https://tony-zorman.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-19 15:16 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-20 16:50 ` Philip Kaludercic
2023-05-21 9:32 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 9+ messages in thread
From: Philip Kaludercic @ 2023-05-20 16:50 UTC (permalink / raw)
To: Tony Zorman; +Cc: 63541, Steven Allen
Tony Zorman <soliditsallgood@mailbox.org> writes:
> On Thu, May 18 2023 19:54, Philip Kaludercic wrote:
>> Steven Allen <steven@stebalien.com> writes:
>>
>>> * lisp/use-package/use-package-core.el (use-package-handler/:vc):
>>> Return the new 'body'. Otherwise, the entire package declaration gets
>>> dropped when byte-compiling.
>>> ---
>>> lisp/use-package/use-package-core.el | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
>>> index 0d99e270a3f..e0e16134ed3 100644
>>> --- a/lisp/use-package/use-package-core.el
>>> +++ b/lisp/use-package/use-package-core.el
>>> @@ -1619,7 +1619,8 @@ defvar use-package--form)
>>> ;; 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
>>> + (push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
>>> + body))
>>>
>>> (defun use-package-normalize--vc-arg (arg)
>>> "Normalize possible arguments to the `:vc' keyword.
>>
>> CC'ing Tony, since he did most of the work here. Do you have any
>> comments on this change?
>
> Looks fine to me; @Steven thanks for catching this! Seems like I wasn't
> careful enough when testing with a byte compiled file.
OK, I'll apply it in that case.
> I'm not super familiar with the testing infrastructure here, but I
> wonder whether there is an easy way to write a quick regression test for
> this?
I'm not a professional software engineer, so I don't know what this
means :/
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-20 16:50 ` Philip Kaludercic
@ 2023-05-21 9:32 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-26 4:54 ` Steven Allen
0 siblings, 1 reply; 9+ messages in thread
From: Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-21 9:32 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: 63541, Steven Allen
On Sat, May 20 2023 16:50, Philip Kaludercic wrote:
>> I'm not super familiar with the testing infrastructure here, but I
>> wonder whether there is an easy way to write a quick regression test for
>> this?
>
> I'm not a professional software engineer,
Me neither, so don't worry about it :)
> so I don't know what this means :/
Basically, I was wondering whether it'd be possible to write a test that
byte compiles some statement and then checks the relevant expansion of
the macro. This way, whenever the (now correct) code changes in the
future (for whatever reason—most likely a refactor, I guess), one could
verify that the functionality wasn't inadvertently broken again.
--
Tony Zorman | https://tony-zorman.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-21 9:32 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-26 4:54 ` Steven Allen
2023-05-31 13:23 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Steven Allen @ 2023-05-26 4:54 UTC (permalink / raw)
To: Tony Zorman; +Cc: 63541
[-- Attachment #1: Type: text/plain, Size: 81 bytes --]
Here's a version of the patch with a test.
(sent to the right list this time)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-use-package-vc-keyword-when-byte-compiled.patch --]
[-- Type: text/x-patch, Size: 2404 bytes --]
From fee8eb4e81f8d541301bdbe74d7090bc5f27e455 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Tue, 16 May 2023 16:07:36 -0700
Subject: [PATCH] ; Fix use-package :vc keyword when byte-compiled
* lisp/use-package/use-package-core.el (use-package-handler/:vc):
Return the new 'body'. Otherwise, the entire package declaration gets
dropped when byte-compiling.
* test/lisp/use-package/use-package-tests.el
(use-package-test-handler/:vc-6): test it.
---
lisp/use-package/use-package-core.el | 3 ++-
test/lisp/use-package/use-package-tests.el | 11 +++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 0d99e270a3f..e0e16134ed3 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -1619,7 +1619,8 @@ use-package-handler/:vc
;; 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
+ (push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
+ body))
(defun use-package-normalize--vc-arg (arg)
"Normalize possible arguments to the `:vc' keyword.
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index c8c20fc51cb..9181a8171a7 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1991,6 +1991,17 @@ use-package-test/:vc-5
(use-package-vc-install '(other-name) ,load-path?)
(require 'foo nil nil)))))
+(ert-deftest use-package-test-handler/:vc-6 ()
+ (let ((byte-compile-current-file "use-package-core.el")
+ tried-to-install)
+ (cl-letf (((symbol-function #'use-package-vc-install)
+ (lambda (arg &optional local-path)
+ (setq tried-to-install arg))))
+ (should (equal
+ (use-package-handler/:vc 'foo nil 'some-pkg '(:init (foo)) nil)
+ '(foo)))
+ (should (eq tried-to-install 'some-pkg)))))
+
(ert-deftest use-package-test-normalize/:vc ()
(should (equal '(foo "version-string")
(use-package-normalize/:vc 'foo :vc '("version-string"))))
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-26 4:54 ` Steven Allen
@ 2023-05-31 13:23 ` Eli Zaretskii
2023-05-31 21:53 ` Philip Kaludercic
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2023-05-31 13:23 UTC (permalink / raw)
To: Steven Allen, Philip Kaludercic; +Cc: soliditsallgood, 63541
> Cc: 63541@debbugs.gnu.org
> From: Steven Allen <steven@stebalien.com>
> Date: Thu, 25 May 2023 21:54:29 -0700
>
> Here's a version of the patch with a test.
Philip, should I install this?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-31 13:23 ` Eli Zaretskii
@ 2023-05-31 21:53 ` Philip Kaludercic
2023-06-03 7:28 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Philip Kaludercic @ 2023-05-31 21:53 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: soliditsallgood, 63541, Steven Allen
Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: 63541@debbugs.gnu.org
>> From: Steven Allen <steven@stebalien.com>
>> Date: Thu, 25 May 2023 21:54:29 -0700
>>
>> Here's a version of the patch with a test.
>
> Philip, should I install this?
I don't value my own input, since this is related to use-package which I
do not use, but from a general intuition it looks fine.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled
2023-05-31 21:53 ` Philip Kaludercic
@ 2023-06-03 7:28 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2023-06-03 7:28 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: soliditsallgood, 63541-done, steven
> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Steven Allen <steven@stebalien.com>, soliditsallgood@mailbox.org,
> 63541@debbugs.gnu.org
> Date: Wed, 31 May 2023 21:53:41 +0000
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> Cc: 63541@debbugs.gnu.org
> >> From: Steven Allen <steven@stebalien.com>
> >> Date: Thu, 25 May 2023 21:54:29 -0700
> >>
> >> Here's a version of the patch with a test.
> >
> > Philip, should I install this?
>
> I don't value my own input, since this is related to use-package which I
> do not use, but from a general intuition it looks fine.
Thanks for the feedback. I've now installed this on the master
branch, and I'm closing the bug.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-03 7:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-16 23:12 bug#63541: [PATCH] ; Fix use-package :vc keyword when byte-compiled Steven Allen
2023-05-18 19:54 ` Philip Kaludercic
2023-05-19 15:16 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-20 16:50 ` Philip Kaludercic
2023-05-21 9:32 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-26 4:54 ` Steven Allen
2023-05-31 13:23 ` Eli Zaretskii
2023-05-31 21:53 ` Philip Kaludercic
2023-06-03 7:28 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).