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

Re: [bug] git checkout lies about number of ahead commits when switching from detached HEAD

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 21, 2011, 15:17 UTC
Message-ID
<7v4o6wtsdz.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20110321103549.GA16334@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 9 quoted lines
>   1. B does not use any of the same marks as A. Otherwise, it will end
>      up clearing part of A's result during cleanup (not to mention that
>      it may get a bogus result because of what was left from A).
>
>   2. B works on a totally disjoint set of history from A and C.
>
> I don't think (1) is that realistic in the general case. Sure, you might
> only want to use TMP_MARK. But the revision walker behind the scenes is
> using SEEN and UNINTERESTING, which is going to bring you into conflict.

I suspect I didn't write it clearly enough, but I think we are saying the same thing.

Of course SEEN/UNINTERESTING needs to be reset for any traversal. That is a given for anybody who uses get_revision() to traverse the history. "B knows more about the bit it uses than other people, so it should be able to do a better job of cleaning after it than others" is exactly about (1).

Show 5 quoted lines
> For (2), there are cases where it works. In this recent bug, for
> example, it would sometimes produce the correct results depending on the
> exact traversal done in the orphan-warning (e.g., if you were exploring
> way back in history near a tag, the traversal wouldn't come near the
> commits needed to get the tracking info for the new branch).

And then "B knows more about where in the history it travesed, so it should be able to do a better job" is about (2).

> But you can only know that it's a reasonable thing to do if you manually
> analyze A, B, and C as a set, and even then only if you can know pretty
> specifically what the inputs are.

Oh, that is a given that whoever is inserting B between A and C needs to know about their relationship, and whoever wishes to design B to be reusable in other different contexts needs to know what masks other people may care about not to stomp on them. I had this idea of allocating masks on-demand instead of the current static assignment in the back of my head for a few years, but my gut feeling keeps me telling that the added complexity (and lax use of object bits by coders leading to memory footprint bloat) may not be worth it.

Show 5 quoted lines
> I can change it to UNINTERESTING. As far as I can tell, that is the only
> one set by prepare_revision_walk. It just felt like a leaky abstraction
> for this code to have to know which flags might get set by the
> underlying code. But obviously callers do need to know and care which
> flags might be set and when.

Yes, the code has to know about the bits, and explicitly declaring it knows what it is doing and using is about being assuring, not being leaky.

Previous: Jeff King
Message 28 of 28 in “[bug] git checkout lies about number of ahead commits when switching from detached HEAD”
  1. Piotr KrukowieckiMar 19, 2011
  2. Jeff KingMar 19, 2011
  3. Jeff KingMar 19, 2011
  4. Junio C HamanoMar 20, 2011
  5. Jeff KingMar 20, 2011
  6. 1/3 checkout: add basic tests for detached-orphan warningJeff King, Mar 20, 2011
  7. 2/3 checkout: clear commit marks after detached-orphan checkJeff King, Mar 20, 2011
  8. Junio C HamanoMar 20, 2011
  9. i18n: use test_i18ncmp in t2020 (checkout --detach)Jonathan Nieder, Apr 13, 2012
  10. checkout --detached test: write supporting files before start of testsJonathan Nieder, Apr 13, 2012
  11. Jeff KingApr 13, 2012
  12. Jonathan NiederApr 13, 2012
  13. Jeff KingApr 14, 2012
  14. Jeff KingApr 13, 2012
  15. Jonathan NiederApr 13, 2012
  16. Jeff KingApr 14, 2012
  17. 0/3 Re: i18n: use test_i18ncmp in t2020 (checkout --detach)Jonathan Nieder, Apr 14, 2012
  18. 1/3 test: do not rely on US English tracking-info messagesJonathan Nieder, Apr 14, 2012
  19. 2/3 test: use test_i18ncmp for "Patch format detection failed" messageJonathan Nieder, Apr 14, 2012
  20. 3/3 test: am of empty patch should not succeedJonathan Nieder, Apr 14, 2012
  21. Jeff KingApr 14, 2012
  22. Jonathan NiederApr 14, 2012
  23. Jeff KingApr 14, 2012
  24. Jonathan NiederApr 14, 2012
  25. 3/3 checkout: tweak detached-orphan warning formatJeff King, Mar 20, 2011
  26. Junio C HamanoMar 20, 2011
  27. Jeff KingMar 21, 2011
  28. Junio C HamanoMar 21, 2011

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.