From 0f64bfa9567f697d6152ef9ea937d4b573029a08 Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Mon, 1 Aug 2011 23:19:58 +0200 Subject: ls-files: fix pathspec display on error The following sequence of commands reveals an issue with error reporting of relative paths: $ mkdir sub $ cd sub $ git ls-files --error-unmatch ../bbbbb error: pathspec 'b' did not match any file(s) known to git. $ git commit --error-unmatch ../bbbbb error: pathspec 'b' did not match any file(s) known to git. This bug is visible only if the normalized path (i.e., the relative path from the repository root) is longer than the prefix. Otherwise, the code skips over the normalized path and reads from an unused memory location which still contains a leftover of the original command line argument. So instead, use the existing facilities to deal with relative paths correctly. Also fix inconsistency between "checkout" and "commit", e.g. $ cd Documentation $ git checkout nosuch.txt error: pathspec 'Documentation/nosuch.txt' did not match... $ git commit nosuch.txt error: pathspec 'nosuch.txt' did not match... by propagating the prefix down the codepath that reports the error. Signed-off-by: Clemens Buchacher Signed-off-by: Junio C Hamano --- builtin/checkout.c | 6 ++-- builtin/commit.c | 2 +- builtin/ls-files.c | 10 +++++-- cache.h | 2 +- quote.c | 8 +++-- t/t3005-ls-files-relative.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 10 deletions(-) create mode 100755 t/t3005-ls-files-relative.sh diff --git a/builtin/checkout.c b/builtin/checkout.c index 28cdc51b85..a5717f1637 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -201,7 +201,7 @@ static int checkout_merged(int pos, struct checkout *state) } static int checkout_paths(struct tree *source_tree, const char **pathspec, - struct checkout_opts *opts) + const char *prefix, struct checkout_opts *opts) { int pos; struct checkout state; @@ -231,7 +231,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec, match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched); } - if (report_path_error(ps_matched, pathspec, 0)) + if (report_path_error(ps_matched, pathspec, prefix)) return 1; /* "checkout -m path" to recreate conflicted state */ @@ -1060,7 +1060,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge) die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index.")); - return checkout_paths(source_tree, pathspec, &opts); + return checkout_paths(source_tree, pathspec, prefix, &opts); } if (patch_mode) diff --git a/builtin/commit.c b/builtin/commit.c index e1af9b19f0..9679a99f99 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -272,7 +272,7 @@ static int list_paths(struct string_list *list, const char *with_tree, item->util = item; /* better a valid pointer than a fake one */ } - return report_path_error(m, pattern, prefix ? strlen(prefix) : 0); + return report_path_error(m, pattern, prefix); } static void add_remove_files(struct string_list *list) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 15701233e2..468bb13c81 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -388,11 +388,13 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix) } } -int report_path_error(const char *ps_matched, const char **pathspec, int prefix_len) +int report_path_error(const char *ps_matched, const char **pathspec, const char *prefix) { /* * Make sure all pathspec matched; otherwise it is an error. */ + struct strbuf sb = STRBUF_INIT; + const char *name; int num, errors = 0; for (num = 0; pathspec[num]; num++) { int other, found_dup; @@ -417,10 +419,12 @@ int report_path_error(const char *ps_matched, const char **pathspec, int prefix_ if (found_dup) continue; + name = quote_path_relative(pathspec[num], -1, &sb, prefix); error("pathspec '%s' did not match any file(s) known to git.", - pathspec[num] + prefix_len); + name); errors++; } + strbuf_release(&sb); return errors; } @@ -611,7 +615,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (ps_matched) { int bad; - bad = report_path_error(ps_matched, pathspec, prefix_len); + bad = report_path_error(ps_matched, pathspec, prefix); if (bad) fprintf(stderr, "Did you forget to 'git add'?\n"); diff --git a/cache.h b/cache.h index e11cf6ab1c..d55a6bbe37 100644 --- a/cache.h +++ b/cache.h @@ -1175,7 +1175,7 @@ extern int ws_blank_line(const char *line, int len, unsigned ws_rule); #define ws_tab_width(rule) ((rule) & WS_TAB_WIDTH_MASK) /* ls-files */ -int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset); +int report_path_error(const char *ps_matched, const char **pathspec, const char *prefix); void overlay_tree_on_cache(const char *tree_name, const char *prefix); char *alias_lookup(const char *alias); diff --git a/quote.c b/quote.c index 63d3b01818..532fd3b7b7 100644 --- a/quote.c +++ b/quote.c @@ -325,8 +325,12 @@ static const char *path_relative(const char *in, int len, if (len < 0) len = strlen(in); - if (prefix && prefix_len < 0) - prefix_len = strlen(prefix); + if (prefix_len < 0) { + if (prefix) + prefix_len = strlen(prefix); + else + prefix_len = 0; + } off = 0; i = 0; diff --git a/t/t3005-ls-files-relative.sh b/t/t3005-ls-files-relative.sh new file mode 100755 index 0000000000..a2b63e2c10 --- /dev/null +++ b/t/t3005-ls-files-relative.sh @@ -0,0 +1,70 @@ +#!/bin/sh + +test_description='ls-files tests with relative paths + +This test runs git ls-files with various relative path arguments. +' + +. ./test-lib.sh + +new_line=' +' +sq=\' + +test_expect_success 'prepare' ' + : >never-mind-me && + git add never-mind-me && + mkdir top && + ( + cd top && + mkdir sub && + x="x xa xbc xdef xghij xklmno" && + y=$(echo "$x" | tr x y) && + touch $x && + touch $y && + cd sub && + git add ../x* + ) +' + +test_expect_success 'ls-files with mixed levels' ' + ( + cd top/sub && + cat >expect <<-EOF && + ../../never-mind-me + ../x + EOF + git ls-files $(cat expect) >actual && + test_cmp expect actual + ) +' + +test_expect_success 'ls-files -c' ' + ( + cd top/sub && + for f in ../y* + do + echo "error: pathspec $sq$f$sq did not match any file(s) known to git." + done >expect && + echo "Did you forget to ${sq}git add${sq}?" >>expect && + ls ../x* >>expect && + test_must_fail git ls-files -c --error-unmatch ../[xy]* >actual 2>&1 && + test_cmp expect actual + ) +' + +test_expect_success 'ls-files -o' ' + ( + cd top/sub && + for f in ../x* + do + echo "error: pathspec $sq$f$sq did not match any file(s) known to git." + done >expect && + echo "Did you forget to ${sq}git add${sq}?" >>expect && + ls ../y* >>expect && + test_must_fail git ls-files -o --error-unmatch ../[xy]* >actual 2>&1 && + test_cmp expect actual + ) +' + +test_done -- cgit v1.2.3 From 385ceec1cb46f8a476fa11ffc853dedba512fd52 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sun, 28 Aug 2011 09:34:56 +0200 Subject: t3005: do not assume a particular order of stdout and stderr of git-ls-files There is no guarantee that stderr is flushed before stdout when both channels are redirected to a file. Check the channels using independent files. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t3005-ls-files-relative.sh | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/t/t3005-ls-files-relative.sh b/t/t3005-ls-files-relative.sh index a2b63e2c10..377869432e 100755 --- a/t/t3005-ls-files-relative.sh +++ b/t/t3005-ls-files-relative.sh @@ -45,11 +45,12 @@ test_expect_success 'ls-files -c' ' for f in ../y* do echo "error: pathspec $sq$f$sq did not match any file(s) known to git." - done >expect && - echo "Did you forget to ${sq}git add${sq}?" >>expect && - ls ../x* >>expect && - test_must_fail git ls-files -c --error-unmatch ../[xy]* >actual 2>&1 && - test_cmp expect actual + done >expect.err && + echo "Did you forget to ${sq}git add${sq}?" >>expect.err && + ls ../x* >expect.out && + test_must_fail git ls-files -c --error-unmatch ../[xy]* >actual.out 2>actual.err && + test_cmp expect.out actual.out && + test_cmp expect.err actual.err ) ' @@ -59,11 +60,12 @@ test_expect_success 'ls-files -o' ' for f in ../x* do echo "error: pathspec $sq$f$sq did not match any file(s) known to git." - done >expect && - echo "Did you forget to ${sq}git add${sq}?" >>expect && - ls ../y* >>expect && - test_must_fail git ls-files -o --error-unmatch ../[xy]* >actual 2>&1 && - test_cmp expect actual + done >expect.err && + echo "Did you forget to ${sq}git add${sq}?" >>expect.err && + ls ../y* >expect.out && + test_must_fail git ls-files -o --error-unmatch ../[xy]* >actual.out 2>actual.err && + test_cmp expect.out actual.out && + test_cmp expect.err actual.err ) ' -- cgit v1.2.3