pkg-autoloads.el vs. pkg-loaddefs.el

pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Hello,

According to 43.1 Packaging Basics section of Elisp manual, package
autoloads are saved into pkg-autoloads.el file.

However, on master, there is also `generated-autoload-file' variable
that can override the file name where to write the autoloads. So,
pkg-autoloads.el is not always the file where autoloads are written.

Further, `elpaa-batch-generate-autoloads' in ELPA/admin/elpa-admin.el
hard-codes pkg-autoloads.el. And I got a report that in some Emacs
packages distributed through ELPA (org-mode, tramp, hyperbole), both
pkg-autoloads and pkg-loaddefs are generated.

Finally, all the libraries in Emacs core are using name-loaddefs.el.

I feel confused about where to use which name. There is apparent
inconsistency in the above facts.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier via Emacs development discussions.]

> Further, `elpaa-batch-generate-autoloads' in ELPA/admin/elpa-admin.el
> hard-codes pkg-autoloads.el. And I got a report that in some Emacs
> packages distributed through ELPA (org-mode, tramp, hyperbole), both
> pkg-autoloads and pkg-loaddefs are generated.
[...]
> Finally, all the libraries in Emacs core are using name-loaddefs.el.
>
> I feel confused about where to use which name. There is apparent
> inconsistency in the above facts.

Here's how I see it:

`<pkg>-autoloads.el` is supposed to contain what would have gone into
`lisp/loaddefs.el` if the package were part of Emacs.  I.e. it should
contain the strict minimum necessary to expose the usual entry points to
the package: enough that users of the package usually don't need to do
things like add `require` or touch `auto-mode-alist` in their `.emacs`,
but not so much that users who *don't* use the package pay an undue cost.
ELPA packages can be installed without ever being used, just like
packages bundled with Emacs, and contrary to `lisp/loaddefs.el`,
`<pkg>-autoloads.el` are loaded at startup so the larger they are, the
slower Emacs starts up: it's really important to keep them minimal.

`<pkg>-loaddefs.el` is not "codified" in the same way, but in practice
it contains those autoloads *internal* to a package, i.e. loaded only
once we know the package is being used and typically used to avoid some
undesirable toplevel `require`s inside the package's files (e.g. to
avoid circular dependencies, or to load the different subparts of the
package more lazily).  It's fairly normal for such files to be
significantly larger than the corresponding `<pkg>-autoloads.el`.

I hope someone can work on providing better support for
`<pkg>-loaddefs.el`: Lars (IIRC) has done some of the work for that in
`loaddefs-gen.el`, but I think there's still some "glue" around that's
needed to make it easy for a package developer to start using such
a file (e.g. have `package-vc` and `elpa-admin.el` generate those
files, documenting how to do it, ...).


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier via "Emacs development discussions."
<emacs-devel@gnu.org> writes:

> Here's how I see it:
>
> `<pkg>-autoloads.el` is supposed to contain what would have gone into
> `lisp/loaddefs.el` if the package were part of Emacs.  I.e. it should
> contain the strict minimum necessary to expose the usual entry points to
> the package: enough that users of the package usually don't need to do
> things like add `require` or touch `auto-mode-alist` in their `.emacs`,
> but not so much that users who *don't* use the package pay an undue cost.
> ELPA packages can be installed without ever being used, just like
> packages bundled with Emacs, and contrary to `lisp/loaddefs.el`,
> `<pkg>-autoloads.el` are loaded at startup so the larger they are, the
> slower Emacs starts up: it's really important to keep them minimal.

Makes sense.

> `<pkg>-loaddefs.el` is not "codified" in the same way, but in practice
> it contains those autoloads *internal* to a package, i.e. loaded only
> once we know the package is being used and typically used to avoid some
> undesirable toplevel `require`s inside the package's files (e.g. to
> avoid circular dependencies, or to load the different subparts of the
> package more lazily).  It's fairly normal for such files to be
> significantly larger than the corresponding `<pkg>-autoloads.el`.

Also makes sense.
However, it potentially creates a problem for packages like Org mode
that use _both_ pkg-autoloads.el and pkg-loaddefs.el.

If I understand your explanation correctly, `package-activate' only
loads org-autoloads.el when Org mode is installed from ELPA.
It means that built-in org-loaddefs.el version pre-loaded with Emacs
remains active and any changes in org-loaddefs.el in ELPA Org mode
vs. built-in Org mode are not updated.

