git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/3] merge-base: return fork-point outside reflog

From
Michael J Gruber <git@grubix.eu>
Date
Sep 21, 2017, 09:39 UTC
Message-ID
<5a2fca1d-4edf-965f-4840-58c924c91051@grubix.eu>
In-Reply-To
<xmqqshfgk1mr.fsf@gitster.mtv.corp.google.com>
Junio C Hamano venit, vidit, dixit 21.09.2017 08:27:
Show 26 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> ...  I agree that there is a value in what your patch 2/3
>> wants to do when the current one that is more strict would say
>> "there is no known fork-point"---we would gain a way to say "... but
>> this is the best guess based on available data that may be better
>> than getting no answer." which we lack.
>>
>> Having said all that, I do not agree with your last sentence in the
>> paragraph I quoted above.  It is a mere implementation detail to
>> consult the reflog to find out the set of "historical tips of the
>> Branch"; the current tip by definition is among the commits in that
>> set, even when the reflog of Branch is missing.  What 4f21454b55 did
>> was a reasonable "fix" that is still in line with the definition of
>> "--fork-point" from that point of view.
>>
>> Whether we add a "looser" version of "--fork-point" to the system or
>> not, the more strict version should still use the current tip as one
>> of the historical tips (i.e. those that we would take from the
>> reflog if the reflog were not empty) in the more "strict" mode.  The
>> looser version may also want to do so as well.
> 
> So, should I mark this in What's cooking report as "expecting a
> reroll", anticipating that a new option would be added to trigger
> the new & looser behaviour?
> 

I dunno. Some participants in this thread considered my patch to be a fix rather than alternative behaviour. So I hoped for more responses to your response. (Re-adding dscho on cc - our thread graph forked...)

Also, I'm undecided about about your reflog argument above - if we leave "--fork-point" to be the current behaviour including Jeff's fix then the documentation would need an even bigger overhaul, because it's neither "reflog also" (as claimed in the doc) nor "reflog only" (as in the original implementation) but "historical tips as inferred from the current value and the reflog".

In any case, for two modes we need two names for the options. Maybe --fork-point and --fork-base because in the loose mode, you may get a "base of a strict fork point"?

Michael
Previous: Junio C HamanoNext: Junio C Hamano
Message 12 of 25 in “RE: merge-base not working as expected when base is ahead”
  1. Ekelhart JakobSep 13, 2017
  2. Michael J GruberSep 14, 2017
  3. 0/3 merge-base --fork-point fixesMichael J Gruber, Sep 14, 2017
  4. Johannes SchindelinSep 14, 2017
  5. Jeff KingSep 14, 2017
  6. 2/3 merge-base: return fork-point outside reflogMichael J Gruber, Sep 14, 2017
  7. Junio C HamanoSep 15, 2017
  8. Phillip WoodSep 15, 2017
  9. Michael J GruberSep 15, 2017
  10. Junio C HamanoSep 15, 2017
  11. Junio C HamanoSep 21, 2017
  12. Michael J GruberSep 21, 2017
  13. Junio C HamanoSep 22, 2017
  14. Michael J GruberSep 22, 2017
  15. Junio C HamanoSep 22, 2017
  16. Junio C HamanoOct 3, 2017
  17. Ekelhart JakobNov 8, 2017
  18. Michael J GruberNov 8, 2017
  19. Junio C HamanoNov 9, 2017
  20. 3/3 merge-base: find fork-point outside partial reflogMichael J Gruber, Sep 14, 2017
  21. Jeff KingSep 14, 2017
  22. 1/3 t6010: test actual test outputMichael J Gruber, Sep 14, 2017
  23. Jeff KingSep 14, 2017
  24. Michael J GruberSep 15, 2017
  25. Jeff KingSep 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.