All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, newren@gmail.com, matheus.bernardino@usp.br,
	stolee@gmail.com, Eric Sunshine <sunshine@sunshineco.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH v2 2/5] add: allow operating on a sparse-only index
Date: Mon, 26 Jul 2021 15:18:44 +0000	[thread overview]
Message-ID: <6e43f118fa0b7a6ba2e841f75a90406d847d9f09.1627312727.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.999.v2.git.1627312727.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

Disable command_requires_full_index for 'git add'. This does not require
any additional removals of ensure_full_index(). The main reason is that
'git add' discovers changes based on the pathspec and the worktree
itself. These are then inserted into the index directly, and calls to
index_name_pos() or index_file_exists() already call expand_to_path() at
the appropriate time to support a sparse-index.

Add a test to check that 'git add -A' and 'git add <file>' does not
expand the index at all, as long as <file> is not within a sparse
directory. This does not help the global 'git add .' case.

We can measure the improvement using p2000-sparse-operations.sh with
these results:

Test                                  HEAD~1           HEAD
------------------------------------------------------------------------------
2000.6: git add -A (full-index-v3)    0.35(0.30+0.05)  0.37(0.29+0.06) +5.7%
2000.7: git add -A (full-index-v4)    0.31(0.26+0.06)  0.33(0.27+0.06) +6.5%
2000.8: git add -A (sparse-index-v3)  0.57(0.53+0.07)  0.05(0.04+0.08) -91.2%
2000.9: git add -A (sparse-index-v4)  0.58(0.55+0.06)  0.05(0.05+0.06) -91.4%

While the 91% improvement seems impressive, it's important to recognize
that previously we had significant overhead for expanding the
sparse-index. Comparing to the full index case, 'git add -A' goes from
0.37s to 0.05s, which is "only" an 86% improvement.

This modification to 'git add' creates some behavior change depending on
the use of a sparse index. We modify a test in t1092 to demonstrate
these changes which will be remedied in future changes.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/add.c                            |  3 +++
 t/t1092-sparse-checkout-compatibility.sh | 25 +++++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index b773b5a4993..c76e6ddd359 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -528,6 +528,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize;
 	require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 
