[PATCH 2/2] commit: lock index before reading

[Jun Wu <quark.zju@gmail.com>]

From: Jun Wu <quark@meta.com>

Take the index lock before reading the index, in one place, instead
of separately in each `if` branch.

`commit` now respects the `core.indexLockTimeout` config.

History suggests no particular reason for reading before locking:
- f5bbc3225c (Port git commit to C., 2007-11-08): lock before reading
- 2888605c64 (builtin-commit: fix partial-commit support, 2007-11-18):
  seems to accidentally move reading to before locking

Signed-off-by: Jun Wu <quark@meta.com>
---
 builtin/commit.c              | 9 ++-------
 t/t0032-index-lock-timeout.sh | 4 ++--
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 840b6b4083..08dd53ad4e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -389,13 +389,13 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	    (!amend || (fixup_message && strcmp(fixup_prefix, "amend"))))))
 		die(_("No paths with --include/--only does not make sense."));
 
+	repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR);
+
 	if (repo_read_index_preload(the_repository, &pathspec, 0) < 0)
 		die(_("index file corrupt"));
 
 	if (interactive) {
 		char *old_index_env = NULL, *old_repo_index_file;
-		repo_hold_locked_index(the_repository, &index_lock,
-				       LOCK_DIE_ON_ERROR);
 
 		refresh_cache_or_die(refresh_flags);
 
@@ -453,8 +453,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	 */
 	if (all || (also && pathspec.nr)) {
 		char *ps_matched = xcalloc(pathspec.nr, 1);
-		repo_hold_locked_index(the_repository, &index_lock,
-				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(the_repository, also ? prefix : NULL,
 				   &pathspec, ps_matched, 0, 0, 0 );
 		if (!all && report_path_error(ps_matched, &pathspec))
@@ -480,8 +478,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	 * We still need to refresh the index here.
 	 */
 	if (!only && !pathspec.nr) {
-		repo_hold_locked_index(the_repository, &index_lock,
-				       LOCK_DIE_ON_ERROR);
 		refresh_cache_or_die(refresh_flags);
 		if (the_repository->index->cache_changed
 		    || !cache_tree_fully_valid(the_repository->index))
@@ -531,7 +527,6 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	if (repo_read_index(the_repository) < 0)
 		die(_("cannot read the index"));
 
-	repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR);
 	add_remove_files(&partial);
 	refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
 	cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
diff --git a/t/t0032-index-lock-timeout.sh b/t/t0032-index-lock-timeout.sh
index ca334c3eef..0c36a7cdfe 100755
--- a/t/t0032-index-lock-timeout.sh
+++ b/t/t0032-index-lock-timeout.sh
@@ -29,9 +29,9 @@ test_expect_success 'index locked, add waits with indexLockTimeout>0' '
 	git -c core.indexLockTimeout=5000 add foo
 '
 
-test_expect_success 'index locked, commit fails even with indexLockTimeout>0 (read index before lock)' '
+test_expect_success 'index locked, commit waits with indexLockTimeout>0' '
 	prepare_index_lock &&
-	test_must_fail_with_index_lock git -c core.indexLockTimeout=5000 commit -m init
+	git -c core.indexLockTimeout=5000 commit -m init
 '
 
 test_done
-- 
2.56.0