On Fri, Aug 17, 2018 at 3:28 PM Stefan Beller <sbel...@google.com> wrote: > > On Fri, Aug 17, 2018 at 3:20 PM Matthew DeVore <matv...@google.com> wrote: > > > > On Fri, Aug 17, 2018 at 2:42 PM Stefan Beller <sbel...@google.com> wrote: > > > > > > On Wed, Aug 15, 2018 at 4:23 PM Matthew DeVore <matv...@google.com> wrote: > > > > > > > > Teach list-objects the "tree:0" filter which allows for filtering > > > > out all tree and blob objects (unless other objects are explicitly > > > > specified by the user). The purpose of this patch is to allow smaller > > > > partial clones. > > > > > > > > The name of this filter - tree:0 - does not explicitly specify that > > > > it also filters out all blobs, but this should not cause much confusion > > > > because blobs are not at all useful without the trees that refer to > > > > them. > > > > > > > > I also consider only:commits as a name, but this is inaccurate because > > > > it suggests that annotated tags are omitted, but actually they are > > > > included. > > > > > > Speaking of tag objects, it is possible to tag anything, including blobs. > > > Would a blob that is tagged (hence reachable without a tree) be not > > > filtered by tree:0 (or in the future any deeper depth) ? > > I think so. If I try to fetch a tagged tree or blob, it should fetch > > that object itself, since I'm referring to it explicitly in the git > > pack-objects arguments (I mention fetch since git rev-list apparently > > doesn't support specifying non-commits on the command line). This is > > similar to how I can fetch a commit that would otherwise be filtered > > *if* I specify it explicitly (rather than a child commit). > > > > If you're fetching a tagged tree, then for depth=0, it will fetch the > > given tree only, and not fetch any referents of an explicitly-given > > tree. For depth=1, it will fetch all direct referents. > > > > If you're fetching a commit, then for depth=0, you will not get any > > tree objects, and for depth=1, you'll get only the root tree object > > and none of its referrents. So the commit itself is like a "layer" in > > the depth count. > > That seems smart. Thanks! > > > > > > > > > I found this series a good read, despite my unfamiliarity of the > > > partial cloning. > > > > > > One situation where I scratched my head for a second were previous patches > > > that use "test_line_count = 0 rev_list_err" whereas using > > > test_must_be_empty > > > would be an equally good choice (I am more used to the latter than the > > > former) > > > > Done. Here is an interdiff (sorry, the tab characters are not > > maintained by my mail client): > > heh. Thanks for switching the style; I should have emphasized that > (after reflection) I found them equally good, I am used to one > over the other more. It seems marginally better to me. I also noticed a clean-up patch going by that aggressively switched to test_must_be_empty wherever possible: https://public-inbox.org/git/20180819215725.29001-1-szeder....@gmail.com/
OTOH, if it were up to me I would have just gotten rid of test_must_be_empty and used an existing function with the right argument, like `test_cmp /dev/null` - but using some form consistently is the most important, whatever it is. > > So if that is the only issue brought up, I would not even ask for a resend. > > Thanks, > Stefan
