Skip to content

restore: better integrate with sparse index

Derrick Stolee via GitGitGadget git 2 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest2 patches 0 repliesNew
  2. v12 patches 3 repliesSuperseded

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2t1092: test 'git restore' with sparse indext/t1092-sparse-checkout-compatibility.sh+50 -00 replies
2/2restore: avoid sparse index expansionbuiltin/checkout.c t/t1092-sparse-checkout-compatibility.sh+63 -100 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/2] restore: better integrate with sparse index

There's still a long tail of situations where Git expands a sparse index in-memory in order to operate on blob path entries instead of intelligently handling trees. I was recently alerted to one such case with git restore --staged -- ..

The basic idea here is that the pathspec . signals that all paths matter, but what we want to do across those pathspecs will ignore the expanded blob paths with the SKIP_WORKTREE bit, so we should avoid expanding the tree when we can.

This series has two patches: first a test to demonstrate the baseline behavior of git restore across different sparsity cases as well as demonstrate that the index is currently expanded. The second patch includes the fix and maintains the same end-to-end behavior with the only change being the performance improvement from not expanding the sparse index.

Update in v2
============

The logic around handling a tree entry is extracted to a helper method, making the diff easier to read.

Thanks, -Stolee

Derrick Stolee (2):
t1092: test 'git restore' with sparse index
restore: avoid sparse index expansion

 builtin/checkout.c                       | 65 +++++++++++++++++++++---
 t/t1092-sparse-checkout-compatibility.sh | 50 ++++++++++++++++++
 2 files changed, 109 insertions(+), 6 deletions(-)

base-commit: aec3f587505a472db67e9462d0702e7d463a449d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2121%2Fderrickstolee%2Frestore-sparse-index-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2121/derrickstolee/restore-sparse-index-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2121

Range-diff vs v1:

 1:  7c56d03830 = 1:  7c56d03830 t1092: test 'git restore' with sparse index
 2:  47542cbd42 ! 2:  88f5d26a33 restore: avoid sparse index expansion
     @@ builtin/checkout.c
       #include "submodule.h"
       #include "symlinks.h"
      @@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm
     + 	return run_hooks_opt(the_repository, "post-checkout", &opt);
       }
       
     ++/*
     ++ * Handle a tree object and determine if we need to recurse into the
     ++ * tree (READ_TREE_RECURSIVE) or skip it (0).
     ++ */
     ++static int try_update_sparse_directory(const struct object_id *oid,
     ++				       struct strbuf *base,
     ++				       const char *pathname,
     ++				       int overlay_mode)
     ++{
     ++	struct strbuf dirpath = STRBUF_INIT;
     ++	struct cache_entry *old;
     ++	int pos, result = READ_TREE_RECURSIVE;
     ++
     ++	if (!the_repository->index->sparse_index)
     ++		return result;
     ++
     ++	strbuf_addbuf(&dirpath, base);
     ++	strbuf_addstr(&dirpath, pathname);
     ++	strbuf_addch(&dirpath, '/');
     ++
     ++	pos = index_name_pos_sparse(the_repository->index,
     ++				    dirpath.buf, dirpath.len);
     ++	if (pos < 0)
     ++		goto cleanup;
     ++
     ++	old = the_repository->index->cache[pos];
     ++	if (!S_ISSPARSEDIR(old->ce_mode))
     ++		goto cleanup;
     ++
     ++	if (oideq(oid, &old->oid)) {
     ++		/* Tree content already matches; no need to descend. */
     ++		result = 0;
     ++	} else if (!overlay_mode) {
     ++		/*
     ++		 * In non-overlay mode (e.g., restore --staged), replace the
     ++		 * sparse directory OID directly since files not present in
     ++		 * the source tree should be removed anyway.
     ++		 */
     ++		oidcpy(&old->oid, oid);
     ++		old->ce_flags |= CE_UPDATE;
     ++		result = 0;
     ++	}
     ++
     ++cleanup:
     ++	strbuf_release(&dirpath);
     ++	return result;
     ++}
     ++
       static int update_some(const struct object_id *oid, struct strbuf *base,
      -		       const char *pathname, unsigned mode, void *context UNUSED)
      +		       const char *pathname, unsigned mode, void *context)
     @@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, str
       	int pos;
      +	int overlay_mode = context ? *((int *)context) : 1;
       
     --	if (S_ISDIR(mode))
     -+	if (S_ISDIR(mode)) {
     -+		/*
     -+		 * If this directory exists as a sparse directory entry in
     -+		 * the index, we can handle it at the tree level without
     -+		 * descending into individual files.
     -+		 */
     -+		if (the_repository->index->sparse_index) {
     -+			struct strbuf dirpath = STRBUF_INIT;
     -+
     -+			strbuf_addbuf(&dirpath, base);
     -+			strbuf_addstr(&dirpath, pathname);
     -+			strbuf_addch(&dirpath, '/');
     -+
     -+			pos = index_name_pos_sparse(the_repository->index,
     -+						    dirpath.buf, dirpath.len);
     -+			if (pos >= 0) {
     -+				struct cache_entry *old =
     -+					the_repository->index->cache[pos];
     -+				if (S_ISSPARSEDIR(old->ce_mode)) {
     -+					if (oideq(oid, &old->oid)) {
     -+						strbuf_release(&dirpath);
     -+						return 0;
     -+					}
     -+					if (!overlay_mode) {
     -+						/*
     -+						 * In non-overlay mode (e.g.,
     -+						 * restore --staged), we can
     -+						 * replace the sparse dir OID
     -+						 * directly since files not in
     -+						 * the source tree should be
     -+						 * removed anyway.
     -+						 */
     -+						oidcpy(&old->oid, oid);
     -+						old->ce_flags |= CE_UPDATE;
     -+						strbuf_release(&dirpath);
     -+						return 0;
     -+					}
     -+				}
     -+			}
     -+			strbuf_release(&dirpath);
     -+		}
     - 		return READ_TREE_RECURSIVE;
     -+	}
     + 	if (S_ISDIR(mode))
     +-		return READ_TREE_RECURSIVE;
     ++		return try_update_sparse_directory(oid, base, pathname,
     ++						   overlay_mode);
       
       	len = base->len + strlen(pathname);
       	ce = make_empty_cache_entry(the_repository->index, len);
Signature · 1 line
gitgitgadget