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

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

[Steven Allen]

* 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

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

[Philip Kaludercic]

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?

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

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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/

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

[Philip Kaludercic]

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 :/

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

[Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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/

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

[Steven Allen]

[-- 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

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

[Eli Zaretskii]

> 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?

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

[Philip Kaludercic]

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.

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

[Eli Zaretskii]

> 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.