Skip to content

Commit

Permalink
ls-files: fix pathspec display on error
Browse files Browse the repository at this point in the history
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 <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Clemens Buchacher authored and gitster committed Aug 11, 2011
1 parent d5b6629 commit 0f64bfa
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 10 deletions.
6 changes: 3 additions & 3 deletions builtin/checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion builtin/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions builtin/ls-files.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions quote.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
70 changes: 70 additions & 0 deletions t/t3005-ls-files-relative.sh
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 0f64bfa

Please sign in to comment.