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

[Philip Kaludercic]

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

>> I guess so.  Here is a possible patch that should behave close enough to
>> elpa-admin.el using "blame"
>
> Why do you use `blame` rather than `log` like `elpa-admin.el`?

I tried to use log like in elpa-admin.el, but the issue was I had to
either

1. Define the generic interface to search for a line using a regular
   expression that is not an Elisp regular expression, and could
   conceivably differ between different VCS that might also implement
   'last-change'.

2. Deal with the issue that the Version header might move between
   revisions, meaning that a line range wouldn't do a good job at
   capturing the right commit.

>> (an obvious exception is the missing handler for the new :merge
>> property, but I have been wondering if it might also be fair to always
>> add "--first-parent" for Git).
>
> We considered using `--first-parent` but it tends to point to more
> recent merge commits than to the actual commit that bumped the
> `Version:`.  If you add to that the fact that order of parents in Git is
> somewhat arbitrary and rarely taken into consideration, I'd rather not
> go there.
>
> For `:merge` we didn't really have a choice, and (to make up for that)
> we get to control how the merge is done and thus which one is the
> first parent.

I see.

>> The default handler just wraps vc-annotate, so it is a bit more fragile.
>
> Hmm... the code I see in your patch uses `vc-region-history` (which is
> only supported for Git and Hg, currently, and is fairly difficult to
> support in a generic way) rather than `vc-annotate`.
> Am I missing something?

Uh, that is my mistake.  I started writing that code yesterday (I
believe?) and simply forgot what I had used.  I'll try to translate that
into vc-annotate before pushing anything.

>> Invoking `package-vc-install' with a prefix argument will now check out
>> the specific commit that bumps the version tag.  At least for git, the
>> slight problem here is that this means the head is in a detached state,
>> not connected to any specific branch.  I don't know if there is any
>> elegant solution to this problem, or if it should even be "solved".
>
> I suspect a better option is to use something like `git reset` instead
> of `git checkout`, so we end up at the right revision but still on the
> main branch.  But yes, it's kind of ugly if you want to try and preserve
> local changes.  I think doing it right requires distinguishing whether
> we're moving forward or backward (moving forward can be done with `git
> merge`, which is well-behaved, whereas moving backward is poorly
> supported, AFAICT).

Checking out a specific revision is currently only done right after
cloning, so this is always a reset.  The issue here is that I am trying
to stay generic and was using 'vc-retrieve-tag' (but perhaps
'vc-checkout' would be better), so this is an issue that might have to
be tackled in vc-git.el...

> You might want to take a look at `elpaa--select-revision` where I try to
> solve this problem (for a slightly more restricted case, admittedly, but
> it's already unsatisfactory).
>
> Maybe there's a better way to do that?

Nothing immediate I can think of.