git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/5] backfill: basic functionality and tests

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 16, 2024, 08:01 UTC
Message-ID
<Z1_eUSGN3B13uGmM@pks.im>
In-Reply-To
<5728dd2702195b7ba3a208859f114e40ba2b6bbd.1733515638.git.gitgitgadget@gmail.com>
On Fri, Dec 06, 2024 at 08:07:15PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 15 quoted lines
> diff --git a/Documentation/git-backfill.txt b/Documentation/git-backfill.txt
> index 640144187d3..0e10f066fef 100644
> --- a/Documentation/git-backfill.txt
> +++ b/Documentation/git-backfill.txt
> @@ -14,6 +14,30 @@ SYNOPSIS
>  DESCRIPTION
>  -----------
>  
> +Blobless partial clones are created using `git clone --filter=blob:none`
> +and then configure the local repository such that the Git client avoids
> +downloading blob objects unless they are required for a local operation.
> +This initially means that the clone and later fetches download reachable
> +commits and trees but no blobs. Later operations that change the `HEAD`
> +pointer, such as `git checkout` or `git merge`, may need to download
> +missing blobs in order to complete their operation.
Okay.
Show 12 quoted lines
> +In the worst cases, commands that compute blob diffs, such as `git blame`,
> +become very slow as they download the missing blobs in single-blob
> +requests to satisfy the missing object as the Git command needs it. This
> +leads to multiple download requests and no ability for the Git server to
> +provide delta compression across those objects.
> +
> +The `git backfill` command provides a way for the user to request that
> +Git downloads the missing blobs (with optional filters) such that the
> +missing blobs representing historical versions of files can be downloaded
> +in batches. The `backfill` command attempts to optimize the request by
> +grouping blobs that appear at the same path, hopefully leading to good
> +delta compression in the packfile sent by the server.

Hm. So we're asking the user to fix a usability issue of git-blame(1), don't we? Ideally, git-blame(1) itself should know to transparently batch the blobs it requires to compute its output, shouldn't it? That usecase alone doesn't yet convince me that git-backfill(1) is a good idea as I'd think we should rather fix the underlying issue.

So are there other usecases for git-backfill(1)? I can imagine that it might be helpful in the context of scripts that know they'll operate on a bunch of blobs.

Show 42 quoted lines
> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index 38e6aaeaa03..e5f2000d5e0 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -1,16 +1,116 @@
>  #include "builtin.h"
> +#include "git-compat-util.h"
>  #include "config.h"
>  #include "parse-options.h"
>  #include "repository.h"
> +#include "commit.h"
> +#include "hex.h"
> +#include "tree.h"
> +#include "tree-walk.h"
>  #include "object.h"
> +#include "object-store-ll.h"
> +#include "oid-array.h"
> +#include "oidset.h"
> +#include "promisor-remote.h"
> +#include "strmap.h"
> +#include "string-list.h"
> +#include "revision.h"
> +#include "trace2.h"
> +#include "progress.h"
> +#include "packfile.h"
> +#include "path-walk.h"
>  
>  static const char * const builtin_backfill_usage[] = {
>  	N_("git backfill [<options>]"),
>  	NULL
>  };
>  
> +struct backfill_context {
> +	struct repository *repo;
> +	struct oid_array current_batch;
> +	size_t batch_size;
> +};
> +
> +static void clear_backfill_context(struct backfill_context *ctx)
> +{
> +	oid_array_clear(&ctx->current_batch);
> +}
Nit: our style guide says that this should rather be
`backfill_context_clear()`.
Show 38 quoted lines
> +static void download_batch(struct backfill_context *ctx)
> +{
> +	promisor_remote_get_direct(ctx->repo,
> +				   ctx->current_batch.oid,
> +				   ctx->current_batch.nr);
> +	oid_array_clear(&ctx->current_batch);
> +
> +	/*
> +	 * We likely have a new packfile. Add it to the packed list to
> +	 * avoid possible duplicate downloads of the same objects.
> +	 */
> +	reprepare_packed_git(ctx->repo);
> +}
> +
> +static int fill_missing_blobs(const char *path UNUSED,
> +			      struct oid_array *list,
> +			      enum object_type type,
> +			      void *data)
> +{
> +	struct backfill_context *ctx = data;
> +
> +	if (type != OBJ_BLOB)
> +		return 0;
> +
> +	for (size_t i = 0; i < list->nr; i++) {
> +		off_t size = 0;
> +		struct object_info info = OBJECT_INFO_INIT;
> +		info.disk_sizep = &size;
> +		if (oid_object_info_extended(ctx->repo,
> +					     &list->oid[i],
> +					     &info,
> +					     OBJECT_INFO_FOR_PREFETCH) ||
> +		    !size)
> +			oid_array_append(&ctx->current_batch, &list->oid[i]);
> +	}
> +
> +	if (ctx->current_batch.nr >= ctx->batch_size)
> +		download_batch(ctx);

Okay, so the batch size is just "best effort". If we walk a tree that makes us exceed the batch size then we wouldn't issue a fetch during the tree walk. Is there any specific reason for this behaviour?

In any case, as long as this is properly documented I think this should be fine in general.

Show 26 quoted lines
> +	return 0;
> +}
> +
> +static int do_backfill(struct backfill_context *ctx)
> +{
> +	struct rev_info revs;
> +	struct path_walk_info info = PATH_WALK_INFO_INIT;
> +	int ret;
> +
> +	repo_init_revisions(ctx->repo, &revs, "");
> +	handle_revision_arg("HEAD", &revs, 0, 0);
> +
> +	info.blobs = 1;
> +	info.tags = info.commits = info.trees = 0;
> +
> +	info.revs = &revs;
> +	info.path_fn = fill_missing_blobs;
> +	info.path_fn_data = ctx;
> +
> +	ret = walk_objects_by_path(&info);
> +
> +	/* Download the objects that did not fill a batch. */
> +	if (!ret)
> +		download_batch(ctx);
> +
> +	clear_backfill_context(ctx);
Are we leaking `revs` and `info`?
Show 22 quoted lines
> +	return ret;
> +}
> +
>  int cmd_backfill(int argc, const char **argv, const char *prefix, struct repository *repo)
>  {
> +	struct backfill_context ctx = {
> +		.repo = repo,
> +		.current_batch = OID_ARRAY_INIT,
> +		.batch_size = 50000,
> +	};
>  	struct option options[] = {
>  		OPT_END(),
>  	};
> @@ -23,7 +123,5 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
>  
>  	repo_config(repo, git_default_config, NULL);
>  
> -	die(_("not implemented"));
> -
> -	return 0;
> +	return do_backfill(&ctx);
>  }

