Slightly simplify partial clone user experience

This was something discussed briefly at the contributor summit: users will have a hard time remembering git clone --filter=blob:none <url>. This series simply adds a --partial option that is equivalent to --filter=blob:none, with the ability to specify a size using --partial=<size> that is equivalent to --filter=blob:limit=<size>.

While going to the git clone documentation to add this option, I noticed that there is no reference to the --filter option there, nor any discussion of partial clone. I resolved the former.

REQUEST FOR HELP: If anyone out there is looking for an opportunity to contribute documentation, I feel there is a need to include a section in the git clone documentation devoted to partial clone. Hopefully such a section would include answers to these questions:

1. What is a partial clone?
2. Why would I want a partial clone?
3. How do I need to be careful when using a partial clone?

I don't have time to write such a document right now, which is why I'm asking for help if anyone is looking for a non-code way to contribute to the project.

Thanks, -Stolee

Derrick Stolee (2):
partial-clone: set default filter with --partial
clone: document --partial and --filter options

 Documentation/git-clone.txt   | 15 ++++++++++++-
 list-objects-filter-options.c | 18 +++++++++++++++
 list-objects-filter-options.h |  8 ++++++-
 t/t5616-partial-clone.sh      | 42 ++++++++++++++++++++++++++---------
 4 files changed, 71 insertions(+), 12 deletions(-)

base-commit: 6c85aac65fb455af85745130ce35ddae4678db84
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-586%2Fderrickstolee%2Fpartial-clone-ux-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-586/derrickstolee/partial-clone-ux-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/586

gitgitgadget

From: Derrick Stolee <[email protected]>

Partial clone means a lot of different things, including filtering out all blobs, large blobs, or objects along a certain pathspec. The pathspec option has limited uses, in particular due to the performance challenges in serving with such filters. The blob size option can be helpful for repositories with a small number of large binaries, but otherwise it is difficult to find a meaningful split between "small" and "large" blobs.

When I think of or recommend partial clone, I specifically mention the case of filtering out all blobs, and downloading those blobs only as needed.

This case is extremely useful, since it takes the best part of shallow clone (a very small initial download) without any of the downsides of restricted history.

However, the command-line interface can be confusing:

	git clone --filter=blob:none <url>

Add a simpler "--partial" option that defaults to this case:

	git clone --partial <url>

This should make the feature more discoverable. However, there is a significant interest in the size-limited filters as that behaves very similarly to Git LFS. For those cases, the following is available:

	git clone --partial=<size> <url>

There are quite a few commands using OPT_PARSE_LIST_OBJECTS_FILTER, includeing clone, fetch, and pack-objects. Augment this macro to include the "--partial[=<size>]" mode for free.

Modify the first partial clone test that checks --filter=blob:none to also check --partial with the same expected conditions after clone. The diff is much simpler to see when ignoring whitespace, since several lines added a leading tab. This test is essentially copied to include the two ways to specify a blob size limit of one byte.

---
 list-objects-filter-options.c | 18 +++++++++++++++
 list-objects-filter-options.h |  8 ++++++-
 t/t5616-partial-clone.sh      | 42 ++++++++++++++++++++++++++---------
 3 files changed, 57 insertions(+), 11 deletions(-)

index 256bcfbdfe6..a71716ef75e 100644
@@ -270,6 +270,24 @@ int opt_parse_list_objects_filter(const struct option *opt,
 	return 0;
 }
 
