bug#62720: 29.0.60; Not easy at all to upgrade :core packages like Eglot

[Dmitry Gutov]

On 25/04/2023 10:47, Eli Zaretskii wrote:
> > Date: Tue, 25 Apr 2023 02:45:46 +0300
> > Cc: jporterbugs@gmail.com, philipk@posteo.net, 62720@debbugs.gnu.org,
> >   joaotavora@gmail.com, larsi@gnus.org, monnier@iro.umontreal.ca
> > From: Dmitry Gutov <dmitry@gutov.dev>
> >
> > > > So what are we guarding against here? That the user will choose a
> > > > built-in package to upgrade by accident?
> > >
> > > Yes.  Also, against invocations of this command from other commands
> > > and from the menu.
> >
> > It's not in the menu.
>
> There's this:
>
>    <menu-bar> <package> <Mark All Available Upgrades> runs the command
>    package-menu-mark-upgrades (found in package-menu-mode-map), which is
>    an interactive byte-compiled Lisp function in ‘package.el’.
>
>    It is bound to U.
>
>    (package-menu-mark-upgrades)
>
>    Mark all upgradable packages in the Package Menu.
>    For each installed package with a newer version available, place
>    an (I)nstall flag on the available version and a (D)elete flag on
>    the installed version.  A subsequent x
>    call will upgrade the package.

package-menu-mark-upgrades does not delegate to package-update, for 
better or worse.

As discussed previously, if we're going to change its behavior (of 
package-menu-mark-upgrades), we *will* hide that behind the pref.

> I also envision that we will at some point have an "upgrade" menu
> item, because it make little sense to have this command, but not to be
> able to invoke it from the menu.

We might. But since that will only happen in the future, by that time 
there won't be any users who are accustomed to using that menu item and 
thus possibly inconvenienced by the change in behavior.

> > Very well, here's the next version. It adds a new optional argument to
> > the function (so that people can evaluate e.g. (package-update 'eglot
> > t)). When called interactively, it is determined by current-prefix-argument.
>
> Thanks, this is very close to what I had in mind.  The only thing that
> is missing is the support for user option, which should then avoid the
> need to invoke the command with a prefix argument.
>
> > Also please review the docstring change.
>
> It looks OK to me.
>
> > -(defun package-update (name)
> > -  "Update package NAME if a newer version exists."
> > +(defun package-update (name &optional update-built-ins)
> > +  "Update package NAME if a newer version exists.
> > +
> > +Only packages installed from ELPA are allowed to be updated this
> > +way.
>
> I'm not sure I understand where this restriction comes from.  Did the
> original code enforce it?

I'm not sure what you mean about "enforce it". That's the essence of the 
bug here: this function's inability to upgrade built-in packages 
(packages installed not from ELPA). Since you are asking to keep that 
behavior by default, it now needs to be documented.

>> Regarding obeying package-install-upgrade-built-in, I think it would
>> need to be renamed, and both package-update-all and
>> package-menu-mark-upgrades would need to be made obey it too. All that
>> could be done in a subsequent change.
>
> If the option will affect more than just package-install, it should
> indeed be renamed.

That will require some more work. On package-menu-mark-upgrades in 
particular.

TBH, I'm getting more doubts about this change now.

What will we do in Emacs 30? If we add the new argument, it will be hard 
to back out of it, to default to the proper behavior.

Perhaps we should just wait and then fix it on master properly. 
Workarounds exist, after all.