In fact, I recently encountered a problem that is very likely caused by
Emacs retaining org-loaddefs.el from built-in Org version. I moved Org
major mode definition to a new library org-mode.el, but M-x org-mode
still insisted on loading org.el, despite new org-loaddefs.el pointing
to org-mode.el.

IMHO, for built-in packages, it seems that pkg-loaddefs _has to be
loaded_ even when the packages is installed via package.el. Otherwise,
we are going to face random breakage every time a function/macro is
relocated to another file or a new function gets autoloaded.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

> Also makes sense.  However, it potentially creates a problem for
> packages like Org mode that use _both_ pkg-autoloads.el and
> pkg-loaddefs.el.

[ Side note: all ELPA packages that use a `<pkg>-loaddefs.el` also use
  a `<pkg>-autoloads.el`.  ]

> If I understand your explanation correctly, `package-activate' only
> loads org-autoloads.el when Org mode is installed from ELPA.

I don't know what you mean to say by that.

> It means that built-in org-loaddefs.el version pre-loaded with Emacs
> remains active and any changes in org-loaddefs.el in ELPA Org mode
> vs. built-in Org mode are not updated.

`org-loaddefs.el` shouldn't be any different from any other `org*.el`
file, in this respect: if it's been loaded before the package is
activated, then (hopefully) `package--reload-previously-loaded` should
reload the newer file.

[ I've heard various reports over the years that indicate that
  `package--reload-previously-loaded` doesn't always do that, but
  I don't know if it's because it's not called at all, or because it's
  called but makes wrong decisions.
  Also of course, there can be lots of cases where reloading a file isn't
  sufficient. ]


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It means that built-in org-loaddefs.el version pre-loaded with Emacs
>> remains active and any changes in org-loaddefs.el in ELPA Org mode
>> vs. built-in Org mode are not updated.
>
> `org-loaddefs.el` shouldn't be any different from any other `org*.el`
> file, in this respect: if it's been loaded before the package is
> activated, then (hopefully) `package--reload-previously-loaded` should
> reload the newer file.
>
> [ I've heard various reports over the years that indicate that
>   `package--reload-previously-loaded` doesn't always do that, but
>   I don't know if it's because it's not called at all, or because it's
>   called but makes wrong decisions.
>   Also of course, there can be lots of cases where reloading a file isn't
>   sufficient. ]

But isn't it called only when `package-activate-1' is called with RELOAD
argument? AFAIU, `package-activate' (and thus `package-activate-all')
calls (package-activate-1 pkg-desc nil 'deps).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

>> [ I've heard various reports over the years that indicate that
>>   `package--reload-previously-loaded` doesn't always do that, but
>>   I don't know if it's because it's not called at all, or because it's
>>   called but makes wrong decisions.
>>   Also of course, there can be lots of cases where reloading a file isn't
>>   sufficient. ]
>
> But isn't it called only when `package-activate-1' is called with RELOAD
> argument?

Indeed.

> AFAIU, `package-activate' (and thus `package-activate-all') calls
> (package-activate-1 pkg-desc nil 'deps).

The reasoning is that this won't be called after some `org*.el` files
have already been loaded.


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> AFAIU, `package-activate' (and thus `package-activate-all') calls
>> (package-activate-1 pkg-desc nil 'deps).
>
> The reasoning is that this won't be called after some `org*.el` files
> have already been loaded.

I have seen configs with package-activate-all being not the first thing
in the config.

Some configs even do package loading in early-init.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

>>> AFAIU, `package-activate' (and thus `package-activate-all') calls
>>> (package-activate-1 pkg-desc nil 'deps).
>> The reasoning is that this won't be called after some `org*.el` files
>> have already been loaded.
> I have seen configs with package-activate-all being not the first thing
> in the config.
> Some configs even do package loading in early-init.

I currently consider these cases to be "the user's mistake".
Maybe we should provide some alternative for "late activation" which
does pass the `reload` argument?

We don't do that by default in an effort to try and keep startup time
under control and also because reloading is a hack which works only
partly and can even have unpredictable effects, so it's really much
better to try and avoid it in the first place.


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I have seen configs with package-activate-all being not the first thing
>> in the config.
>> Some configs even do package loading in early-init.
>
> I currently consider these cases to be "the user's mistake".

This is a very common mistake.
I'd assert more - most users are not aware about the intricacies we are
talking about here. They just copy-paste the code from web and get
cryptic errors (and the errors from mixed installations are especially
cryptic). And they do not want to know these details (or, at least, do
not know where to search).

And the worst thing is that mixed installation problem often manifests
itself only between major releases and things "work fine" most of the
time. This frustrates people a lot.

> Maybe we should provide some alternative for "late activation" which
> does pass the `reload` argument?

Maybe. Although I am afraid that having multiple ways to activate
packages will only confuse people who struggle from the problem most.

> We don't do that by default in an effort to try and keep startup time
> under control and also because reloading is a hack which works only
> partly and can even have unpredictable effects, so it's really much
> better to try and avoid it in the first place.

May an approach like `require-with-check' be used here? Then, the
reloading only occurs when there is a potential mixing of package
versions.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

> I'd assert more - most users are not aware about the intricacies we
> are talking about here.

That, I can definitely agree with.  🙂

>> Maybe we should provide some alternative for "late activation" which
>> does pass the `reload` argument?
> Maybe. Although I am afraid that having multiple ways to activate
> packages will only confuse people who struggle from the problem most.

I tend to agree.  I think we should try and actively discourage users to
get into such situations.  I.e. focus on detecting the problem and
report it to the user rather than patching it up.

>> We don't do that by default in an effort to try and keep startup time
>> under control and also because reloading is a hack which works only
>> partly and can even have unpredictable effects, so it's really much
>> better to try and avoid it in the first place.
> May an approach like `require-with-check' be used here?  Then, the
> reloading only occurs when there is a potential mixing of package
> versions.

Could be, tho I'm not sure exactly what "an approach like
`require-with-check`" would look like.


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> May an approach like `require-with-check' be used here?  Then, the
>> reloading only occurs when there is a potential mixing of package
>> versions.
>
> Could be, tho I'm not sure exactly what "an approach like
> `require-with-check`" would look like.

Something like

(defun package-activate-1...
...

(let ((autoload-feature (intern (format "%s-autoloads"
                             (package-desc-name pkg-desc)))))
  (when (and (featurep autoload-feature)
             (require-with-check autoload-feature nil 'reload))
    ;; If pkg-autoload.el is already loaded from a different path,
    ;; reload the package.
    (warn ...)
    (setq reload t)))
    
(when reload
  (package--reload-previously-loaded pkg-desc))
  
(with-demoted-errors "Error loading autoloads: %s"
  (load (package--autoloads-file-name pkg-desc) nil t))


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

>>> May an approach like `require-with-check' be used here?  Then, the
>>> reloading only occurs when there is a potential mixing of package
>>> versions.
>>
>> Could be, tho I'm not sure exactly what "an approach like
>> `require-with-check`" would look like.
>
> Something like
>
> (defun package-activate-1...
> ...
>
> (let ((autoload-feature (intern (format "%s-autoloads"
>                              (package-desc-name pkg-desc)))))
>   (when (and (featurep autoload-feature)
>              (require-with-check autoload-feature nil 'reload))
>     ;; If pkg-autoload.el is already loaded from a different path,
>     ;; reload the package.
>     (warn ...)
>     (setq reload t)))
>     
> (when reload
>   (package--reload-previously-loaded pkg-desc))
>   
> (with-demoted-errors "Error loading autoloads: %s"
>   (load (package--autoloads-file-name pkg-desc) nil t))

Hmm... I wonder when that `featurep` test would trigger, tho.

E.g. for Org usually the "old already loaded Org" is the one from Emacs
which doesn't come with an `org-autoloads.el`.

As for the case where we've already loaded an older version of an
ELPA package, this requires two versions of the same ELPA package which
is unusual (except during an upgrade, but that case already passes the
`reload` arg).


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>   (when (and (featurep autoload-feature)
>>              (require-with-check autoload-feature nil 'reload))
>>     ;; If pkg-autoload.el is already loaded from a different path,
>>     ;; reload the package.
>>     (warn ...)
>>     (setq reload t)))
> ...
> Hmm... I wonder when that `featurep` test would trigger, tho.
>
> E.g. for Org usually the "old already loaded Org" is the one from Emacs
> which doesn't come with an `org-autoloads.el`.

Right.
Maybe org-loaddefs.el should (provide 'org-autoloads)? And same for
other built-in packages. Or there might be a token pkg-autoloads.el in
the tree for built-in packages.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

> Right.
> Maybe org-loaddefs.el should (provide 'org-autoloads)? And same for
> other built-in packages. Or there might be a token pkg-autoloads.el in
> the tree for built-in packages.

Maybe we can use `package-builtin-versions`?
How 'bout a patch like the one below?


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index fac824d44a4..7c534542503 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -837,11 +837,13 @@ package--library-stem
         (unless (equal file result)
           (throw 'done result))))))
 
-(defun package--reload-previously-loaded (pkg-desc)
+(defun package--reload-previously-loaded (pkg-desc &optional warn)
   "Force reimportation of files in PKG-DESC already present in `load-history'.
 New editions of files contain macro definitions and
 redefinitions, the overlooking of which would cause
-byte-compilation of the new package to fail."
+byte-compilation of the new package to fail.
+If WARN is a string, display a warning (using WARN as a format string)
+before reloading the files."
   (with-demoted-errors "Error in package--load-files-for-activation: %s"
     (let* (result
            (dir (package-desc-dir pkg-desc))
@@ -877,6 +879,10 @@ package--reload-previously-loaded
           (unless (equal (file-name-base library)
                          (format "%s-autoloads" (package-desc-name pkg-desc)))
             (push (cons (expand-file-name library dir) recent-index) result))))
+      (when (and result warn)
+        (display-warning 'package
+                         (format warn (package-desc-name pkg-desc)
+                                 (mapcar #'car result))))
       (mapc (lambda (c) (load (car c) nil t))
             (sort result (lambda (x y) (< (cdr x) (cdr y))))))))
 
@@ -904,8 +910,11 @@ package-activate-1
       (if (listp package--quickstart-pkgs)
           ;; We're only collecting the set of packages to activate!
           (push pkg-desc package--quickstart-pkgs)
-        (when reload
-          (package--reload-previously-loaded pkg-desc))
+        (when (or reload (assq name package--builtin-versions))
+          (package--reload-previously-loaded
+           pkg-desc (unless reload
+                      "Package %S is activated too late.
+The following files have already been loaded: %S")))
         (with-demoted-errors "Error loading autoloads: %s"
           (load (package--autoloads-file-name pkg-desc) nil t)))
       ;; Add info node.

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Wed, 19 Jun 2024 16:35:56 -0400
> 
> -(defun package--reload-previously-loaded (pkg-desc)
> +(defun package--reload-previously-loaded (pkg-desc &optional warn)
>    "Force reimportation of files in PKG-DESC already present in `load-history'.
>  New editions of files contain macro definitions and
>  redefinitions, the overlooking of which would cause
> -byte-compilation of the new package to fail."
> +byte-compilation of the new package to fail.
> +If WARN is a string, display a warning (using WARN as a format string)
> +before reloading the files."

You say that WARN is treated as a format string, but that begs the
questions: what are the arguments to be formatted with that string?

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Right.
>> Maybe org-loaddefs.el should (provide 'org-autoloads)? And same for
>> other built-in packages. Or there might be a token pkg-autoloads.el in
>> the tree for built-in packages.
>
> Maybe we can use `package-builtin-versions`?
> How 'bout a patch like the one below?

I just tested it a bit by applying onto Emacs master and editing the
built-in Org version to be 9.7.4-pre (this was necessary because Org
mode release is currently the same on ELPA and on master).

Then, I did
1. emacs -Q and install Org mode into .emacs.d/elpa
2. emacs -Q
3. M-x org-version (get 9.7.4-pre)
4. M-: (package-activate-all)
5. I saw

⛔ Warning (package): Package org is activated too late.
The following files have already been loaded: ("/home/yantar92/.emacs.d/elpa/org-9.7.4/org" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-version" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-table" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-src" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-pcomplete" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-macs" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-macro" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-loaddefs" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-list" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-keys" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-footnote" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-fold" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-fold-core" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-faces" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-entities" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-cycle" "/home/yantar92/.emacs.d/elpa/org-9.7.4/org-compat" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ol" "/home/yantar92/.emacs.d/elpa/org-9.7.4/oc" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-tangle" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-table" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-ref" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-lob" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-exp" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-eval" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-emacs-lisp" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-core" "/home/yantar92/.emacs.d/elpa/org-9.7.4/ob-comint")

which looks right

6. M-x org-version now reports 9.7.4
7. M-: (org-check-version) does not complain either

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> -(defun package--reload-previously-loaded (pkg-desc)
>> +(defun package--reload-previously-loaded (pkg-desc &optional warn)
>>    "Force reimportation of files in PKG-DESC already present in `load-history'.
>>  New editions of files contain macro definitions and
>>  redefinitions, the overlooking of which would cause
>> -byte-compilation of the new package to fail."
>> +byte-compilation of the new package to fail.
>> +If WARN is a string, display a warning (using WARN as a format string)
>> +before reloading the files."
>
> You say that WARN is treated as a format string, but that begs the
> questions: what are the arguments to be formatted with that string?

Stefan, do you have any comments? It seems to be the only blocker.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Stefan Monnier]

>>> -(defun package--reload-previously-loaded (pkg-desc)
>>> +(defun package--reload-previously-loaded (pkg-desc &optional warn)
>>>    "Force reimportation of files in PKG-DESC already present in `load-history'.
>>>  New editions of files contain macro definitions and
>>>  redefinitions, the overlooking of which would cause
>>> -byte-compilation of the new package to fail."
>>> +byte-compilation of the new package to fail.
>>> +If WARN is a string, display a warning (using WARN as a format string)
>>> +before reloading the files."
>>
>> You say that WARN is treated as a format string, but that begs the
>> questions: what are the arguments to be formatted with that string?
>
> Stefan, do you have any comments? It seems to be the only blocker.

It's an internal function, so I think the docstring is verbose
enough already.


        Stefan

Re: pkg-autoloads.el vs. pkg-loaddefs.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 13 Aug 2024 20:31:45 -0400
> 
> >>> -(defun package--reload-previously-loaded (pkg-desc)
> >>> +(defun package--reload-previously-loaded (pkg-desc &optional warn)
> >>>    "Force reimportation of files in PKG-DESC already present in `load-history'.
> >>>  New editions of files contain macro definitions and
> >>>  redefinitions, the overlooking of which would cause
> >>> -byte-compilation of the new package to fail."
> >>> +byte-compilation of the new package to fail.
> >>> +If WARN is a string, display a warning (using WARN as a format string)
> >>> +before reloading the files."
> >>
> >> You say that WARN is treated as a format string, but that begs the
> >> questions: what are the arguments to be formatted with that string?
> >
> > Stefan, do you have any comments? It seems to be the only blocker.
> 
> It's an internal function, so I think the docstring is verbose
> enough already.

I agree, but since we've went to the trouble of having a doc string,
why not add a few more words to clarify it?