From: Jeff King <peff@peff.net>
To: mhagger@alum.mit.edu
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 00/17] Clean up how fetch_pack() handles the heads list
Date: Thu, 23 Aug 2012 05:26:24 -0400 [thread overview]
Message-ID: <20120823092624.GG6963@sigill.intra.peff.net> (raw)
In-Reply-To: <1345709442-16046-1-git-send-email-mhagger@alum.mit.edu>
On Thu, Aug 23, 2012 at 10:10:25AM +0200, mhagger@alum.mit.edu wrote:
> There were various confusing things (and a couple of bugs) in the way
> that fetch_pack() handled the list (nr_heads, heads) of references to
> be sought from the remote:
Aside from the minor comments I made to individual patches, this all
looks good. As usual, thanks for breaking it down; I wish all series
were as easy to review as this.
> I'm still suspicious about the logic related to args.fetch_all and
> args.depth, but I don't think I've made anything worse.
I think the point of that is that when doing "git fetch-pack --all
--depth=1", the meaning of "--all" is changed from "all refs" to
"everything but tags".
Which I kind of see the point of, because you don't want to grab ancient
tags that will be expensive. But wouldn't it make more sense to limit it
only to the contents of refs/heads in that case? Surely you wouldn't
want refs/notes, refs/remotes, or other hierarchies.
I suspect this code is never even run at all these days. All of the
callers inside git should actually provide a real list of refs, not
"--all". So it is really historical cruft for anybody who calls the
fetch-pack plumbing (I wonder if any third-party callers even exist;
this is such a deep part of the network infrastructure that any sane
scripts would probably just be calling fetch).
-Peff
next prev parent reply other threads:[~2012-08-23 9:26 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-23 8:10 [PATCH 00/17] Clean up how fetch_pack() handles the heads list mhagger
2012-08-23 8:10 ` [PATCH 01/17] t5500: add tests of error output for missing refs mhagger
2012-08-23 8:10 ` [PATCH 02/17] Rename static function fetch_pack() to http_fetch_pack() mhagger
2012-08-23 8:10 ` [PATCH 03/17] Fix formatting mhagger
2012-08-23 8:10 ` [PATCH 04/17] Name local variables more consistently mhagger
2012-08-23 8:39 ` Jeff King
2012-08-24 7:05 ` Michael Haggerty
2012-08-26 17:39 ` Junio C Hamano
2012-08-27 9:22 ` Michael Haggerty
2012-08-27 9:25 ` Jeff King
2012-08-27 16:55 ` Junio C Hamano
2012-08-27 16:50 ` Junio C Hamano
2012-08-23 8:10 ` [PATCH 05/17] Do not check the same match_pos twice mhagger
2012-08-23 8:42 ` Jeff King
2012-08-23 8:10 ` [PATCH 06/17] Let fetch_pack() inform caller about number of unique heads mhagger
2012-08-23 8:54 ` Jeff King
2012-08-25 5:05 ` Michael Haggerty
2012-08-23 8:10 ` [PATCH 07/17] Pass nr_heads to do_pack_ref() by reference mhagger
2012-08-23 8:10 ` [PATCH 08/17] Pass nr_heads to everything_local() " mhagger
2012-08-23 8:10 ` [PATCH 09/17] Pass nr_heads to filter_refs() " mhagger
2012-08-23 8:10 ` [PATCH 10/17] Remove ineffective optimization mhagger
2012-08-23 8:10 ` [PATCH 11/17] filter_refs(): do not leave gaps in return_refs mhagger
2012-08-23 8:10 ` [PATCH 12/17] filter_refs(): compress unmatched refs in heads array mhagger
2012-08-23 8:10 ` [PATCH 13/17] cmd_fetch_pack: return early if finish_connect() returns an error mhagger
2012-08-23 8:10 ` [PATCH 14/17] Report missing refs even if no existing refs were received mhagger
2012-08-23 8:10 ` [PATCH 15/17] cmd_fetch_pack(): simplify computation of return value mhagger
2012-08-23 8:10 ` [PATCH 16/17] fetch_pack(): free matching heads mhagger
2012-08-23 9:04 ` Jeff King
2012-08-23 8:10 ` [PATCH 17/17] fetch_refs(): simplify logic mhagger
2012-08-23 9:07 ` Jeff King
2012-08-25 6:37 ` Michael Haggerty
2012-08-23 9:26 ` Jeff King [this message]
2012-08-23 19:13 ` [PATCH 00/17] Clean up how fetch_pack() handles the heads list Philip Oakley
2012-08-23 19:56 ` Jeff King
2012-08-23 20:31 ` Jeff King
2012-08-25 7:05 ` Michael Haggerty
2012-09-02 7:02 ` Michael Haggerty
2012-08-23 22:09 ` Philip Oakley
2012-08-24 4:23 ` Junio C Hamano
2012-08-24 12:46 ` Philip Oakley
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=20120823092624.GG6963@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
/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;
as well as URLs for NNTP newsgroup(s).