Re: [PATCH v6] add --summary option to git-push and git-fetch

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2010, 17:55 UTC
Message-ID: <7v3a1gdgxq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vwrysdiaq.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I dunno.  If it delegated to a subprocess it would certainly be easier to
> review and get convinced that the change won't affect object flags for
> other parts of the system in bad ways, but there obviously is a
> performance downside.

You fundamentally cannot use the same "summary" logic for push and fetch, and it is especially true if you are doing it inside the same process, I think.

When you force a fetch, you will have the complete histories for both old and new, as you started from old (and I am assuming that you are fsck clean) and you successfully fetched new. When you force a push, however, you may already have the old in your object store, but there is no guarantee that you have the complete history leading to it (i.e. you may have got the tip commit left by an earlier fetch done with a commit walker that you interrupted in the middle).

So at the very least, your "summary_impossible" logic should work a lot harder than a single lookup-commit-reference-gently; it needs to walk the ancestry until you hit some ref to prove that you have a complete history for that commit, without dying. Otherwise get_revision() loop inside print_summary_for_push_or_fetch() would say "oops -- I don't have the parent commit" when it tries to call add_parents_to_list() and die.

Doing the summary traversal inside a subprocess would simplify the handling of the error for such a case, I guess.

My gut feeling from the beginning has been that a patch that touches revision.c for this topic would add unacceptable cruft to the already complex logic in that library for no real gain. Doing the traversal inside a subprocess would allay that worry as well ;-)