+int opt_set_blob_none_filter(const struct option *opt,
+			     const char *arg, int unset)
+{
+	struct strbuf filter_arg = STRBUF_INIT;
+	struct list_objects_filter_options *filter_options = opt->value;
+	
+	if (unset || !arg || !strcmp(arg, "0")) {
+		parse_list_objects_filter(filter_options, "blob:none");
+		return 0;
+	}
+	
+	strbuf_addf(&filter_arg, "blob:limit=%s", arg);
+	parse_list_objects_filter(filter_options, filter_arg.buf);
+	strbuf_release(&filter_arg);
+
+	return 0;
+}
+
 const char *list_objects_filter_spec(struct list_objects_filter_options *filter)
 {
 	if (!filter->filter_spec.nr)

index 2ffb39222c4..ac38ffcbe86 100644
@@ -62,6 +62,7 @@ struct list_objects_filter_options {
 
 /* Normalized command line arguments */
 #define CL_ARG__FILTER "filter"
+#define CL_ARG__PARTIAL "partial"
 
 void list_objects_filter_die_if_populated(
 	struct list_objects_filter_options *filter_options);

@@ -80,11 +81,16 @@ void parse_list_objects_filter(
 
 int opt_parse_list_objects_filter(const struct option *opt,
 				  const char *arg, int unset);
+int opt_set_blob_none_filter(const struct option *opt,
+			     const char *arg, int unset);
 
 #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \
 	{ OPTION_CALLBACK, 0, CL_ARG__FILTER, fo, N_("args"), \
 	  N_("object filtering"), 0, \
-	  opt_parse_list_objects_filter }
+	  opt_parse_list_objects_filter }, \
+	{ OPTION_CALLBACK, 0, CL_ARG__PARTIAL, fo, N_("size"), \
+	  N_("partial clone with blob filter"), \
+	  PARSE_OPT_OPTARG | PARSE_OPT_NONEG , opt_set_blob_none_filter }
 
 /*
  * Translates abbreviated numbers in the filter's filter_spec into their

gitgitgadget

I can do that.

I should have been more careful about the use of "unset" (which would never be true with the current option parsing).

This is a better organization and I will use it.

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.

You're right. I'm being inconsistent. Your reasons above point to a good reason to have --no-partial available.

Thanks,
-Stolee

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

index 256bcfbdfe..0225b61912 100644
@@ -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

From: Derrick Stolee <[email protected]>

The previous change added the "--partial[=<size>]" option to "git clone" equivalent to "--filter=blob:none" or "--filter=blob:limit=<size>" but did not document that addition. It turns out that the "--filter=<filter-spec>" option was not documented anywhere in the "git clone" page, and instead is detailed carefully in "git rev-list" where it serves a different purpose.

Add a small bit about these options in the documentation. It would be worth some time to create a subsection in the "git clone" documentation about partial clone as a concept and how it can be a surprising experience. For example, "git checkout" will likely trigger a pack download.

---
 Documentation/git-clone.txt | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

index bf24f1813ad..eafa1c39927 100644
@@ -15,7 +15,8 @@ SYNOPSIS
 	  [--dissociate] [--separate-git-dir <git dir>]
 	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
 	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--] <repository>
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--partial[=<size>]|--filter=<filter>] [--] <repository>
 	  [<directory>]
 
 DESCRIPTION

@@ -162,6 +163,18 @@ objects from the source repository into a pack in the cloned repository.
 	of the repository. The sparse-checkout file can be
 	modified to grow the working directory as needed.
 
+--partial[=<size>]::
+--filter=<filter-spec>::
+	Use the partial clone feature and request that the server sends
+	a subset of reachable objects according to a given object filter.
+	When using `--filter`, the supplied `<filter-spec>` is used for
+	the partial clone filter. When using `--partial` with no `<size>`,
+	the `blob:none` filter is applied to filter all blobs. When using
+	`--partial=<size>` the `blob:limit=<size>` filter is applied to
+	filter all blobs with size larger than `<size>`. For more details
+	on filter specifications, see the `--filter` option in
+	linkgit:git-rev-list[1].
+
 --mirror::
 	Set up a mirror of the source repository.  This implies `--bare`.
 	Compared to `--bare`, `--mirror` not only maps local branches of the

gitgitgadget

I reached out on Twitter to see if anyone was interested, and I got a response. I'm working with them to get started on the mailing list. I'll send a proper introduction when I know which email address they plan to use on the list.

Thanks,
-Stolee

I have mixed feelings on this. I do like making things less arcane for users. But are we locking in a behavior for --partial that we might not want to live with forever? I.e., the current thinking for partial clones is to fetch no blobs at all, get all commits and trees, apply sparse filters, and then fault in the blobs we need. But imagine we later grow the ability to easily avoid fetching all of the trees. Would we regret having the simple name "--partial" taken?

-Peff

I agree with that. Something like "--filter-blobs" for "--filter=blob:none" and perhaps "--filter-blobs=<size>" for "--filter=blob:limit=<size>" might be worth it though.

Thanks for the perspective on this. The --filter-blobs[=<size>] should be less likely to collide with an alternative definition of "partial".

While we are thinking in this space, what if we had a "partial-clone" builtin? It could be a light wrapper around "git clone" where

git partial-clone [--limit=<size>] [options] <url> [<dir>]

would do the same thing as

  git clone --filter=blob:[none|limit=<size>] [options] <url> [<dir>]

Just spit-balling here.

In the meantime, I'll work to adjust my patches to only be the documentation of the --filter option.

Thanks,
-Stolee

Yes, though I wonder if it's really that big an improvement over "--filter=blob:none".

I think that just shifts my concern out to the other command; i.e., would we later want the partial-clone command to behave differently. And introducing a wrapper creates a weird non-orthogonality where it's not necessary. I _do_ think a wrapper could make sense when it's doing substantially different things, or multiple steps that don't belong in the original program (e.g., the way Scalar works).

Thanks, that seems like an obvious and easy improvement.

-Peff

I think that this is probably my fault :).

I was saying at the contributor summit that I think I may have created more confusion than clarity when I was describing how to use partial clones in my last release blog post.

In that post, I introduced partial clones by asking and answering the following series of four questions.

1. What do you do when your repository is too large? Partial clones.
2. How do you only fetch down some of the objects? Object filters.
3. How do you restrict which parts of your working copy are populated? Write to '.git/info/sparse-checkout'.
4. Simplify things even further by using 'git sparse-checkout' instead.

I think that some of the criticism was along the lines of "you said that partial clones would be easy, but nothing in the steps 2-4 is straightforward".

I think that things would have been much clearer if I said:

If your repository is too large to clone all at once, or has a checkout footprint that is too big, you can issue a partial clone against it by writing the following:

    $ git clone --filter=blob:none --sparse /path/to/your/repo

I think that I have mixed feelings about this, too. On the one hand, it irrefutably makes the above incantation a lot easier to write out and remember. On the other hand, I worry about it locking in options that we don't want to keep around forever, like adding more filters, changing some rules about sparse checkout and so on.

Maybe these concerns aren't well-founded, and that we're not likely to change either of those anytime soon. But, I'd rather be wrong and not be squatting '--partial[=<n>]' than not.

This seems like a worthy goal outside of this series, and I would certainly love to have some more documentation in this area.

Thanks,
Taylor

From: Derrick Stolee <[email protected]>

It turns out that the "--filter=<filter-spec>" option is not documented anywhere in the "git clone" page, and instead is detailed carefully in "git rev-list" where it serves a different purpose.

Add a small bit about this option in the documentation. It would be worth some time to create a subsection in the "git clone" documentation about partial clone as a concept and how it can be a surprising experience. For example, "git checkout" will likely trigger a pack download.

---
Slightly simplify partial clone user experience

V2: Only update the documentation of --filter.

Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-586%2Fderrickstolee%2Fpartial-clone-ux-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-586/derrickstolee/partial-clone-ux-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/586

Range-diff vs v1:

 1:  6f340d9aadf < -:  ----------- partial-clone: set default filter with --partial
 2:  9baf4c8ba38 ! 1:  a55c2d975ab clone: document --partial and --filter options

@@ -1,16 +1,13 @@
Author: Derrick Stolee <[email protected]>

- clone: document --partial and --filter options
+ clone: document --filter options

- The previous change added the "--partial[=<size>]" option to
- "git clone" equivalent to "--filter=blob:none" or
- "--filter=blob:limit=<size>" but did not document that addition.
- It turns out that the "--filter=<filter-spec>" option was not
+ It turns out that the "--filter=<filter-spec>" option is not
documented anywhere in the "git clone" page, and instead is detailed carefully in "git rev-list" where it serves a different purpose.

- Add a small bit about these options in the documentation. It
+ Add a small bit about this option in the documentation. It
would be worth some time to create a subsection in the "git clone" documentation about partial clone as a concept and how it can be a surprising experience. For example, "git checkout" will likely
@@ -27,7 +24,7 @@
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
- [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--] <repository>
+ [--[no-]remote-submodules] [--jobs <n>] [--sparse]
-+ [--partial[=<size>]|--filter=<filter>] [--] <repository>
++ [--filter=<filter>] [--] <repository>
[<directory>]

DESCRIPTION
@@ -35,17 +32,15 @@
of the repository. The sparse-checkout file can be modified to grow the working directory as needed.

-+--partial[=<size>]::
+--filter=<filter-spec>::
+ Use the partial clone feature and request that the server sends
+ a subset of reachable objects according to a given object filter.
+ When using --filter, the supplied <filter-spec> is used for
-+ the partial clone filter. When using --partial with no <size>,
-+ the blob:none filter is applied to filter all blobs. When using
-+ --partial=<size> the blob:limit=<size> filter is applied to
-+ filter all blobs with size larger than <size>. For more details
-+ on filter specifications, see the --filter option in
-+ linkgit:git-rev-list[1].
++ the partial clone filter. For example, --filter=blob:none will
++ filter out all blobs (file contents) until needed by Git. Also,
++ --filter=blob:limit=<size> will filter out all blobs of size
++ at least <size>. For more details on filter specifications, see
++ the --filter option in linkgit:git-rev-list[1].
+
--mirror::
Set up a mirror of the source repository. This implies --bare.

 Documentation/git-clone.txt | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

index bf24f1813ad..08d6045c4a8 100644
@@ -15,7 +15,8 @@ SYNOPSIS
 	  [--dissociate] [--separate-git-dir <git dir>]
 	  [--depth <depth>] [--[no-]single-branch] [--no-tags]
 	  [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
-	  [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--] <repository>
+	  [--[no-]remote-submodules] [--jobs <n>] [--sparse]
+	  [--filter=<filter>] [--] <repository>
 	  [<directory>]
 
 DESCRIPTION

@@ -162,6 +163,16 @@ objects from the source repository into a pack in the cloned repository.
 	of the repository. The sparse-checkout file can be
 	modified to grow the working directory as needed.
 
+--filter=<filter-spec>::
+	Use the partial clone feature and request that the server sends
+	a subset of reachable objects according to a given object filter.
+	When using `--filter`, the supplied `<filter-spec>` is used for
+	the partial clone filter. For example, `--filter=blob:none` will
+	filter out all blobs (file contents) until needed by Git. Also,
+	`--filter=blob:limit=<size>` will filter out all blobs of size
+	at least `<size>`. For more details on filter specifications, see
+	the `--filter` option in linkgit:git-rev-list[1].
+
 --mirror::
 	Set up a mirror of the source repository.  This implies `--bare`.
 	Compared to `--bare`, `--mirror` not only maps local branches of the

base-commit: 6c85aac65fb455af85745130ce35ddae4678db84

gitgitgadget

Thanks, this part is definitely an improvement (and I read over the proposed text again, and it looks very good).

-Peff

[...]

Thanks.

I think that belongs in its own git-partial-clone(7) page.

Thanks,
Jonathan