GNU bug report logs - #63541
[PATCH] ; Fix use-package :vc keyword when byte-compiled

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 63541 in the body.
You can then email your comments to 63541 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: Steven Allen <steven <at> stebalien.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] ; Fix use-package :vc keyword when byte-compiled
Date: Tue, 16 May 2023 16:12:04 -0700

* 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

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Tony Zorman <soliditsallgood <at> mailbox.org>
Cc: 63541 <at> debbugs.gnu.org, Steven Allen <steven <at> stebalien.com>
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Thu, 18 May 2023 19:54:50 +0000

Steven Allen <steven <at> 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?

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

From: Tony Zorman <soliditsallgood <at> mailbox.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 63541 <at> debbugs.gnu.org, Steven Allen <steven <at> stebalien.com>
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Fri, 19 May 2023 17:16:20 +0200

On Thu, May 18 2023 19:54, Philip Kaludercic wrote:
> Steven Allen <steven <at> 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/

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Tony Zorman <soliditsallgood <at> mailbox.org>
Cc: 63541 <at> debbugs.gnu.org, Steven Allen <steven <at> stebalien.com>
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Sat, 20 May 2023 16:50:18 +0000

Tony Zorman <soliditsallgood <at> mailbox.org> writes:

> On Thu, May 18 2023 19:54, Philip Kaludercic wrote:
>> Steven Allen <steven <at> 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 :/

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

From: Tony Zorman <soliditsallgood <at> mailbox.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 63541 <at> debbugs.gnu.org, Steven Allen <steven <at> stebalien.com>
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Sun, 21 May 2023 11:32:00 +0200

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/

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

From: Steven Allen <steven <at> stebalien.com>
To: Tony Zorman <soliditsallgood <at> mailbox.org>
Cc: 63541 <at> debbugs.gnu.org
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Thu, 25 May 2023 21:54:29 -0700

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

Here's a version of the patch with a test.

(sent to the right list this time)

[0001-Fix-use-package-vc-keyword-when-byte-compiled.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steven Allen <steven <at> stebalien.com>, Philip Kaludercic <philipk <at> posteo.net>
Cc: soliditsallgood <at> mailbox.org, 63541 <at> debbugs.gnu.org
Subject: Re: bug#63541: [PATCH] ;
 Fix use-package :vc keyword when byte-compiled
Date: Wed, 31 May 2023 16:23:18 +0300

> Cc: 63541 <at> debbugs.gnu.org
> From: Steven Allen <steven <at> 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?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: soliditsallgood <at> mailbox.org, 63541 <at> debbugs.gnu.org,
 Steven Allen <steven <at> stebalien.com>
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Wed, 31 May 2023 21:53:41 +0000

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

>> Cc: 63541 <at> debbugs.gnu.org
>> From: Steven Allen <steven <at> 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.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: soliditsallgood <at> mailbox.org, 63541-done <at> debbugs.gnu.org,
 steven <at> stebalien.com
Subject: Re: bug#63541: [PATCH] ; Fix use-package :vc keyword when
 byte-compiled
Date: Sat, 03 Jun 2023 10:28:51 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: Steven Allen <steven <at> stebalien.com>,  soliditsallgood <at> mailbox.org,
>   63541 <at> debbugs.gnu.org
> Date: Wed, 31 May 2023 21:53:41 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Cc: 63541 <at> debbugs.gnu.org
> >> From: Steven Allen <steven <at> 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.

Previous Next

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