| Message ID | 20240402213640.139682-7-shyamthakkar001@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | commit, add: error out when passing untracked path | expand |
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes: > When passing untracked path with -u option, it silently succeeds. > There is no error message and the exit code is zero. This is > inconsistent with other instances of git commands where the expected > argument is a known path. In those other instances, we error out when > the path is not known. > > Fix this by passing a character array to add_files_to_cache() to > collect the pathspec matching information and report the error and > exit if a pathspec does not match any cache entry. Also add a testcase > to cover this scenario. > > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > --- > builtin/add.c | 11 ++++++++++- > t/t2200-add-update.sh | 10 ++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/builtin/add.c b/builtin/add.c > index dc4b42d0ad..88261b0f2b 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -370,6 +370,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) > int add_new_files; > int require_pathspec; > char *seen = NULL; > + char *ps_matched = NULL; > struct lock_file lock_file = LOCK_INIT; > > git_config(add_config, NULL); > @@ -549,13 +550,20 @@ int cmd_add(int argc, const char **argv, const char *prefix) > > begin_odb_transaction(); > > + ps_matched = xcalloc(pathspec.nr, 1); > if (add_renormalize) > exit_status |= renormalize_tracked_files(&pathspec, flags); > else > exit_status |= add_files_to_cache(the_repository, prefix, > - &pathspec, NULL, > + &pathspec, ps_matched, > include_sparse, flags); > > + if (take_worktree_changes && !add_renormalize && > + report_path_error(ps_matched, &pathspec)) { > + free(ps_matched); > + exit(1); > + } Shouldn't we pay attention to ignore_add_errors? The same comments about free'ing and exit code from the review on the previous step apply here, too. Other than that, looking good.
On Tue, 02 Apr 2024, Junio C Hamano <gitster@pobox.com> wrote: > Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes: > > > When passing untracked path with -u option, it silently succeeds. > > There is no error message and the exit code is zero. This is > > inconsistent with other instances of git commands where the expected > > argument is a known path. In those other instances, we error out when > > the path is not known. > > > > Fix this by passing a character array to add_files_to_cache() to > > collect the pathspec matching information and report the error and > > exit if a pathspec does not match any cache entry. Also add a testcase > > to cover this scenario. > > > > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > > --- > > builtin/add.c | 11 ++++++++++- > > t/t2200-add-update.sh | 10 ++++++++++ > > 2 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/builtin/add.c b/builtin/add.c > > index dc4b42d0ad..88261b0f2b 100644 > > --- a/builtin/add.c > > +++ b/builtin/add.c > > @@ -370,6 +370,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) > > int add_new_files; > > int require_pathspec; > > char *seen = NULL; > > + char *ps_matched = NULL; > > struct lock_file lock_file = LOCK_INIT; > > > > git_config(add_config, NULL); > > @@ -549,13 +550,20 @@ int cmd_add(int argc, const char **argv, const char *prefix) > > > > begin_odb_transaction(); > > > > + ps_matched = xcalloc(pathspec.nr, 1); > > if (add_renormalize) > > exit_status |= renormalize_tracked_files(&pathspec, flags); > > else > > exit_status |= add_files_to_cache(the_repository, prefix, > > - &pathspec, NULL, > > + &pathspec, ps_matched, > > include_sparse, flags); > > > > + if (take_worktree_changes && !add_renormalize && > > + report_path_error(ps_matched, &pathspec)) { > > + free(ps_matched); > > + exit(1); > > + } > > Shouldn't we pay attention to ignore_add_errors? The same comments > about free'ing and exit code from the review on the previous step > apply here, too. Will update. > Other than that, looking good. Thanks.
diff --git a/builtin/add.c b/builtin/add.c index dc4b42d0ad..88261b0f2b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -370,6 +370,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) int add_new_files; int require_pathspec; char *seen = NULL; + char *ps_matched = NULL; struct lock_file lock_file = LOCK_INIT; git_config(add_config, NULL); @@ -549,13 +550,20 @@ int cmd_add(int argc, const char **argv, const char *prefix) begin_odb_transaction(); + ps_matched = xcalloc(pathspec.nr, 1); if (add_renormalize) exit_status |= renormalize_tracked_files(&pathspec, flags); else exit_status |= add_files_to_cache(the_repository, prefix, - &pathspec, NULL, + &pathspec, ps_matched, include_sparse, flags); + if (take_worktree_changes && !add_renormalize && + report_path_error(ps_matched, &pathspec)) { + free(ps_matched); + exit(1); + } + if (add_new_files) exit_status |= add_files(&dir, flags); @@ -568,6 +576,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) COMMIT_LOCK | SKIP_IF_UNCHANGED)) die(_("unable to write new index file")); + free(ps_matched); dir_clear(&dir); clear_pathspec(&pathspec); return exit_status; diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index c01492f33f..df235ac306 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -65,6 +65,16 @@ test_expect_success 'update did not touch untracked files' ' test_must_be_empty out ' +test_expect_success 'error out when passing untracked path' ' + git reset --hard && + echo content >>baz && + echo content >>top && + test_must_fail git add -u baz top 2>err && + test_grep -e "error: pathspec .baz. did not match any file(s) known to git" err && + git diff --cached --name-only >actual && + test_must_be_empty actual +' + test_expect_success 'cache tree has not been corrupted' ' git ls-files -s |
When passing untracked path with -u option, it silently succeeds. There is no error message and the exit code is zero. This is inconsistent with other instances of git commands where the expected argument is a known path. In those other instances, we error out when the path is not known. Fix this by passing a character array to add_files_to_cache() to collect the pathspec matching information and report the error and exit if a pathspec does not match any cache entry. Also add a testcase to cover this scenario. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- builtin/add.c | 11 ++++++++++- t/t2200-add-update.sh | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)