Re: [PATCH] Remove duplicate pathspecs from ls-files command line
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 29, 2007, 20:44 UTC
- Message-ID
- <7v4piioyu1.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20070829194410.GA11824@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
Show 22 quoted lines
> The first entry wins, all the subsequent entries will be discarded. > > Signed-off-by: Alex Riesen <raa.lkml@gmail.com> > --- > > martin f krafft, Wed, Aug 29, 2007 10:11:22 +0200: >> when using git-add from a script, the following fails: >> >> $ git commit -m. foo foo >> error: pathspec 'foo' did not match any file(s) known to git. >> Did you forget to 'git add'? >> >> I am bringing this up in the context of >> http://bugs.debian.org/439992, where debcommit.pl would duplicate >> a file argument under certain conditions. It's since been fixed, but >> I wonder whether git-commit could be made more robust in the >> presence of duplicate arguments? Or is this behaviour by choice? > > Don't think so. Looks like accident. The patch below fixes it, > by introducing a costly argument duplication check. Shouldn't > be a problem for a normal use (git-ls-files expects globs, not > pathnames).
Thanks both for your attention to the detail. It was to catch
git commit Makefiel
and did not mean to warn about listing the same thing twice (it is still a mistaken usage in the sense that it is unnecessary to list things twice, not in the sense that it instructs the command to commit the same file twice).
The patch is not wrong per-se from correctness standpoint, but I must say that it is a horrible thing to do from both performance and principle point of view.
That loop is plain old O(n^2) that penalizes everybody.
Please do not penalize sane callers when you try to improve support of mistaken usage. Move expensive error recovery in the error path when possible, and have _only_ mistaken users pay the price.
Like this perhaps.
--- builtin-ls-files.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/builtin-ls-files.c b/builtin-ls-files.c index d36181a..cce17b5 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -511,8 +511,28 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) */ int num, errors = 0; for (num = 0; pathspec[num]; num++) { + int other, found_dup; + if (ps_matched[num]) continue; + /* + * The caller might have fed identical pathspec + * twice. Do not barf on such a mistake. + */ + for (found_dup = other = 0; + !found_dup && pathspec[other]; + other++) { + if (other == num || !ps_matched[other]) + continue; + if (!strcmp(pathspec[other], pathspec[num])) + /* + * Ok, we have a match already. + */ + found_dup = 1; + } + if (found_dup) + continue; + error("pathspec '%s' did not match any file(s) known to git.", pathspec[num] + prefix_offset); errors++;