+	prepare_repo_settings(the_repository);
+	the_repository->settings.command_requires_full_index = 0;
+
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
 	/*
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 47f8e5e54e3..19d38f18ed6 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -340,21 +340,27 @@ test_expect_success 'status/add: outside sparse cone' '
 
 	test_sparse_match git status --porcelain=v2 &&
 
-	# This "git add folder1/a" fails with a warning
-	# in the sparse repos, differing from the full
-	# repo. This is intentional.
+	# Adding the path outside of the sparse-checkout cone should fail.
 	test_sparse_match test_must_fail git add folder1/a &&
-	test_sparse_match test_must_fail git add --refresh folder1/a &&
-	test_all_match git status --porcelain=v2 &&
+
+	test_must_fail git -C sparse-checkout add --refresh folder1/a 2>sparse-checkout-err &&
+	test_must_fail git -C sparse-index add --refresh folder1/a 2>sparse-index-err &&
+	# NEEDSWORK: A sparse index changes the error message.
+	! test_cmp sparse-checkout-err sparse-index-err &&
+
+	# NEEDSWORK: Adding a newly-tracked file outside the cone succeeds
+	test_sparse_match git add folder1/new &&
 
 	test_all_match git add . &&
 	test_all_match git status --porcelain=v2 &&
 	test_all_match git commit -m folder1/new &&
+	test_all_match git rev-parse HEAD^{tree} &&
 
 	run_on_all ../edit-contents folder1/newer &&
 	test_all_match git add folder1/ &&
 	test_all_match git status --porcelain=v2 &&
-	test_all_match git commit -m folder1/newer
+	test_all_match git commit -m folder1/newer &&
+	test_all_match git rev-parse HEAD^{tree}
 '
 
 test_expect_success 'checkout and reset --hard' '
@@ -636,7 +642,12 @@ test_expect_success 'sparse-index is not expanded' '
 	git -C sparse-index reset --hard &&
 	ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 &&
 	git -C sparse-index reset --hard &&
-	ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1
+	ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 &&
+
+	echo >>sparse-index/README.md &&
+	ensure_not_expanded add -A &&
+	echo >>sparse-index/extra.txt &&
+	ensure_not_expanded add extra.txt
 '
 
 # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
-- 
gitgitgadget


  parent reply	other threads:[~2021-07-26 15:19 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 21:06 [PATCH 0/5] Sparse Index: Integrate with 'git add' Derrick Stolee via GitGitGadget
2021-07-21 21:06 ` [PATCH 1/5] t1092: test merge conflicts outside cone Derrick Stolee via GitGitGadget
2021-07-23 17:34   ` Elijah Newren
2021-07-23 17:44     ` Eric Sunshine
2021-07-23 17:47       ` Elijah Newren
2021-07-26 14:10     ` Derrick Stolee
2021-07-21 21:06 ` [PATCH 2/5] add: allow operating on a sparse-only index Derrick Stolee via GitGitGadget
2021-07-21 22:19   ` Junio C Hamano
2021-07-21 22:50     ` Derrick Stolee
2021-07-23 17:45   ` Elijah Newren
2021-07-26 13:11     ` Derrick Stolee
2021-07-26 13:33     ` Derrick Stolee
2021-07-21 21:06 ` [PATCH 3/5] pathspec: stop calling ensure_full_index Derrick Stolee via GitGitGadget
2021-07-23 18:17   ` Elijah Newren
2021-07-21 21:06 ` [PATCH 4/5] t1092: 'git add --refresh' difference with sparse-index Derrick Stolee via GitGitGadget
2021-07-21 21:06 ` [PATCH 5/5] add: ignore outside the sparse-checkout in refresh() Derrick Stolee via GitGitGadget
2021-07-23 19:46   ` Elijah Newren
2021-07-23 12:51 ` [PATCH 0/5] Sparse Index: Integrate with 'git add' Elijah Newren
2021-07-23 20:10   ` Elijah Newren
2021-07-26 15:18 ` [PATCH v2 " Derrick Stolee via GitGitGadget
2021-07-26 15:18   ` [PATCH v2 1/5] t1092: test merge conflicts outside cone Derrick Stolee via GitGitGadget
2021-07-26 15:18   ` Derrick Stolee via GitGitGadget [this message]
2021-07-26 15:18   ` [PATCH v2 3/5] pathspec: stop calling ensure_full_index Derrick Stolee via GitGitGadget
2021-07-26 15:18   ` [PATCH v2 4/5] add: ignore outside the sparse-checkout in refresh() Derrick Stolee via GitGitGadget
2021-07-26 15:18   ` [PATCH v2 5/5] add: remove ensure_full_index() with --renormalize Derrick Stolee via GitGitGadget
2021-07-28 23:13   ` [PATCH v2 0/5] Sparse Index: Integrate with 'git add' Elijah Newren
2021-07-29  2:03     ` Derrick Stolee
2021-07-29  2:57       ` Elijah Newren
2021-07-29 14:49         ` Derrick Stolee
2021-07-30 12:52           ` Elijah Newren
2021-07-29 14:52   ` [PATCH v3 " Derrick Stolee via GitGitGadget
2021-07-29 14:52     ` [PATCH v3 1/5] t1092: test merge conflicts outside cone Derrick Stolee via GitGitGadget
2021-07-29 14:52     ` [PATCH v3 2/5] add: allow operating on a sparse-only index Derrick Stolee via GitGitGadget
2021-07-29 14:52     ` [PATCH v3 3/5] pathspec: stop calling ensure_full_index Derrick Stolee via GitGitGadget
2021-07-29 14:52     ` [PATCH v3 4/5] add: ignore outside the sparse-checkout in refresh() Derrick Stolee via GitGitGadget
2021-07-29 14:52     ` [PATCH v3 5/5] add: remove ensure_full_index() with --renormalize Derrick Stolee via GitGitGadget
2021-07-29 14:58     ` [PATCH v3 0/5] Sparse Index: Integrate with 'git add' Elijah Newren
2021-07-29 23:00     ` Junio C Hamano

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=6e43f118fa0b7a6ba2e841f75a90406d847d9f09.1627312727.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=matheus.bernardino@usp.br \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    --cc=sunshine@sunshineco.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.