builtin/commit: error out when passing untracked path with -i

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 is also described in the TODO comment before the relevant testcase. Fix this by passing a character array to add_files_to_cache() to collect the pathspec matching information and error out if the given path is untracked. Also, amend the testcase to check for the error message and remove the TODO comment. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ghanshyam Thakkar committed Apr 3, 2024 at 23:44 UTC ac5946e6248eb84458d236099f356e5e09e2482f
2 files changed +7 -16
builtin/commit.c
+6 -1
@@ -441,16 +441,21 @@ static const char *prepare_index(const char **argv, const char *prefix,
441 * (B) on failure, rollback the real index.
442 */
443 if (all || (also && pathspec.nr)) {
444 + char *ps_matched = xcalloc(pathspec.nr, 1);
445 repo_hold_locked_index(the_repository, &index_lock,
446 LOCK_DIE_ON_ERROR);
447 add_files_to_cache(the_repository, also ? prefix : NULL,
447 - &pathspec, NULL, 0, 0);
448 + &pathspec, ps_matched, 0, 0);
449 + if (!all && report_path_error(ps_matched, &pathspec))
450 + exit(128);
451 +
452 refresh_cache_or_die(refresh_flags);
453 cache_tree_update(&the_index, WRITE_TREE_SILENT);
454 if (write_locked_index(&the_index, &index_lock, 0))
455 die(_("unable to write new index file"));
456 commit_style = COMMIT_NORMAL;
457 ret = get_lock_file_path(&index_lock);
458 + free(ps_matched);
459 goto out;
460 }
461
t/t7501-commit-basic-functionality.sh
+1 -15
@@ -101,22 +101,8 @@ test_expect_success 'fail to commit untracked file (even with --include/--only)'
101 test_must_fail git commit --only -m "baz" baz 2>err &&
102 test_grep -e "$error" err &&
103
104 - # TODO: as for --include, the below command will fail because
105 - # nothing is staged. If something was staged, it would not fail
106 - # even though the provided pathspec does not match any tracked
107 - # path. (However, the untracked paths that match the pathspec are
108 - # not committed and only the staged changes get committed.)
109 - # In either cases, no error is returned to stderr like in (--only
110 - # and without --only/--include) cases. In a similar manner,
111 - # "git add -u baz" also does not error out.
112 - #
113 - # Therefore, the below test is just to document the current behavior
114 - # and is not an endorsement to the current behavior, and we may
115 - # want to fix this. And when that happens, this test should be
116 - # updated accordingly.
117 -
104 test_must_fail git commit --include -m "baz" baz 2>err &&
119 - test_must_be_empty err
105 + test_grep -e "$error" err
106 '
107
108 test_expect_success 'setup: non-initial commit' '