From 546e0fd9e918b2ea3d9943124641d3b6af59a4fe Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Jun 2012 14:09:50 -0400 Subject: [PATCH 1/2] diff: handle relative paths in no-index When diff-no-index is given a relative path to a file outside the repository, it aborts with error. However, if the file is given using an absolute path, the diff runs as expected. The two cases should be treated the same. Tests and commit message by Tim Henigan. Signed-off-by: Jeff King Signed-off-by: Tim Henigan Signed-off-by: Junio C Hamano --- cache.h | 1 + diff-no-index.c | 21 ++------------------- setup.c | 24 ++++++++++++++++++++++-- t/t4053-diff-no-index.sh | 15 ++++++++++++++- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/cache.h b/cache.h index 84b8229748..8e691957e9 100644 --- a/cache.h +++ b/cache.h @@ -455,6 +455,7 @@ extern const char *prefix_filename(const char *prefix, int len, const char *path extern int check_filename(const char *prefix, const char *name); extern void verify_filename(const char *prefix, const char *name); extern void verify_non_filename(const char *prefix, const char *name); +extern int path_inside_repo(const char *prefix, const char *path); #define INIT_DB_QUIET 0x0001 diff --git a/diff-no-index.c b/diff-no-index.c index f0b0010aed..e6b9952b34 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -151,23 +151,6 @@ static int queue_diff(struct diff_options *o, } } -static int path_outside_repo(const char *path) -{ - const char *work_tree; - size_t len; - - if (!is_absolute_path(path)) - return 0; - work_tree = get_git_work_tree(); - if (!work_tree) - return 1; - len = strlen(work_tree); - if (strncmp(path, work_tree, len) || - (path[len] != '\0' && path[len] != '/')) - return 1; - return 0; -} - void diff_no_index(struct rev_info *revs, int argc, const char **argv, int nongit, const char *prefix) @@ -197,8 +180,8 @@ void diff_no_index(struct rev_info *revs, * a colourful "diff" replacement. */ if ((argc != i + 2) || - (!path_outside_repo(argv[i]) && - !path_outside_repo(argv[i+1]))) + (path_inside_repo(prefix, argv[i]) && + path_inside_repo(prefix, argv[i+1]))) return; } if (argc != i + 2) diff --git a/setup.c b/setup.c index 731851a4a8..2cfa037772 100644 --- a/setup.c +++ b/setup.c @@ -4,7 +4,7 @@ static int inside_git_dir = -1; static int inside_work_tree = -1; -char *prefix_path(const char *prefix, int len, const char *path) +static char *prefix_path_gently(const char *prefix, int len, const char *path) { const char *orig = path; char *sanitized; @@ -31,7 +31,8 @@ char *prefix_path(const char *prefix, int len, const char *path) if (strncmp(sanitized, work_tree, len) || (len > root_len && sanitized[len] != '\0' && sanitized[len] != '/')) { error_out: - die("'%s' is outside repository", orig); + free(sanitized); + return NULL; } if (sanitized[len] == '/') len++; @@ -40,6 +41,25 @@ char *prefix_path(const char *prefix, int len, const char *path) return sanitized; } +char *prefix_path(const char *prefix, int len, const char *path) +{ + char *r = prefix_path_gently(prefix, len, path); + if (!r) + die("'%s' is outside repository", path); + return r; +} + +int path_inside_repo(const char *prefix, const char *path) +{ + int len = prefix ? strlen(prefix) : 0; + char *r = prefix_path_gently(prefix, len, path); + if (r) { + free(r); + return 1; + } + return 0; +} + int check_filename(const char *prefix, const char *arg) { const char *name; diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 4dc8c67edc..979e98398b 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -8,7 +8,12 @@ test_expect_success 'setup' ' mkdir a && mkdir b && echo 1 >a/1 && - echo 2 >a/2 + echo 2 >a/2 && + git init repo && + echo 1 >repo/a && + mkdir -p non/git && + echo 1 >non/git/a && + echo 1 >non/git/b ' test_expect_success 'git diff --no-index directories' ' @@ -16,4 +21,12 @@ test_expect_success 'git diff --no-index directories' ' test $? = 1 && test_line_count = 14 cnt ' +test_expect_success 'git diff --no-index relative path outside repo' ' + ( + cd repo && + test_expect_code 0 git diff --no-index a ../non/git/a && + test_expect_code 0 git diff --no-index ../non/git/a ../non/git/b + ) +' + test_done From 304970dd5d9e542e70eca98932c1e9f26770678e Mon Sep 17 00:00:00 2001 From: Tim Henigan Date: Thu, 21 Jun 2012 14:09:51 -0400 Subject: [PATCH 2/2] diff-no-index: exit(1) if 'diff --quiet ' finds changes When running 'git diff --quiet ', if file1 or file2 is outside the repository, it will exit(0) even if the files differ. It should exit(1) when they differ. This happens because 'diff_no_index' looks at the 'found_changes' member from 'diff_options' to determine if changes were made. This is the wrong thing to do, since it is only set if xdiff is actually run and it finds a change (the diff machinery will optimize out the xdiff call when it is not necessary) and in that case HAS_CHANGED flag needs to be taken into account. Use diff_result_code() that knows all these details for the correct exit value instead. Signed-off-by: Tim Henigan Signed-off-by: Junio C Hamano --- diff-no-index.c | 2 +- t/t4035-diff-quiet.sh | 73 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index e6b9952b34..63c31cc4de 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -256,5 +256,5 @@ void diff_no_index(struct rev_info *revs, * The return code for --no-index imitates diff(1): * 0 = no changes, 1 = changes, else error */ - exit(revs->diffopt.found_changes); + exit(diff_result_code(&revs->diffopt, 0)); } diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index cdb9202f57..231412d100 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh @@ -10,7 +10,22 @@ test_expect_success 'setup' ' git commit -m first && echo 2 >b && git add . && - git commit -a -m second + git commit -a -m second && + mkdir -p test-outside/repo && ( + cd test-outside/repo && + git init && + echo "1 1" >a && + git add . && + git commit -m 1 + ) && + mkdir -p test-outside/non/git && ( + cd test-outside/non/git && + echo "1 1" >a && + echo "1 1" >matching-file && + echo "1 1 " >trailing-space && + echo "1 1" >extra-space && + echo "2" >never-match + ) ' test_expect_success 'git diff-tree HEAD^ HEAD' ' @@ -77,4 +92,60 @@ test_expect_success 'git diff-index --cached HEAD' ' } ' +test_expect_success 'git diff, one file outside repo' ' + ( + cd test-outside/repo && + test_expect_code 0 git diff --quiet a ../non/git/matching-file && + test_expect_code 1 git diff --quiet a ../non/git/extra-space + ) +' + +test_expect_success 'git diff, both files outside repo' ' + ( + GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" && + export GIT_CEILING_DIRECTORIES && + cd test-outside/non/git && + test_expect_code 0 git diff --quiet a matching-file && + test_expect_code 1 git diff --quiet a extra-space + ) +' + +test_expect_success 'git diff --ignore-space-at-eol, one file outside repo' ' + ( + cd test-outside/repo && + test_expect_code 0 git diff --quiet --ignore-space-at-eol a ../non/git/trailing-space && + test_expect_code 1 git diff --quiet --ignore-space-at-eol a ../non/git/extra-space + ) +' + +test_expect_success 'git diff --ignore-space-at-eol, both files outside repo' ' + ( + GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" && + export GIT_CEILING_DIRECTORIES && + cd test-outside/non/git && + test_expect_code 0 git diff --quiet --ignore-space-at-eol a trailing-space && + test_expect_code 1 git diff --quiet --ignore-space-at-eol a extra-space + ) +' + +test_expect_success 'git diff --ignore-all-space, one file outside repo' ' + ( + cd test-outside/repo && + test_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/trailing-space && + test_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/extra-space && + test_expect_code 1 git diff --quiet --ignore-all-space a ../non/git/never-match + ) +' + +test_expect_success 'git diff --ignore-all-space, both files outside repo' ' + ( + GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/test-outside" && + export GIT_CEILING_DIRECTORIES && + cd test-outside/non/git && + test_expect_code 0 git diff --quiet --ignore-all-space a trailing-space && + test_expect_code 0 git diff --quiet --ignore-all-space a extra-space && + test_expect_code 1 git diff --quiet --ignore-all-space a never-match + ) +' + test_done