Re: feature/package+vc 04c4c578c7 3/4: Allow for packages to be installed directly from VACS

[Philip Kaludercic]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
>   > I have pushed a commit to feature/package+vc that will use the release
>   > revision (if it is known) when `package-vc-install' is invoked with a
>   > prefix argument.
>
> It is good to offer both options.  However, there may be a bug in this
> way of doing it.
>
> The words "the release revision" are somewhat terse and I am not
> entirely sure what they mean.  My guess is they mean "the revision in
> the upstream repo that corresponds to the released code in ELPA."  Is
> that correct?

Yes, that is correct.

> In the usual case, the released code in ELPA does correspond to some
> revision in the upstream repo.  Therefore, assuming the release
> revision "is known", this implementation will find the right code.

No, the revision used to create a release is always 

> However, in unusual cases the code in ELPA includes changes that did
> not come from the upstream repo, and may not be present there.  This
> feature needs to DTRT in those cases too.

Ah, now I understand where you are coming from.  As I said in my
previous message, if this is the case and we have a diverged package, we
can always point to elpa.git/nongnu.git.  package-vc.el uses a new file
added to {GNU,NonGNU} ELPA that you can find here

https://elpa.gnu.org/packages/elpa-packages.eld
https://elpa.nongnu.org/nongnu/elpa-packages.eld

These consist of package specifications like

      ("ace-window" :url "https://github.com/abo-abo/ace-window"; :auto-sync t)

for upstream repositories, or
                                                                              
      ("adjust-parens" :url "https://git.sv.gnu.org/git/emacs/elpa.git"; :branch 
"externals/adjust-parens")

for repositories that are to be checked out directly from elpa.git.

> To check out the latest revision in the upstream repo is certainly wrong,
> and it could cause serious confusion.  The command should never do
> that, not even as a fallback.

Fallback from what?  If you think the ELPA mirror ought to always be
used, then why even bother with pointing to the upstream repository?  If
anyone needs the upstream repository, they can always add it themselves.
This would mean that the first package specification I have above would
just become

      ("ace-window" :url "https://git.sv.gnu.org/git/emacs/elpa.git"; :branch 
"externals/ace-window")

The only arguments I see are to reduce the load on Savannah and to avoid
the update-lag for auto-synchronised packages between the point in time
when a commit is pushed to the upstream repository and the point it is
mirrored.

> What other behavior would be a better fallback?
> What is TRT in these cases?
       
> I see two reasonable possibilities:
>
> 1. To signal an error and do nothing.
>
> 2. To check out the current release version from the ELPA repo (NOT
> the upstream repo).
>
> I think #2 is better, but #1 at least avoids confusion.

I believe we are talking about diverged packages, right?  Just to
clarify my previous point, this ought to be detectable on the server
side of things, in which case the mirror is used -- unless we always
decide to use the mirror.  There is no need for special handling in
package-vc.el -- in fact the entire point is unrelated to the branch and
any issue blocking the merging into master.

> In what situations is the release revision not known?
> We should think about what is TRT in those cases too.
> First, what cases are they?

The release revision is the most recent release that bumped the
"Version" header, as described in (elisp) Simple Packages.  If there is
no such header, the package is not released onto ELPA, hence it wouldn't
appear in the package archive to begin with.

This is only an issue for unreleased packages, in which case we cannot
state what the release revision is supposed to be in the first place.
This is handled by the following code right now:

   (if-let ((release-rev (package-vc-release-rev pkg-DECs)))
            (vc-retrieve-tag pkg-dir release-rev)
          (message "No release revision was found, continuing..."))

I fear we might disagree as to how fatal or not this case is.  I think
it is better to complete the installation at all instead of aborting it,
which is why I just emit a message and continue on.  But I do recognise
the point that if someone wants the revision used to release a package,
then we should take that request seriously.