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

[Dmitry Gutov]

On 21/04/2023 14:05, Eli Zaretskii wrote:

> > > And at least
> > > João (and I think others as well) expected it to upgrade Eglot even
> > > though it is now built in.
> >
> > I think he wants that because this way (package-install 'eglot) and
> > (use-package eglot :ensure t) could match the behavior of Emacs 28 with
> > an empty init directory. Backward compatibility and all that.
>
> But if, with older Emacsen, package-install would refuse to update to
> a newer version of Eglot if _some_ older version of Eglot is already
> installed, then where's the problem with the default behavior of
> package-install? it behaves exactly like in previous versions of
> Emacs.  And why is this a problem for users of Eglot, if they couldn't
> use package-install more than once for Eglot anyway?
>
> Something is amiss here.

The above scenarios (package-install or (use-package eglot :ensure t)) 
when ~/.emacs.d/elpa is empty, result in the very latest Eglot being 
installed when using Emacs 28. And will result in something different 
with Emacs 29 (not the latest version). That's not nothing: the CI 
scripts will have to be updated, and the user instructions for reporting 
problems will have to be made more complicated as well. Some 
possibilities will simply be gone (the user won't be able to upgrade 
Eglot to the very latest by deleting it from ~/.emacs.d/elpa and calling 
package-install, for example). This *is* a non-backward compatible 
change from the perspective of Eglot's maintainer.

I, personally, don't really buy this kind of argument, but I figured you 
might. After all, it's rather in line with reasoning we've seen voiced 
around these parts many times ("X has worked this way for Y years, let's 
never change it from now on"). Classic https://xkcd.com/1172/.

> > But I think that's questionable, semantically. Given that Eglot is
> > already "installed". Though, of course, one could argue that a bundled
> > package is not exactly installed, but then we should change what
> > 'package-installed-p' does as well. And think hard before doing that.
>
> I'd question why we have two commands instead of just one, but that's
> probably water under the bridge at this point.

Either way would be fine, IMO, as long as the behavior is logical and 
matches documentation. But having a separate command to upgrade now lets 
us fix it separately without worry of breaking something more globally.

> > > > > > - package-upgrade (nee package-update) doesn't upgrade builtin packages
> > > > > > that never been upgraded before. It's a bug. Hopefully not too hard to fix.
> > > > >
> > > > > I'm okay with adding the same prefix argument to package-upgrade,
> > > > > which would then allow upgrading a built-in package.  IOW, a change
> > > > > similar to what we did in package-install -- provided that the change
> > > > > is safe enough to go into Emacs 29.
> > > >
> > > > If we agree it's a bug, why don't we just fix it?
> > >
> > > Precisely because, as with package-install, this is a bug for some and
> > > a feature for others, depending on whether people do or don't want the
> > > built-in packages to be upgraded by default.
> >
> > I'm having a hard time imagining someone evaluating (package-upgrade
> > 'eglot) without intention to upgrade it to the latest version. Or
> > invoking it interactively with same argument, expecting a different result.
>
> In interactive invocation, package-upgrade calls completing-read with
> its 4th argument non-nil, so you cannot select a package which is not
> in the collection returned by package--updateable-packages.  What I
> meant above is to allow that collection to include built-in packages
> as optional behavior.  I just tried invoking package-update for ElDoc,
> and I get "No match" after typing "eldoc" to its prompt, although
> eldoc version 1.14.0 is in the list presented by list-packages as
> "available".

That's what I imagined: adding a new optional argument to 
package--updateable-packages which would include builtins in the result.

And only pass it when called from package-upgrade.

Hopefully that's the kind of optional that you meant.

> > > > 'package-update' is an
> > > > interactive function which itself it called from only one place:
> > > > 'package-update-all', and since the plan is to improve both, we can make
> > > > sure they only do what we ask of them: package-update will upgrade
> > > > builtins when invoked directly, and package-update-all will upgrade them
> > > > only when the builtin has been upgraded before (making it not a builtin
> > > > anymore), or a new user option is set.
> > >
> > > This is one possibility, and it might make sense to some users.  But I
> > > don't think we can be sure it will make sense to an overwhelming
> > > majority of the users.
> >
> > Hence the user option?
>
> Which one?  Are you suggesting to add a new one?  If so, why not use
> the one we already added, package-install-upgrade-built-in?

The user option I was thinking of would probably be called a little 
shorter: package-upgrade-built-in. And it would only affect the upgrader 
commands.

I'm not sure tying that optional behavior to the new optional behavior 
of package-install is a good idea (one upgrades, another installs). But 
I'm also not sure keeping the latter around is a good idea, as you know.

> > But okay, this particular addition, though trivial, we could probably
> > postpone until Emacs 30, or even avoid adding at all. It is indeed not
> > obvious that people will really need it, although
>
> If by "this particular addition" you mean to allow package-update to
> update built-in packages, then I thought adding that for consistency
> with package-install was one of your main bothers?  Or what am I
> missing?

That particular addition would be the option package-upgrade-built-in 
proposed above. One that would affect package-upgrade-all and 
package-menu-mark-upgrades only.

So the plan minimum is to fix package-upgrade.

> >     (setq package-upgradable-builtins '(eglot use-package))
> >     (package-upgrade-all)
> >     ;; or M-x package-upgrade-all
> >     ;; or 'U' in the list-packages menu
> >
> > seems like a plausible scenario for a certain kind of user.
>
> Why not treat the fact that some version was already installed from
> ELPA as an indication that the user wants this?

That's how it works now, indeed. And we _could_ leave it at that.

> > Because
> > package-upgrade does not have a menu entry, or a button anywhere,
> > whereas package-upgradable-builtins can be altered from the Customize UI.
>
> Maybe marking a package in the list for update could be interpreted as
> "upgrade that, no questions asked", and we will need no user options?

There is no handy "upgrade that" binding in the packages menu. The only 
command that's available there related to upgrading is 
package-menu-mark-upgrades, which does that to all packages (except 
builtins).

To manually execute an upgrade of one package, one needs to both mark 
the new version for installation (after first scrolling down the list to 
find it), and mark the current version for deletion. This is what 
currently an upgrade consists of. If they just do the first step, they 
end up with two installed versions, one of them "obsolete". package.el 
will hopefully handle this fine during activation after restart 
(ignoring the older one), but it's not a great configuration to leave 
the user in.

All this is to say, the first step (upgrading Eglot to the version from 
ELPA) will be less user-friendly compared to the other UIs we have. But 
it's probably manageable, especially if documented well.

> > > > > We could make it do that if
> > > > > package-install-upgrade-built-in is non-nil -- again, if such a change
> > > > > could be safe enough.  If not, then the same workaround as for
> > > > > package-upgrade would do here, I suppose?
> > > >
> > > > What workaround would that be? use-package is not invoked interactively
> > > > -- there is no prefix argument to pass.
> > >
> > > The workaround is to manually install the package from ELPA, once,
> > > using "C-u M-x package-install RET".
> >
> > That's not the use-package workflow.
>
> The use-package workflow should perhaps get a separate and different
> solution.

Possibly (I suppose people are welcome to submit related proposals -- 
the only reasonable one I heard is related to pinning, see previous msgs).

My point here, however, is that commit 580d8278c5f48 improved the 
situation to a lesser degree that some people here might have expected.

> > > > It could continue doing what it's done before: when a package is already
> > > > installed, abort. For upgrading, we should recommend commands with
> > > > "upgrade" in their names.
> > >
> > > If people agree with that, I don't think I'll object.  But this is in
> > > a sense a breaking change: package-install will only install, and
> > > thereafter users will need to use package-upgrade.  Some might dislike
> > > such behavior changes.  And we will need to make sure that all the
> > > available methods of "installing" do not "upgrade", for consistency.
> >
> > Yeah, apparently it won't be a breaking change, or a change at all.
>
> I'm not sure, see above.
>
> Also, when you mark packages for update from the list presented by
> list-packages, the menu entry says
>
>    i    Mark for Install
>
> and its help-echo says "Mark a package for installation and move to
> the next line", so we already confuse "install" and "upgrade".

But that's the thing: we don't have a separate action in the packages 
list to "upgrade" a single package. The bindings that you see in the 
menu are "Mark for [i]nstall" and "Mark for [d]eletion". The first marks 
a specific (usually newer) version of a package for installation without 
deleting an existing one. The second marks a version for deletion. 
Neither means "upgrade".

The fact that having multiple versions installed will (probably) result 
in the newest one being used after a restart is great, of course, but 
that's still not a proper upgrade.

But we can conclude that we do have two different installation actions:

 - package-install which will install the latest version of a package, 
but only if it's not installed;
 - package-menu-mark-install, which will mark a specific version for 
installation, disregarding whether some earlier version is already 
installed; the previous version will remain installed still.

> > > > > > For this and other minor reasons I would suggest reverting
> > > > > > 580d8278c5f48.
> > > > >
> > > > > Not going to happen, not unless someone comes up with a better
> > > > > solution that is much better and still safe enough.  Personally, I
> > > > > don't believe such a solution exists, since we don't really know the
> > > > > answer to the above question.
> > > >
> > > > Could you specify which problem it's currently solving? Some particular
> > > > scenario.
> > >
> > > The scenario which started this bug report, see the message whose URL
> > > I mentioned above.  IOW, we now allow users to explicitly request that
> > > package-install includes built-in packages in the list of candidates,
> > > and will therefore allow to upgrade them.
> >
> > After we fix 'package-upgrade', users will be able to 'M-x
> > package-upgrade RET eglot RET'.
>
> This goes back to the issue of having two confusingly-similar but
> different commands, as mentioned above.  I guess we should first make
> up our minds what, if anything, we want to do about this.

Sure.

But even if we decide that we want to eliminate that split, doing *that* 
would really be a breaking change. I don't have a reasonable plan to 
present for doing that in Emacs 29, so far.