diff mbox series

[2/2] builtin/add: error out when passing untracked path with -u

Message ID 20240318155219.494206-6-shyamthakkar001@gmail.com (mailing list archive)
State New
Headers show
Series fix certain cases of add and commit with untracked path not erroring out | expand

Commit Message

Ghanshyam Thakkar March 18, 2024, 3:52 p.m. UTC
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(+)

Comments

Junio C Hamano March 18, 2024, 5:31 p.m. UTC | #1
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> 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.

> 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 |
diff mbox series

Patch

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 |