The current iteration only backfills blobs as far as I can see. Do we maybe want to keep the door open for future changes in git-backfill(1) by implementing this via a "blob" subcommand?

Patrick
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 7 of 39 in “PATH WALK III: Add 'git backfill' command”
  1. 0/5 PATH WALK III: Add 'git backfill' commandDerrick Stolee via GitGitGadget, Dec 6, 2024
  2. 1/5 backfill: add builtin boilerplateDerrick Stolee via GitGitGadget, Dec 6, 2024
  3. Patrick SteinhardtJan 16, 2025
  4. Junio C HamanoJan 16, 2025
  5. Derrick StoleeFeb 3, 2025
  6. 2/5 backfill: basic functionality and testsDerrick Stolee via GitGitGadget, Dec 6, 2024
  7. Patrick SteinhardtDec 16, 2024
  8. Derrick StoleeDec 18, 2024
  9. 3/5 backfill: add --batch-size=<n> optionDerrick Stolee via GitGitGadget, Dec 6, 2024
  10. Patrick SteinhardtDec 16, 2024
  11. Derrick StoleeDec 18, 2024
  12. Jean-Noël AVILAJan 19, 2025
  13. 4/5 backfill: add --sparse optionDerrick Stolee via GitGitGadget, Dec 6, 2024
  14. Patrick SteinhardtDec 16, 2024
  15. 5/5 backfill: assume --sparse when sparse-checkout is enabledDerrick Stolee via GitGitGadget, Dec 6, 2024
  16. Junio C HamanoDec 8, 2024
  17. Junio C HamanoDec 9, 2024
  18. 0/5 PATH WALK III: Add 'git backfill' commandDerrick Stolee via GitGitGadget, Dec 20, 2024
  19. 1/5 backfill: add builtin boilerplateDerrick Stolee via GitGitGadget, Dec 20, 2024
  20. 2/5 backfill: basic functionality and testsDerrick Stolee via GitGitGadget, Dec 20, 2024
  21. Patrick SteinhardtJan 16, 2025
  22. Derrick StoleeFeb 3, 2025
  23. 3/5 backfill: add --min-batch-size=<n> optionDerrick Stolee via GitGitGadget, Dec 20, 2024
  24. Patrick SteinhardtJan 16, 2025
  25. 4/5 backfill: add --sparse optionDerrick Stolee via GitGitGadget, Dec 20, 2024
  26. Patrick SteinhardtJan 16, 2025
  27. Derrick StoleeFeb 3, 2025
  28. 5/5 backfill: assume --sparse when sparse-checkout is enabledDerrick Stolee via GitGitGadget, Dec 20, 2024
  29. Patrick SteinhardtJan 16, 2025
  30. Junio C HamanoJan 17, 2025
  31. 0/5 PATH WALK III: Add 'git backfill' commandDerrick Stolee via GitGitGadget, Feb 3, 2025
  32. 1/5 backfill: add builtin boilerplateDerrick Stolee via GitGitGadget, Feb 3, 2025
  33. 2/5 backfill: basic functionality and testsDerrick Stolee via GitGitGadget, Feb 3, 2025
  34. 3/5 backfill: add --min-batch-size=<n> optionDerrick Stolee via GitGitGadget, Feb 3, 2025
  35. 4/5 backfill: add --sparse optionDerrick Stolee via GitGitGadget, Feb 3, 2025
  36. 5/5 backfill: assume --sparse when sparse-checkout is enabledDerrick Stolee via GitGitGadget, Feb 3, 2025
  37. Junio C HamanoFeb 4, 2025
  38. Patrick SteinhardtFeb 5, 2025
  39. Junio C HamanoFeb 5, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.