Hi Junio,
On 09/21/2020 11:48, Junio C Hamano wrote:
> Srinidhi Kaushik <shrinidhi.kaushik@xxxxxxxxx> writes:
>
> >> If we were talking about older parts of the history, optional
> >> generation numbers could change the equation somewhat, but the
> >> common case for the workflow this series is trying to help is that
> >> these local commits ane the remote tip are relatively new and it is
> >> not unlikely that the generation numbers have not been computed for
> >> them, which is why I suspect that in_merges_many may be a win.
> >
> > Nice! We can definitely try batching commits from the reflog and
> > pass it along to "in_merge_bases_many()". As for being faster than
> > calling "in_merge_bases()" for each commit entry in the reflog --
> > I am not familiar with how the former works. Do we still keep the
> > "reflog_entry_exists()" part? It might still be faster to go through
> > the entries once to check with "oideq()" in the first run.
>
> That is what I meant. You go through local reflog entries until you
> find one that is older than the timestamp of the reflog entry of the
> remote-tracking branch, check with oideq() to see if the tip was ever
> directly checked out. Then, using these same local reflog entries,
> you can make in_merge_bases_many() tranversal to see if any of them
> reach the tip. I suspect that the number of local reflog entries you
> need to examine would not be too many, so if you can put them all in
> a single array of "struct commit *" pointers in the first "oideq()"
> phase, you may be able to do just a single in_merge_bases_many() batch
> to check for the reachability.
Gotcha.
> > Also, I was wondering if it is worth considering this:
> > - check if the reflog of the HEAD has the remote ref
>
> It would help the workflow I had in mind, but it would raise the
> risk of false positives according to Dscho and I tend to agree, so
> I do not know if it is overall a good idea.
Oh, right. This doesn't work when a "git pull --rebase" is run on
a different branch (and a few other cases, as mentioned by Johannes).
> > - check if the reflog of the local branch has the remote ref
>
> Isn't that the oideq() test?
Yes.
> > - check if the remote ref is reachable from any of the local ref's
> > "reflog" entries using "in_merge_bases_many()" in batches as
> > suggested here.
>
> I think it amounts to the same as "does any reflog entry of HEAD
> reach it?" and shares the same issues with false positives as the
> first one.
Hmm, isn't this the same as what was mentioned by you earlier (without
the timestamp:
> [...] Then, using these same local reflog entries,
> you can make in_merge_bases_many() tranversal to see if any of them
> reach the tip.
In v5 (the new patch) [1], the check does this:
- go through the local reflog until it hits an entry with a timestamp
older than the remote commit, and doing an "oideq()" check and
collecting commits into a list along the way.
- if an exact entry was found, the test passes; otherwise use
the commit list and make a call to "in_merge_bases_many()" to
check for reachability, and report it.
> >> > + deletion:1,
> >> > + if_includes:1, /* If "--force-with-includes" was specified. */
> >>
> >> The description needs to be tightened.
> >>
> >> > + unreachable:1; /* For "if_includes"; unreachable in reflog. */
> >
> > OK, you're right. Perhaps, we could rename it to something like
> > "if_includes_for_tracking" and update the comment description
> > with saying something along the lines of:
>
> That is overlong. Let me try:
>
> /* need to check if local reflog reaches the remote tip */
> check_reachable:1,
>
> /* local reflog does not reach the remote tip */
> unreachable:1;
>
I have updated the description in v5 [1]; thanks!
[1]: https://public-inbox.org/git/20200923073022.61293-1-shrinidhi.kaushik@xxxxxxxxx/
Thanks.
--
Srinidhi Kaushik
