{"thread":{"id":"30860","subject":"[PATCH 1/2] diff: handle relative paths in no-index","startedAt":"2012-06-21T18:09:50Z","lastAt":"2012-06-21T18:09:51Z","messageCount":2,"participants":["Tim Henigan"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"194046","messageId":"1340302191-23444-1-git-send-email-tim.henigan@gmail.com","threadId":"30860","inReplyTo":null,"subject":"[PATCH 1/2] diff: handle relative paths in no-index","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-06-21T18:09:50Z","receivedAt":"2012-06-21T18:09:50Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nWhen diff-no-index is given a relative path to a file outside the\nrepository, it aborts with error. However, if the file is given\nusing an absolute path, the diff runs as expected. The two cases\nshould be treated the same.\n\nTests and commit message by Tim Henigan.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Tim Henigan <tim.henigan@gmail.com>\n---\n\nJeff King implemented these changes as part of a discussion on the list.\nHe gave me permission to post to them as a patch on his behalf [1].\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/200160/focus=200301\n\n\n cache.h                  |  1 +\n diff-no-index.c          | 21 ++-------------------\n setup.c                  | 24 ++++++++++++++++++++++--\n t/t4053-diff-no-index.sh | 15 ++++++++++++++-\n 4 files changed, 39 insertions(+), 22 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 06413e1..0bd14ca 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -411,6 +411,7 @@ extern const char *prefix_filename(const char *prefix, int len, const char *path\n extern int check_filename(const char *prefix, const char *name);\n extern void verify_filename(const char *prefix, const char *name);\n extern void verify_non_filename(const char *prefix, const char *name);\n+extern int path_inside_repo(const char *prefix, const char *path);\n \n #define INIT_DB_QUIET 0x0001\n \ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f0b0010..e6b9952 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -151,23 +151,6 @@ static int queue_diff(struct diff_options *o,\n \t}\n }\n \n-static int path_outside_repo(const char *path)\n-{\n-\tconst char *work_tree;\n-\tsize_t len;\n-\n-\tif (!is_absolute_path(path))\n-\t\treturn 0;\n-\twork_tree = get_git_work_tree();\n-\tif (!work_tree)\n-\t\treturn 1;\n-\tlen = strlen(work_tree);\n-\tif (strncmp(path, work_tree, len) ||\n-\t    (path[len] != '\\0' && path[len] != '/'))\n-\t\treturn 1;\n-\treturn 0;\n-}\n-\n void diff_no_index(struct rev_info *revs,\n \t\t   int argc, const char **argv,\n \t\t   int nongit, const char *prefix)\n@@ -197,8 +180,8 @@ void diff_no_index(struct rev_info *revs,\n \t\t * a colourful \"diff\" replacement.\n \t\t */\n \t\tif ((argc != i + 2) ||\n-\t\t    (!path_outside_repo(argv[i]) &&\n-\t\t     !path_outside_repo(argv[i+1])))\n+\t\t    (path_inside_repo(prefix, argv[i]) &&\n+\t\t     path_inside_repo(prefix, argv[i+1])))\n \t\t\treturn;\n \t}\n \tif (argc != i + 2)\ndiff --git a/setup.c b/setup.c\nindex 731851a..2cfa037 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -4,7 +4,7 @@\n static int inside_git_dir = -1;\n static int inside_work_tree = -1;\n \n-char *prefix_path(const char *prefix, int len, const char *path)\n+static char *prefix_path_gently(const char *prefix, int len, const char *path)\n {\n \tconst char *orig = path;\n \tchar *sanitized;\n@@ -31,7 +31,8 @@ char *prefix_path(const char *prefix, int len, const char *path)\n \t\tif (strncmp(sanitized, work_tree, len) ||\n \t\t    (len > root_len && sanitized[len] != '\\0' && sanitized[len] != '/')) {\n \t\terror_out:\n-\t\t\tdie(\"'%s' is outside repository\", orig);\n+\t\t\tfree(sanitized);\n+\t\t\treturn NULL;\n \t\t}\n \t\tif (sanitized[len] == '/')\n \t\t\tlen++;\n@@ -40,6 +41,25 @@ char *prefix_path(const char *prefix, int len, const char *path)\n \treturn sanitized;\n }\n \n+char *prefix_path(const char *prefix, int len, const char *path)\n+{\n+\tchar *r = prefix_path_gently(prefix, len, path);\n+\tif (!r)\n+\t\tdie(\"'%s' is outside repository\", path);\n+\treturn r;\n+}\n+\n+int path_inside_repo(const char *prefix, const char *path)\n+{\n+\tint len = prefix ? strlen(prefix) : 0;\n+\tchar *r = prefix_path_gently(prefix, len, path);\n+\tif (r) {\n+\t\tfree(r);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n int check_filename(const char *prefix, const char *arg)\n {\n \tconst char *name;\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 4dc8c67..979e983 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -8,7 +8,12 @@ test_expect_success 'setup' '\n \tmkdir a &&\n \tmkdir b &&\n \techo 1 >a/1 &&\n-\techo 2 >a/2\n+\techo 2 >a/2 &&\n+\tgit init repo &&\n+\techo 1 >repo/a &&\n+\tmkdir -p non/git &&\n+\techo 1 >non/git/a &&\n+\techo 1 >non/git/b\n '\n \n test_expect_success 'git diff --no-index directories' '\n@@ -16,4 +21,12 @@ test_expect_success 'git diff --no-index directories' '\n \ttest $? = 1 && test_line_count = 14 cnt\n '\n \n+test_expect_success 'git diff --no-index relative path outside repo' '\n+\t(\n+\t\tcd repo &&\n+\t\ttest_expect_code 0 git diff --no-index a ../non/git/a &&\n+\t\ttest_expect_code 0 git diff --no-index ../non/git/a ../non/git/b\n+\t)\n+'\n+\n test_done\n-- \n1.7.11.3.gf4ddae1\n"},{"id":"194047","messageId":"1340302191-23444-2-git-send-email-tim.henigan@gmail.com","threadId":"30860","inReplyTo":"1340302191-23444-1-git-send-email-tim.henigan@gmail.com","subject":"[PATCH 2/2] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-06-21T18:09:51Z","receivedAt":"2012-06-21T18:09:51Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"When running 'git diff --quiet <file1> <file2>', if file1 or file2\nis outside the repository, it will exit(0) even if the files differ.\nIt should exit(1) when they differ.\n\nThis happens because 'diff_no_index' uses the 'found_changes' member\nfrom 'diff_options' to determine if changes were made. This is the\nwrong flag, since it is only set if xdiff is actually run and it\nfinds a change. The diff machinery will optimize out the xdiff call\nwhen it is not necessary.\n\n'diff_no_index' needs to check the 'HAS_CHANGES' flag instead, which\nis done in the 'diff_result_code' function. This matches the code\npaths used for regular index-aware diff.\n\nSigned-off-by: Tim Henigan <tim.henigan@gmail.com>\n---\n\nPatch 1/2 is new to this series, but 3 earlier drafts of this patch\n(2/2) were sent to the list for review.\n\nChanges in this version:\n  - Improved commit message based on suggestions from Jeff King.\n  - Removed declaration after statement in diff-no-index.c.\n  - Removed space after redirection operator in t4035.\n  - Changed non-git paths in t4035 to match naming used in t7810.\n  - Changed non-git paths to be relative rather than absolute.\n\n\n diff-no-index.c       |  2 +-\n t/t4035-diff-quiet.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 73 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex e6b9952..63c31cc 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -256,5 +256,5 @@ void diff_no_index(struct rev_info *revs,\n \t * The return code for --no-index imitates diff(1):\n \t * 0 = no changes, 1 = changes, else error\n \t */\n-\texit(revs->diffopt.found_changes);\n+\texit(diff_result_code(&revs->diffopt, 0));\n }\ndiff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh\nindex cdb9202..231412d 100755\n--- a/t/t4035-diff-quiet.sh\n+++ b/t/t4035-diff-quiet.sh\n@@ -10,7 +10,22 @@ test_expect_success 'setup' '\n \tgit commit -m first &&\n \techo 2 >b &&\n \tgit add . &&\n-\tgit commit -a -m second\n+\tgit commit -a -m second &&\n+\tmkdir -p test-outside/repo && (\n+\t\tcd test-outside/repo &&\n+\t\tgit init &&\n+\t\techo \"1 1\" >a &&\n+\t\tgit add . &&\n+\t\tgit commit -m 1\n+\t) &&\n+\tmkdir -p test-outside/non/git && (\n+\t\tcd test-outside/non/git &&\n+\t\techo \"1 1\" >a &&\n+\t\techo \"1 1\" >matching-file &&\n+\t\techo \"1 1 \" >trailing-space &&\n+\t\techo \"1   1\" >extra-space &&\n+\t\techo \"2\" >never-match\n+\t)\n '\n \n test_expect_success 'git diff-tree HEAD^ HEAD' '\n@@ -77,4 +92,60 @@ test_expect_success 'git diff-index --cached HEAD' '\n \t}\n '\n \n+test_expect_success 'git diff, one file outside repo' '\n+\t(\n+\t\tcd test-outside/repo &&\n+\t\ttest_expect_code 0 git diff --quiet a ../non/git/matching-file &&\n+\t\ttest_expect_code 1 git diff --quiet a ../non/git/extra-space\n+\t)\n+'\n+\n+test_expect_success 'git diff, both files outside repo' '\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$TRASH_DIRECTORY/test-outside\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd test-outside/non/git &&\n+\t\ttest_expect_code 0 git diff --quiet a matching-file &&\n+\t\ttest_expect_code 1 git diff --quiet a extra-space\n+\t)\n+'\n+\n+test_expect_success 'git diff --ignore-space-at-eol, one file outside repo' '\n+\t(\n+\t\tcd test-outside/repo &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-space-at-eol a ../non/git/trailing-space &&\n+\t\ttest_expect_code 1 git diff --quiet --ignore-space-at-eol a ../non/git/extra-space\n+\t)\n+'\n+\n+test_expect_success 'git diff --ignore-space-at-eol, both files outside repo' '\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$TRASH_DIRECTORY/test-outside\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd test-outside/non/git &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-space-at-eol a trailing-space &&\n+\t\ttest_expect_code 1 git diff --quiet --ignore-space-at-eol a extra-space\n+\t)\n+'\n+\n+test_expect_success 'git diff --ignore-all-space, one file outside repo' '\n+\t(\n+\t\tcd test-outside/repo &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/trailing-space &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-all-space a ../non/git/extra-space &&\n+\t\ttest_expect_code 1 git diff --quiet --ignore-all-space a ../non/git/never-match\n+\t)\n+'\n+\n+test_expect_success 'git diff --ignore-all-space, both files outside repo' '\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$TRASH_DIRECTORY/test-outside\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd test-outside/non/git &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-all-space a trailing-space &&\n+\t\ttest_expect_code 0 git diff --quiet --ignore-all-space a extra-space &&\n+\t\ttest_expect_code 1 git diff --quiet --ignore-all-space a never-match\n+\t)\n+'\n+\n test_done\n-- \n1.7.11.3.gf4ddae1\n"}]}