[PATCH 1/2] builtin/commit: error out when passing untracked path with -i

[Ghanshyam Thakkar <shyamthakkar001@gmail.com>]

Currently when we provide a pathspec which does not match any tracked
path alongside --include, we do not error like without --include. If
there is something staged, it will commit the staged changes and ignore
the pathspec which does not match any tracked path. And if nothing is
staged, it will print the status. Exit code is 0 in both cases (unlike
without --include). This was also described in the TODO comment before
the relevant testcase.

Fix this by matching the pathspec against index and report error if
any. And amend the relevant testcase and remove the TODO comment.
As this matches the pathspec against index, we need to also make sure
that the sparse index is expanded before matching the pathspec. A
side-effect of this is removal of --include related lines from the
testcase which checks if the sparse-index is expanded or not in t1092.

Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
[RFC]: I am still unsure about the removal of --include related lines
from the testcase which checks whether the index is expanded or not from
t1092. Will separating it into a separate testcase of its own and
marking that to expect failure be better?

 builtin/commit.c                         | 15 +++++++++++++++
 t/t1092-sparse-checkout-compatibility.sh |  4 ----
 t/t7501-commit-basic-functionality.sh    | 16 +---------------
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a91197245f..f8f5909673 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -441,6 +441,21 @@ static const char *prepare_index(const char **argv, const char *prefix,
 	 * (B) on failure, rollback the real index.
 	 */
 	if (all || (also && pathspec.nr)) {
+		if (!all) {
+			int i, ret;
+			char *ps_matched = xcalloc(pathspec.nr, 1);
+
+			/* TODO: audit for interaction with sparse-index. */
+			ensure_full_index(&the_index);
+			for (i = 0; i < the_index.cache_nr; i++)
+				ce_path_match(&the_index, the_index.cache[i],
+					      &pathspec, ps_matched);
+
+			ret = report_path_error(ps_matched, &pathspec);
+			free(ps_matched);
+			if (ret)
+				exit(1);
+		}
 		repo_hold_locked_index(the_repository, &index_lock,
 				       LOCK_DIE_ON_ERROR);
 		add_files_to_cache(the_repository, also ? prefix : NULL,
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 2f1ae5fd3b..b55c81d4f7 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -1418,10 +1418,6 @@ test_expect_success 'sparse-index is not expanded' '
 	ensure_not_expanded commit --allow-empty -m empty &&
 	echo >>sparse-index/a &&
 	ensure_not_expanded commit -a -m a &&
-	echo >>sparse-index/a &&
-	ensure_not_expanded commit --include a -m a &&
-	echo >>sparse-index/deep/deeper1/a &&
-	ensure_not_expanded commit --include deep/deeper1/a -m deeper &&
 	ensure_not_expanded checkout rename-out-to-out &&
 	ensure_not_expanded checkout - &&
 	ensure_not_expanded switch rename-out-to-out &&
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index bced44a0fc..cc12f99f11 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -101,22 +101,8 @@ test_expect_success 'fail to commit untracked file (even with --include/--only)'
 	test_must_fail git commit --only -m "baz" baz 2>err &&
 	test_grep -e "$error" err &&
 
-	# TODO: as for --include, the below command will fail because
-	# nothing is staged. If something was staged, it would not fail
-	# even though the provided pathspec does not match any tracked
-	# path. (However, the untracked paths that match the pathspec are
-	# not committed and only the staged changes get committed.)
-	# In either cases, no error is returned to stderr like in (--only
-	# and without --only/--include) cases. In a similar manner,
-	# "git add -u baz" also does not error out.
-	#
-	# Therefore, the below test is just to document the current behavior
-	# and is not an endorsement to the current behavior, and we may
-	# want to fix this. And when that happens, this test should be
-	# updated accordingly.
-
 	test_must_fail git commit --include -m "baz" baz 2>err &&
-	test_must_be_empty err
+	test_grep -e "$error" err
 '
 
 test_expect_success 'setup: non-initial commit' '
-- 
2.44.0