git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Derrick Stolee <stolee@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, me@ttaylorr.com, jonathantanmy@google.com,
	christian.couder@gmail.com, git@jeffhostetler.com,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 1/2] partial-clone: set default filter with --partial
Date: Sun, 22 Mar 2020 05:46:59 -0400	[thread overview]
Message-ID: <20200322094659.GA635598@coredump.intra.peff.net> (raw)
In-Reply-To: <2a75325b-ea50-a6f6-87dc-12184e706ac2@gmail.com>

On Fri, Mar 20, 2020 at 04:38:27PM -0400, Derrick Stolee wrote:

> > Specifically, I find it unsatisifying to see the "0" optimization at
> > this level.  Shouldn't it be done in parse_list_objects_filter() to
> > parse "blob:limit=<num>" and after realizing <num> is zero, pretend
> > as if it got "blob:none" to optimize?  That way, people can even say
> > "--partial=0k" and get it interpreted as "--filter=blob:none", right?
> 
> I suppose it would be worth checking the recent server-side improvements
> to see if they translate a limit=0k to a "size 0" and then ignore the
> size check and simply remove all blobs.

The new bitmap code doesn't do anything special there. It does rely on
the normal filter parsing, though. If we rewrote it at that level,
perhaps like this (completely untested):

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 256bcfbdfe..0225b61912 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -49,7 +49,10 @@ static int gently_parse_list_objects_filter(
 
 	} else if (skip_prefix(arg, "blob:limit=", &v0)) {
 		if (git_parse_ulong(v0, &filter_options->blob_limit_value)) {
-			filter_options->choice = LOFC_BLOB_LIMIT;
+			if (filter_options->blob_limit_value)
+				filter_options->choice = LOFC_BLOB_LIMIT;
+			else
+				filter_options->choice = LOFC_BLOB_NONE;
 			return 0;
 		}
 

then it would just work for regular and bitmapped filters.

One interesting user-visible effect would be in the patches we've been
discussing to limit which filters are allowed. If you allowed, say,
blob:none but not blob:limit, this would quietly allow blob:limit=0
(because it really _is_ blob:none under the hood). 

I'm not sure if that would be confusing or convenient. I doubt anybody
cares much for the blob filters (you'd either enable neither or both,
because they cost about the same). But in another thread, I mentioned
that doing "tree:depth=0" _would_ be cheap to do with bitmaps, but
tree:depth=1" wouldn't. If we quietly rewrote the former to tree:none
(which doesn't yet exist, but could), that would let you distinguish
between the two (of course if tree:none existed, perhaps nobody would
have any reason to write tree:depth=0 in the first place).

-Peff

  reply	other threads:[~2020-03-22  9:47 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 17:28 [PATCH 0/2] Slightly simplify partial clone user experience Derrick Stolee via GitGitGadget
2020-03-19 17:28 ` [PATCH 1/2] partial-clone: set default filter with --partial Derrick Stolee via GitGitGadget
2020-03-20 20:26   ` Junio C Hamano
2020-03-20 20:38     ` Derrick Stolee
2020-03-22  9:46       ` Jeff King [this message]
2020-03-19 17:28 ` [PATCH 2/2] clone: document --partial and --filter options Derrick Stolee via GitGitGadget
2020-03-20 12:27 ` [PATCH 0/2] Slightly simplify partial clone user experience Derrick Stolee
2020-03-22  9:51 ` Jeff King
2020-03-22 10:58   ` Christian Couder
2020-03-22 16:45     ` Derrick Stolee
2020-03-22 19:22       ` Jeff King
2020-03-22 16:03 ` Taylor Blau
2020-03-22 19:50 ` [PATCH v2] clone: document --filter options Derrick Stolee via GitGitGadget
2020-03-24  3:40   ` Jeff King
2020-03-24  5:17   ` Jonathan Nieder

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=20200322094659.GA635598@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=christian.couder@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=me@ttaylorr.com \
    --cc=stolee@gmail.com \
    --subject='Re: [PATCH 1/2] partial-clone: set default filter with --partial' \
    /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

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).