Re: [PATCH 2/2] builtin/add: error out when passing untracked path with -u
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 18, 2024, 17:31 UTC
- Message-ID
- <xmqqa5mvh2fi.fsf@gitster.g>
- In-Reply-To
- <20240318155219.494206-6-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
Show 12 quoted lines
> Currently when we pass a pathspec which does not match any tracked path > along side --update, it silently succeeds, unlike without --update. As > --update only touches known paths, match the pathspec against the index > and error out when no match found. And ensure that the index is fully > expanded before matching the pathspec. Also add a testcase to check > for the error. > > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > --- > builtin/add.c | 16 ++++++++++++++++ > t/t2200-add-update.sh | 5 +++++ > 2 files changed, 21 insertions(+)
Exactly the same comment applies here. If we are using pathspec, we should already have a loop that calls ce_path_match() for each and every path we know about, and we should be updating the code to collect "have we used all pathspec elements?" information at the same time if it is not doing so already. Let's not make another loop that checks what we should already be doing elsewhere.
Thanks.
Show 50 quoted lines
> diff --git a/builtin/add.c b/builtin/add.c
> index 393c10cbcf..7ec5ea4a3e 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -24,6 +24,7 @@
> #include "strvec.h"
> #include "submodule.h"
> #include "add-interactive.h"
> +#include "sparse-index.h"
>
> static const char * const builtin_add_usage[] = {
> N_("git add [<options>] [--] <pathspec>..."),
> @@ -536,6 +537,21 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> }
> }
>
> + if (take_worktree_changes && pathspec.nr) {
> + 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);
> + }
>
> if (only_match_skip_worktree.nr) {
> advise_on_updating_sparse_paths(&only_match_skip_worktree);
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index c01492f33f..f6a9615d1b 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -65,6 +65,11 @@ test_expect_success 'update did not touch untracked files' '
> test_must_be_empty out
> '
>
> +test_expect_success 'error out when given untracked path' '
> + test_must_fail git add -u dir2/other 2>err &&
> + test_grep -e "error: pathspec .dir2/other. did not match any file(s) known to git" err
> +'
> +
> test_expect_success 'cache tree has not been corrupted' '
>
> git ls-files -s |