bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Al Haji-Ali]

In my config file, I have a use-package statement like this

,----
| (use-package org
| :custom
| (org-log-reschedule nil))
`----

If I byte-compile my config file, run Emacs and run:

,----
| (message "START value is %S" org-log-reschedule)
| (setq org-log-reschedule t)
| (message "BEFORE value is %S" org-log-reschedule)
| (define-advice enable-theme (:before (theme) enable-theme@debug)
|     (message "Loading theme %S" theme)))
| (require 'use-package)
| (message "AFTER value is %S" org-log-reschedule)
`----

I get the messages:
,----
| START value is nil
| BEFORE value is t
| Loading theme `use-package`
| AFTER value is nil
`----

I believe that is because, in my byte-compiled config, `use-package` is no longer required since all `use-package` statements are expanded. This means that `use-package-core` is also no longer required and in that file, a naked (enable-theme 'use-package) statement, which resets all custom random variables, is always executed. This is exactly what happens when I eventually require `use-package`.

I can fix this by simply always requiring `use-package` in my config so that the enable-theme statement in `use-package-core` is executed before I modify any variables. Alternatively, this small patch seems to fix the root cause of the issue.

--8<---------------cut here---------------start------------->8---
diff --git a/use-package-core.el b/use-package-core.el
index bb523f6..5b1a414 100644
--- a/use-package-core.el
+++ b/use-package-core.el
@@ -38,7 +38,8 @@
   ;; Necessary in order to avoid having those variables saved by custom.el.
   (deftheme use-package))
 
-(enable-theme 'use-package)
+(unless (memq 'use-package custom-known-themes)
+  (enable-theme 'use-package))
 ;; Remove the synthetic use-package theme from the enabled themes, so
 ;; iterating over them to "disable all themes" won't disable it.
 (setq custom-enabled-themes (remq 'use-package custom-enabled-themes))
--8<---------------cut here---------------end--------------->8---

-- Al

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Eli Zaretskii]

> From: Al Haji-Ali <abdo.haji.ali@gmail.com>
> Date: Thu, 03 Oct 2024 06:47:51 +0100
> 
> 
> In my config file, I have a use-package statement like this
> 
> ,----
> | (use-package org
> | :custom
> | (org-log-reschedule nil))
> `----
> 
> If I byte-compile my config file, run Emacs and run:
> 
> ,----
> | (message "START value is %S" org-log-reschedule)
> | (setq org-log-reschedule t)
> | (message "BEFORE value is %S" org-log-reschedule)
> | (define-advice enable-theme (:before (theme) enable-theme@debug)
> |     (message "Loading theme %S" theme)))
> | (require 'use-package)
> | (message "AFTER value is %S" org-log-reschedule)
> `----
> 
> I get the messages:
> ,----
> | START value is nil
> | BEFORE value is t
> | Loading theme `use-package`
> | AFTER value is nil
> `----
> 
> I believe that is because, in my byte-compiled config, `use-package` is no longer required since all `use-package` statements are expanded. This means that `use-package-core` is also no longer required and in that file, a naked (enable-theme 'use-package) statement, which resets all custom random variables, is always executed. This is exactly what happens when I eventually require `use-package`.
> 
> I can fix this by simply always requiring `use-package` in my config so that the enable-theme statement in `use-package-core` is executed before I modify any variables. Alternatively, this small patch seems to fix the root cause of the issue.

Can you try this with the latest emacs-30 branch of the Emacs Git
repository?  I believe some changes were done to use-package there,
and they might fix your problem as well.

Thanks.

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Al Haji-Ali]

> Can you try this with the latest emacs-30 branch of the Emacs Git
> repository?  I believe some changes were done to use-package there,
> and they might fix your problem as well.

I just tried it on the main branch (version 31.0.50) and saw the same behaviour. I also checked the part of the code that I think is the reason and it remains the same.

Here's how I am able to reproduce the issue:

--8<---------------cut here---------------start------------->8---
;;; 1. Run emacs -Q
;;; 2. Define the following function

(defun bug-73609 (&optional setup)
  (let ((filename (file-name-concat temporary-file-directory "temp.el")))
    (if setup
        (progn (write-region "(use-package org\n:custom\n(org-log-reschedule 'note))"
                             nil filename)
               (require 'use-package)
               (byte-compile-file filename))
      (load (concat filename "c"))
      (message "START value is %S, use-package is%s loaded"
               org-log-reschedule
               (if (featurep 'use-package)
                   ""
                 " NOT"))
      (setq org-log-reschedule t)
      (message "BEFORE value is %S" org-log-reschedule)
      (define-advice enable-theme (:before (theme) enable-theme@debug)
        (message "Loading theme %S" theme))
      (require 'use-package)
      (message "AFTER value is %S" org-log-reschedule))))

;;; 3. Call
(bug-73609 t)

;;; 4. Restart `emacs -Q` and redefine the function above
;;; 5. Run
(bug-73609)

;; The output is
; START value is note, use-package is NOT loaded
; BEFORE value is t
; Loading theme use-package
; Loading theme user
; AFTER value is note
--8<---------------cut here---------------end--------------->8---


I expect the final line to be

; AFTER value is t

as I do not expect that loading `use-package` would change the value of the variable `org-log-reschedule`. 

-- Al

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Eli Zaretskii]

> From: Al Haji-Ali <abdo.haji.ali@gmail.com>
> Cc: 73609@debbugs.gnu.org
> Date: Fri, 11 Oct 2024 23:25:08 +0100
> 
> 
> 
> > Can you try this with the latest emacs-30 branch of the Emacs Git
> > repository?  I believe some changes were done to use-package there,
> > and they might fix your problem as well.
> 
> I just tried it on the main branch (version 31.0.50) and saw the same behaviour. I also checked the part of the code that I think is the reason and it remains the same.
> 
> Here's how I am able to reproduce the issue:
> 
> --8<---------------cut here---------------start------------->8---
> ;;; 1. Run emacs -Q
> ;;; 2. Define the following function
> 
> (defun bug-73609 (&optional setup)
>   (let ((filename (file-name-concat temporary-file-directory "temp.el")))
>     (if setup
>         (progn (write-region "(use-package org\n:custom\n(org-log-reschedule 'note))"
>                              nil filename)
>                (require 'use-package)
>                (byte-compile-file filename))
>       (load (concat filename "c"))
>       (message "START value is %S, use-package is%s loaded"
>                org-log-reschedule
>                (if (featurep 'use-package)
>                    ""
>                  " NOT"))
>       (setq org-log-reschedule t)
>       (message "BEFORE value is %S" org-log-reschedule)
>       (define-advice enable-theme (:before (theme) enable-theme@debug)
>         (message "Loading theme %S" theme))
>       (require 'use-package)
>       (message "AFTER value is %S" org-log-reschedule))))
> 
> ;;; 3. Call
> (bug-73609 t)
> 
> ;;; 4. Restart `emacs -Q` and redefine the function above
> ;;; 5. Run
> (bug-73609)
> 
> ;; The output is
> ; START value is note, use-package is NOT loaded
> ; BEFORE value is t
> ; Loading theme use-package
> ; Loading theme user
> ; AFTER value is note
> --8<---------------cut here---------------end--------------->8---
> 
> 
> I expect the final line to be
> 
> ; AFTER value is t
> 
> as I do not expect that loading `use-package` would change the value of the variable `org-log-reschedule`. 

John, any suggestions or ideas?

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[John Wiegley]

I don’t fully understand. The purpose of of `:custom (X Y)` is exactly to customize X to the value Y using the value setting functions of the customization framework.

Is the user expecting that byte-compilation would cause the value to be set without loading use-package?

I need a bit more clarity. Also, a much simpler approach to assessing what use-package is “doing” is to expand the macro, as that should make the intended behavior obvious.

John

> On Oct 12, 2024, at 1:07 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Al Haji-Ali <abdo.haji.ali@gmail.com>
>> Cc: 73609@debbugs.gnu.org
>> Date: Fri, 11 Oct 2024 23:25:08 +0100
>> 
>> 
>> 
>>> Can you try this with the latest emacs-30 branch of the Emacs Git
>>> repository?  I believe some changes were done to use-package there,
>>> and they might fix your problem as well.
>> 
>> I just tried it on the main branch (version 31.0.50) and saw the same behaviour. I also checked the part of the code that I think is the reason and it remains the same.
>> 
>> Here's how I am able to reproduce the issue:
>> 
>> --8<---------------cut here---------------start------------->8---
>> ;;; 1. Run emacs -Q
>> ;;; 2. Define the following function
>> 
>> (defun bug-73609 (&optional setup)
>>  (let ((filename (file-name-concat temporary-file-directory "temp.el")))
>>    (if setup
>>        (progn (write-region "(use-package org\n:custom\n(org-log-reschedule 'note))"
>>                             nil filename)
>>               (require 'use-package)
>>               (byte-compile-file filename))
>>      (load (concat filename "c"))
>>      (message "START value is %S, use-package is%s loaded"
>>               org-log-reschedule
>>               (if (featurep 'use-package)
>>                   ""
>>                 " NOT"))
>>      (setq org-log-reschedule t)
>>      (message "BEFORE value is %S" org-log-reschedule)
>>      (define-advice enable-theme (:before (theme) enable-theme@debug)
>>        (message "Loading theme %S" theme))
>>      (require 'use-package)
>>      (message "AFTER value is %S" org-log-reschedule))))
>> 
>> ;;; 3. Call
>> (bug-73609 t)
>> 
>> ;;; 4. Restart `emacs -Q` and redefine the function above
>> ;;; 5. Run
>> (bug-73609)
>> 
>> ;; The output is
>> ; START value is note, use-package is NOT loaded
>> ; BEFORE value is t
>> ; Loading theme use-package
>> ; Loading theme user
>> ; AFTER value is note
>> --8<---------------cut here---------------end--------------->8---
>> 
>> 
>> I expect the final line to be
>> 
>> ; AFTER value is t
>> 
>> as I do not expect that loading `use-package` would change the value of the variable `org-log-reschedule`.
> 
> John, any suggestions or ideas?

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Al Haji-Ali]

Hello,

On 12/10/2024, John Wiegley wrote:

> I don’t fully understand. The purpose of of `:custom (X Y)` is exactly to customize X to the value Y using the value setting functions of the customization framework.
>
> Is the user expecting that byte-compilation would cause the value to be set without loading use-package?

No, the point is that this is already the case, and that is causing a discrepancy between having a compiled file vs not. The original message had the context, but if you've seen that already, here's another crack at explaining. Apologies for the repetition.

The function definition above essentially has the sequence:
1- load a file that uses `use-package` with a `:custom` statement to set a variable. 
2- modify said variable to some other value.
3- require `use-package`.
4- Check value of variable.

If the file is not compiled, then `use-package` is loaded in step 1 in order to expand the `use-package` statement. This means that the third step doesn't do anything, and the value of the variable is set to whatever I had in step 2.

However, if the file is compiled, the first step does not load `use-package`, due to lazy loading and since all `use-package` macro calls are expanded in a compiled file. This means that the loading of `use-package` happens in the third step, after I have modified the variable. The in turns loads `use-package-core` and the 'use-package theme is enabled, which resets the value of all custom variables that were set using `:custom` statements. The final result is that value of the variable in step 4 is the one from step 1, rather than step 2.

I hope this clarifies the issue. I included a potential patch in my original message which fixes the issue, but I am not sure if it would have unintended consequences,

-- Al

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[John Wiegley]

>>>>> Al Haji-Ali <abdo.haji.ali@gmail.com> writes:

> I hope this clarifies the issue. I included a potential patch in my original
> message which fixes the issue, but I am not sure if it would have unintended
> consequences,

Ah, I understand now, this is indeed a complication that should be addressed.
Byte-compiling use-package declarations has always been rather tricky, and it
still does not even work for my own personal configuration.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Eli Zaretskii]

> From: John Wiegley <johnw@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  73609@debbugs.gnu.org
> Date: Mon, 14 Oct 2024 08:10:39 -0700
> 
> >>>>> Al Haji-Ali <abdo.haji.ali@gmail.com> writes:
> 
> > I hope this clarifies the issue. I included a potential patch in my original
> > message which fixes the issue, but I am not sure if it would have unintended
> > consequences,
> 
> Ah, I understand now, this is indeed a complication that should be addressed.
> Byte-compiling use-package declarations has always been rather tricky, and it
> still does not even work for my own personal configuration.

How should we proceed with this bug report?  Should we close it as
wontfix?

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[John Wiegley]

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

>> > I hope this clarifies the issue. I included a potential patch in my original
>> > message which fixes the issue, but I am not sure if it would have unintended
>> > consequences,
>> 
>> Ah, I understand now, this is indeed a complication that should be addressed.
>> Byte-compiling use-package declarations has always been rather tricky, and it
>> still does not even work for my own personal configuration.

> How should we proceed with this bug report? Should we close it as wontfix?

I think this one has to be “won’t fix”, indeed, with a note in the
documentation about possible bad interactions between :custom settings and
byte-compiled files…

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[abdo.haji.ali]

> I think this one has to be “won’t fix”, indeed, with a note in the
> documentation about possible bad interactions between :custom settings and
> byte-compiled files…

Just to mention that a fix on the user side would be to always require use-package to force the loading of the theme before other user code is executed. Of course this defeats the purpose of lazy-loading use-package that byte compiling would normally offer.

An easy “fix” from the use-package side, with virtually zero side effects, would be to move the loading of the theme into a small module that the user can always require without loading all of use-package.

bug#73609: 29.1; use-package overwrites custom variables in byte-compiled files

[Eli Zaretskii]

> From: abdo.haji.ali@gmail.com
> Date: Wed, 30 Oct 2024 08:49:30 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 73609@debbugs.gnu.org
> 
> 
> > I think this one has to be “won’t fix”, indeed, with a note in the
> > documentation about possible bad interactions between :custom settings and
> > byte-compiled files…
> 
> Just to mention that a fix on the user side would be to always require use-package to force the loading of the theme before other user code is executed. Of course this defeats the purpose of lazy-loading use-package that byte compiling would normally offer.
> 
> An easy “fix” from the use-package side, with virtually zero side effects, would be to move the loading of the theme into a small module that the user can always require without loading all of use-package.

Thanks, I've now updated the use-package manual with the relevant
information (on the emacs-30 branch), and I'm therefore closing this
bug.