bug#72414: 29.4; use-package :defer keyword does not honor ":defer nil"

bug#72414: 29.4; use-package :defer keyword does not honor ":defer nil"

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

- The relevant file is "lisp/use-package/use-package-core.el".
- The function is "use-package-normalize-keywords".
- This is as of commit 4c6e9f5b004, current for this file on the
  "emacs-30" tag.

Near the bottom of the use-package-normalize-keywords function, we have
the following code block.

    ;; If at this point no :load, :defer or :no-require has been seen, then
    ;; :load the package itself.
    (when (and (not (plist-member args :load))
               (not (plist-member args :defer))
               (not (plist-member args :no-require)))
      (setq args (append args `(:load (,name)))))

The intent is that we add an immediate-load instruction to the keyword
list, if there is no reason to not add it.  We have a couple of possible
reasons to not add it:

1. There's already a :load keyword.
2. We have asked use-package to :defer loading.
3. We have asked use-package to not require the package, to avoid
   certain byte-compilation problems.

The Use-Package documentation section 3.2 goes into more detail about
the :defer keyword behavior in (2).

   If you customize the user option ‘use-package-always-defer’ to
   non-‘nil’, the ‘use-package’ macro will behave as if ‘:defer t’ is
   always specified.  This can be overridden for individual declarations
   using either ‘:defer nil’ or ‘:demand t’ (*note Forcing loading::).

The way the code above uses plist-member doesn't match the
documentation.  The test looks for the simple presence of a :defer
key/value, and doesn't distinguish between any of the possible values:
t, nil, or an integer.

Replacing plist-member with plist-get should fix the problem.  A ":defer
nil" would produce the same nil value as a missing :defer.




In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.42,
 cairo version 1.18.0) of 2024-06-29, modified by Debian built on sbuild
Windowing system distributor 'The X.Org Foundation', version 11.0.12101011
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --with-cairo --with-x=yes
 --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -Werror=implicit-function-declaration
 -ffile-prefix-map=/build/reproducible-path/emacs-29.4+1=. -fstack-protector-strong
 -fstack-clash-protection -Wformat -Werror=format-security
 -fcf-protection -Wall' 'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2'
 LDFLAGS=-Wl,-z,relro'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2
XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

-- 
Derek Upham
derek_upham@mailfence.com

bug#72414: 29.4; use-package :defer keyword does not honor ":defer nil"

[Eli Zaretskii]

> Date: Thu, 01 Aug 2024 16:30:40 -0700
> From:  Derek Upham via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> - The relevant file is "lisp/use-package/use-package-core.el".
> - The function is "use-package-normalize-keywords".
> - This is as of commit 4c6e9f5b004, current for this file on the
>   "emacs-30" tag.
> 
> Near the bottom of the use-package-normalize-keywords function, we have
> the following code block.
> 
>     ;; If at this point no :load, :defer or :no-require has been seen, then
>     ;; :load the package itself.
>     (when (and (not (plist-member args :load))
>                (not (plist-member args :defer))
>                (not (plist-member args :no-require)))
>       (setq args (append args `(:load (,name)))))
> 
> The intent is that we add an immediate-load instruction to the keyword
> list, if there is no reason to not add it.  We have a couple of possible
> reasons to not add it:
> 
> 1. There's already a :load keyword.
> 2. We have asked use-package to :defer loading.
> 3. We have asked use-package to not require the package, to avoid
>    certain byte-compilation problems.
> 
> The Use-Package documentation section 3.2 goes into more detail about
> the :defer keyword behavior in (2).
> 
>    If you customize the user option ‘use-package-always-defer’ to
>    non-‘nil’, the ‘use-package’ macro will behave as if ‘:defer t’ is
>    always specified.  This can be overridden for individual declarations
>    using either ‘:defer nil’ or ‘:demand t’ (*note Forcing loading::).
> 
> The way the code above uses plist-member doesn't match the
> documentation.  The test looks for the simple presence of a :defer
> key/value, and doesn't distinguish between any of the possible values:
> t, nil, or an integer.
> 
> Replacing plist-member with plist-get should fix the problem.  A ":defer
> nil" would produce the same nil value as a missing :defer.

Thanks for the report and the analysis.

John, any comments or suggestions about this?

bug#72414: 29.4; use-package :defer keyword does not honor ":defer nil"

[John Wiegley]

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

>> The way the code above uses plist-member doesn't match the
>> documentation.  The test looks for the simple presence of a :defer
>> key/value, and doesn't distinguish between any of the possible values:
>> t, nil, or an integer.
>> 
>> Replacing plist-member with plist-get should fix the problem.  A ":defer
>> nil" would produce the same nil value as a missing :defer.

> Thanks for the report and the analysis.
>
> John, any comments or suggestions about this?

This sounds like a very good analysis, and I’m so glad you caught this. Yes,
it shouldn’t just be a plist membership test; it should lookup the member in
the plist AND if it’s present and non-nil, then the truth of the condition
should be established.

So, from:

    (when (and (not (plist-member args :load))
               (not (plist-member args :defer))
               (not (plist-member args :no-require)))
      (setq args (append args `(:load (,name)))))

to:

    (when (and (not (plist-get args :load))
               (not (plist-get args :defer))
               (not (plist-get args :no-require)))
      (setq args (append args `(:load (,name)))))


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

bug#72414: 29.4; use-package :defer keyword does not honor ":defer nil"

[Eli Zaretskii]

> From: John Wiegley <johnw@gnu.org>
> Cc: Derek Upham <derek_upham@mailfence.com>,  72414@debbugs.gnu.org
> Date: Fri, 02 Aug 2024 15:32:07 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The way the code above uses plist-member doesn't match the
> >> documentation.  The test looks for the simple presence of a :defer
> >> key/value, and doesn't distinguish between any of the possible values:
> >> t, nil, or an integer.
> >> 
> >> Replacing plist-member with plist-get should fix the problem.  A ":defer
> >> nil" would produce the same nil value as a missing :defer.
> 
> > Thanks for the report and the analysis.
> >
> > John, any comments or suggestions about this?
> 
> This sounds like a very good analysis, and I’m so glad you caught this. Yes,
> it shouldn’t just be a plist membership test; it should lookup the member in
> the plist AND if it’s present and non-nil, then the truth of the condition
> should be established.
> 
> So, from:
> 
>     (when (and (not (plist-member args :load))
>                (not (plist-member args :defer))
>                (not (plist-member args :no-require)))
>       (setq args (append args `(:load (,name)))))
> 
> to:
> 
>     (when (and (not (plist-get args :load))
>                (not (plist-get args :defer))
>                (not (plist-get args :no-require)))
>       (setq args (append args `(:load (,name)))))

Thanks, installed on the emacs-30 release branch, and closing the bug.