From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
Michal Hocko <mhocko@suse.cz>,
git@vger.kernel.org
Subject: Re: git describe --contains doesn't work properly for a commit
Date: Wed, 04 Mar 2015 22:00:08 -0800 [thread overview]
Message-ID: <xmqq385k56hj.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150305051211.GA3344@peff.net> (Jeff King's message of "Thu, 5 Mar 2015 00:12:11 -0500")
Jeff King <peff@peff.net> writes:
> On Wed, Mar 04, 2015 at 12:41:57PM -0800, Junio C Hamano wrote:
>
>
>> This one, and $gmane/264101, are a few instances of this known issue
>> raised here recently.
>
> If $gmane/264101 is caused by clock skew, I'd find that disturbing.
> Those algorithms are supposed to be "correct, but slower" in the face of
> skew, not ever incorrect.
My understanding is that the commit painting done by merge-base is
designed to be always correct, but the A..B revision traversal
depends on SLOP being big enough. When the traversal queue is
filled with all UNINTERESTING commits, some of which needs to be dug
to reveal newer commits that are not yet painted as UNINTERESTING in
order to get the complete picture, the still_interesting() logic
will end up stopping prematurely, leaving commits that are actually
UNINTERESTING in the topological sense unpainted in the resulting
newlist that is assigned to revs->commits in limit_list(), yielding
an incorrect result.
>> > Calculating them is simple. Caching and storage is the bigger question.
>>
>> Yes, also having to handle the ones whose generation numbers haven't
>> been computed yet adds to the complexity.
>
> I'm not sure it's that bad. If you cache generation numbers for all
> known commits when you repack, then worst case you have to traverse all
> commits not in the pack.
> ...
> IMHO, if you are going to go to the trouble to detect and store skew,
> you should just go to the trouble to calculate and store reliable
> generation numbers.
I would actually prefer a solution with generation numbers, of
course, because that would give us provably correct result. If it
can be done without too much hassle, I am all for it. Scrap the
half-baked "I've been wondering" compromise ;-)
Thanks.
prev parent reply other threads:[~2015-03-05 6:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-26 13:35 git describe --contains doesn't work properly for a commit Michal Hocko
2015-02-26 14:23 ` Michal Hocko
2015-03-04 10:54 ` Jeff King
2015-03-04 15:06 ` Michael J Gruber
2015-03-04 18:05 ` Jeff King
2015-03-04 20:41 ` Junio C Hamano
2015-03-04 22:05 ` Mike Hommey
2015-03-05 5:12 ` Jeff King
2015-03-05 6:00 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq385k56hj.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=mhocko@suse.cz \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox