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

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

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

-- 
gitgitgadget