# git add -u nonexistent-file 13 messages from 2010-02-08 to 2010-02-10. Participants: SZEDER Gábor, Chris Packham, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/22572 ## SZEDER Gábor, 2010-02-08 18:29 Subject: git add -u nonexistent-file Message-ID: <20100208182929.GB14355@neumann> URL: https://gitlist.dev/e/20100208182929.GB14355%40neumann ``` Hi, $ git --version git version 1.7.0.rc1.84.g9879 $ git add -u nonexistent-file $ echo $? 0 No error message, no error in exit status. Is it OK this way? Why? Best, Gábor ``` ## Chris Packham, 2010-02-08 19:12 Subject: Re: git add -u nonexistent-file Message-ID: URL: https://gitlist.dev/e/a038bef51002081112s2d552835m4b34973f797ab4f8%40mail.gmail.com In-Reply-To: <20100208182929.GB14355@neumann> ``` 2010/2/8 SZEDER Gábor : > Hi, > > > $ git --version > git version 1.7.0.rc1.84.g9879 > $ git add -u nonexistent-file > $ echo $? > 0 > > No error message, no error in exit status. > > Is it OK this way?  Why? > > > Best, > Gábor > Hi, I see the same behaviour with git version 1.6.4.2. From the man page of git-add -u, --update Update only files that git already knows about, staging modified content for commit and marking deleted files for removal. So git will only look at files that are already in the repository. It looks like in the case you've highlighted git is ignoring the extra non-option parameters on the command line. I'll let other people argue whether this is by design or omission. 'git add nonexistent-file' works as expected, exiting with an error message and non-zero exit status. ``` ## Jeff King, 2010-02-09 00:39 Subject: Re: git add -u nonexistent-file Message-ID: <20100209003958.GA4065@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100209003958.GA4065%40coredump.intra.peff.net In-Reply-To: ``` On Mon, Feb 08, 2010 at 02:12:41PM -0500, Chris Packham wrote: > > $ git add -u nonexistent-file > > $ echo $? > > 0 > [...] > It looks like in the case you've highlighted git is ignoring the extra > non-option parameters on the command line. I'll let other people argue > whether this is by design or omission. It's not ignoring the extra parameters. They limit the scope of the operation. So: $ git init $ touch file && mkdir subdir && touch subdir/file $ git add . && git commit -m one $ echo changes >file && echo changes >subdir/file $ git add -u subdir $ git status # On branch master # Changes to be committed: # modified: subdir/file # # Changed but not updated: # modified: file # That being said, you noticed that the regular add case notes unused pathspecs on the command line: $ git add bogus fatal: pathspec 'bogus' did not match any files We could probably do the same here. -Peff ``` ## Chris Packham, 2010-02-09 14:43 Subject: Re: git add -u nonexistent-file Message-ID: URL: https://gitlist.dev/e/a038bef51002090643g23b35bb6xaa765ff13b80f5bb%40mail.gmail.com In-Reply-To: <20100209003958.GA4065@coredump.intra.peff.net> ``` On Mon, Feb 8, 2010 at 7:39 PM, Jeff King wrote: > On Mon, Feb 08, 2010 at 02:12:41PM -0500, Chris Packham wrote: > >> > $ git add -u nonexistent-file >> > $ echo $? >> > 0 >> [...] >> It looks like in the case you've highlighted git is ignoring the extra >> non-option parameters on the command line. I'll let other people argue >> whether this is by design or omission. > > It's not ignoring the extra parameters. They limit the scope of the > operation. So: > >  $ git init >  $ touch file && mkdir subdir && touch subdir/file >  $ git add . && git commit -m one >  $ echo changes >file && echo changes >subdir/file >  $ git add -u subdir >  $ git status >  # On branch master >  # Changes to be committed: >  #       modified:   subdir/file >  # >  # Changed but not updated: >  #       modified:   file >  # Yep my bad. I tried the non-existent case but not the "normal" case. Re reading the man page it makes sense it just happens that the part scrolls off the top when I get to the -u part. > That being said, you noticed that the regular add case notes unused > pathspecs on the command line: > >  $ git add bogus >  fatal: pathspec 'bogus' did not match any files > > We could probably do the same here. I think so. By not having this error it led me to think "OK it just throws away the pathspec". Gabor rightly thought that it does use it so shouldn't it be giving an error. If I get brave enough I could attempt a patch but I wouldn't let that dissuade anyone that actually knows what they're doing from jumping in with a patch. ``` ## Chris Packham, 2010-02-09 21:31 Subject: [PATCH] test for add with non-existent pathspec Message-ID: <1265751100-21558-1-git-send-email-judge.packham@gmail.com> URL: https://gitlist.dev/e/1265751100-21558-1-git-send-email-judge.packham%40gmail.com In-Reply-To: ``` Add a test for 'git add -u pathspec' and 'git add pathspec' where pathspec does not exist. The expected result is that git add exits with an error message and an appropriate exit code. This adds 1 expected failure to t/t2200-add-update.sh. Signed-off-by: Chris Packham --- If anyone feels like pursuing this heres a patch to two simple tests around this behaviour. I made an initial attempt to change builtin-add.c to print an error message and set the exit code to a non-zero value but my naive attempt broke the normal usage of 'git add pathspec'. t/t2200-add-update.sh | 5 +++++ t/t3700-add.sh | 5 +++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 9120750..dbabc3c 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -176,4 +176,9 @@ test_expect_success 'add -u resolves unmerged paths' ' ' +test_expect_failure 'error out when attempting to add -u non-existent pathspec' ' + test_must_fail git add -u non-existent && + ! (git ls-files | grep "non-existent") +' + test_done diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 85eb0fb..c77bb71 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -255,4 +255,9 @@ test_expect_success 'git add to resolve conflicts on otherwise ignored path' ' git add track-this ' +test_expect_success 'error out when attempting to add non-existent pathspec' ' + test_must_fail git add non-existent && + ! (git ls-files | grep "non-existent") +' + test_done -- 1.6.4.2 ``` ## Junio C Hamano, 2010-02-09 21:58 Subject: Re: git add -u nonexistent-file Message-ID: <7v7hqm2hs7.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7hqm2hs7.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100209003958.GA4065@coredump.intra.peff.net> ``` Jeff King writes: > It's not ignoring the extra parameters. They limit the scope of the > operation. So: > > $ git init > $ touch file && mkdir subdir && touch subdir/file > $ git add . && git commit -m one > $ echo changes >file && echo changes >subdir/file > $ git add -u subdir > $ git status > # On branch master > # Changes to be committed: > # modified: subdir/file > # > # Changed but not updated: > # modified: file > # > > That being said, you noticed that the regular add case notes unused > pathspecs on the command line: > > $ git add bogus > fatal: pathspec 'bogus' did not match any files > > We could probably do the same here. It won't be entirely trivial to do so efficiently but it shouldn't be a rocket surgery. Something like this (untested of course)? builtin-add.c | 38 +++++++++++++++++++++++++++++--------- 1 files changed, 29 insertions(+), 9 deletions(-) diff --git a/builtin-add.c b/builtin-add.c index 2705f8d..87d2980 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -117,7 +117,19 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs) } } -static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix) +static char *find_used_pathspec(const char **pathspec) +{ + char *seen; + int i; + + for (i = 0; pathspec[i]; i++) + ; /* just counting */ + seen = xcalloc(i, 1); + fill_pathspec_matches(pathspec, seen, i); + return seen; +} + +static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix) { char *seen; int i, specs; @@ -137,13 +149,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p } dir->nr = dst - dir->entries; fill_pathspec_matches(pathspec, seen, specs); - - for (i = 0; i < specs; i++) { - if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i])) - die("pathspec '%s' did not match any files", - pathspec[i]); - } - free(seen); + return seen; } static void treat_gitlinks(const char **pathspec) @@ -359,6 +365,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) int flags; int add_new_files; int require_pathspec; + char *seen = NULL; git_config(add_config, NULL); @@ -418,7 +425,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) /* This picks up the paths that are not tracked */ baselen = fill_directory(&dir, pathspec); if (pathspec) - prune_directory(&dir, pathspec, baselen); + seen = prune_directory(&dir, pathspec, baselen); } if (refresh_only) { @@ -426,6 +433,19 @@ int cmd_add(int argc, const char **argv, const char *prefix) goto finish; } + if (pathspec) { + int i; + if (!seen) + seen = find_used_pathspec(pathspec); + for (i = 0; pathspec[i]; i++) { + if (!seen[i] && pathspec[i][0] + && !file_exists(pathspec[i])) + die("pathspec '%s' did not match any files", + pathspec[i]); + } + free(seen); + } + exit_status |= add_files_to_cache(prefix, pathspec, flags); if (add_new_files) ``` ## Chris Packham, 2010-02-09 22:17 Subject: Re: git add -u nonexistent-file Message-ID: URL: https://gitlist.dev/e/a038bef51002091417o1048c098x898517596115ed90%40mail.gmail.com In-Reply-To: <7v7hqm2hs7.fsf@alter.siamese.dyndns.org> ``` On Tue, Feb 9, 2010 at 4:58 PM, Junio C Hamano wrote: > > It won't be entirely trivial to do so efficiently but it shouldn't be a > rocket surgery. > > Something like this (untested of course)? > Passes my new test and all the rest in t2200-add-update.sh and t3700-add.sh. Hows this for a commit message: git add -u: give an error if pathspec unmatched If a pathspec is supplied to 'git add -u' and no matching path is matched, fail with an approriate error message and exit code. ``` ## Chris Packham, 2010-02-09 22:30 Subject: Re: git add -u nonexistent-file Message-ID: <1265754650-25438-1-git-send-email-judge.packham@gmail.com> URL: https://gitlist.dev/e/1265754650-25438-1-git-send-email-judge.packham%40gmail.com In-Reply-To: ``` Heres the up to date series with Junios fix and my test. The two test patches could be squashed together if we want to change the order. ``` ## Chris Packham, 2010-02-09 22:30 Subject: [PATCH 1/3] test for add with non-existent pathspec Message-ID: <1265754650-25438-2-git-send-email-judge.packham@gmail.com> URL: https://gitlist.dev/e/1265754650-25438-2-git-send-email-judge.packham%40gmail.com In-Reply-To: <1265754650-25438-1-git-send-email-judge.packham@gmail.com> ``` Add a test for 'git add -u pathspec' and 'git add pathspec' where pathspec does not exist. The expected result is that git add exits with an error message and an appropriate exit code. This adds 1 expected failure to t/t2200-add-update.sh. Signed-off-by: Chris Packham --- t/t2200-add-update.sh | 5 +++++ t/t3700-add.sh | 5 +++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 9120750..dbabc3c 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -176,4 +176,9 @@ test_expect_success 'add -u resolves unmerged paths' ' ' +test_expect_failure 'error out when attempting to add -u non-existent pathspec' ' + test_must_fail git add -u non-existent && + ! (git ls-files | grep "non-existent") +' + test_done diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 85eb0fb..c77bb71 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -255,4 +255,9 @@ test_expect_success 'git add to resolve conflicts on otherwise ignored path' ' git add track-this ' +test_expect_success 'error out when attempting to add non-existent pathspec' ' + test_must_fail git add non-existent && + ! (git ls-files | grep "non-existent") +' + test_done -- 1.6.4.2 ``` ## Chris Packham, 2010-02-09 22:30 Subject: [PATCH 2/3] git add -u: give an error if pathspec unmatched Message-ID: <1265754650-25438-3-git-send-email-judge.packham@gmail.com> URL: https://gitlist.dev/e/1265754650-25438-3-git-send-email-judge.packham%40gmail.com In-Reply-To: <1265754650-25438-2-git-send-email-judge.packham@gmail.com> ``` From: Junio C Hamano If a pathspec is supplied to 'git add -u' and no matching path is matched, fail with an approriate error message and exit code. Tested-by: Chris Packham --- builtin-add.c | 38 +++++++++++++++++++++++++++++--------- 1 files changed, 29 insertions(+), 9 deletions(-) diff --git a/builtin-add.c b/builtin-add.c index 2705f8d..87d2980 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -117,7 +117,19 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs) } } -static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix) +static char *find_used_pathspec(const char **pathspec) +{ + char *seen; + int i; + + for (i = 0; pathspec[i]; i++) + ; /* just counting */ + seen = xcalloc(i, 1); + fill_pathspec_matches(pathspec, seen, i); + return seen; +} + +static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix) { char *seen; int i, specs; @@ -137,13 +149,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p } dir->nr = dst - dir->entries; fill_pathspec_matches(pathspec, seen, specs); - - for (i = 0; i < specs; i++) { - if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i])) - die("pathspec '%s' did not match any files", - pathspec[i]); - } - free(seen); + return seen; } static void treat_gitlinks(const char **pathspec) @@ -359,6 +365,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) int flags; int add_new_files; int require_pathspec; + char *seen = NULL; git_config(add_config, NULL); @@ -418,7 +425,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) /* This picks up the paths that are not tracked */ baselen = fill_directory(&dir, pathspec); if (pathspec) - prune_directory(&dir, pathspec, baselen); + seen = prune_directory(&dir, pathspec, baselen); } if (refresh_only) { @@ -426,6 +433,19 @@ int cmd_add(int argc, const char **argv, const char *prefix) goto finish; } + if (pathspec) { + int i; + if (!seen) + seen = find_used_pathspec(pathspec); + for (i = 0; pathspec[i]; i++) { + if (!seen[i] && pathspec[i][0] + && !file_exists(pathspec[i])) + die("pathspec '%s' did not match any files", + pathspec[i]); + } + free(seen); + } + exit_status |= add_files_to_cache(prefix, pathspec, flags); if (add_new_files) -- 1.6.4.2 ``` ## Chris Packham, 2010-02-09 22:30 Subject: [PATCH 3/3] t2200-add-update.sh: change expected fail to success Message-ID: <1265754650-25438-4-git-send-email-judge.packham@gmail.com> URL: https://gitlist.dev/e/1265754650-25438-4-git-send-email-judge.packham%40gmail.com In-Reply-To: <1265754650-25438-3-git-send-email-judge.packham@gmail.com> ``` This changes the test added in bb73e5c to an expected success now that Junio has supplied a fix. Signed-off-by: Chris Packham --- t/t2200-add-update.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index dbabc3c..6302137 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -176,7 +176,7 @@ test_expect_success 'add -u resolves unmerged paths' ' ' -test_expect_failure 'error out when attempting to add -u non-existent pathspec' ' +test_expect_success 'error out when attempting to add -u non-existent pathspec' ' test_must_fail git add -u non-existent && ! (git ls-files | grep "non-existent") ' -- 1.6.4.2 ``` ## Junio C Hamano, 2010-02-09 23:18 Subject: Re: git add -u nonexistent-file Message-ID: <7vy6j2vw08.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy6j2vw08.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Chris Packham writes: > On Tue, Feb 9, 2010 at 4:58 PM, Junio C Hamano wrote: >> >> It won't be entirely trivial to do so efficiently but it shouldn't be a >> rocket surgery. >> >> Something like this (untested of course)? >> > > Passes my new test and all the rest in t2200-add-update.sh and t3700-add.sh. Thanks; will squash two tests into one. ``` ## Jeff King, 2010-02-10 05:57 Subject: Re: git add -u nonexistent-file Message-ID: <20100210055727.GH28526@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100210055727.GH28526%40coredump.intra.peff.net In-Reply-To: <7v7hqm2hs7.fsf@alter.siamese.dyndns.org> ``` On Tue, Feb 09, 2010 at 01:58:16PM -0800, Junio C Hamano wrote: > > That being said, you noticed that the regular add case notes unused > > pathspecs on the command line: > > > > $ git add bogus > > fatal: pathspec 'bogus' did not match any files > > > > We could probably do the same here. > > It won't be entirely trivial to do so efficiently but it shouldn't be a > rocket surgery. > > Something like this (untested of course)? Looks like Chris gave it some basic testing. I read over the patch itself, and it looks sane to me. -Peff ```