On Tue, Jun 19, 2012 at 12:47 PM, Tim Henigan <tim.henigan@xxxxxxxxx> wrote: > On Tue, Jun 19, 2012 at 9:58 AM, Jeff King <peff@xxxxxxxx> wrote: >> On Tue, Jun 19, 2012 at 09:05:40AM -0400, Tim Henigan wrote: >> >>> As a side note, I found that these tests fail if a relative path is >>> used for the file in 'non/git'. In other words, this passes: >>> >>> test_expect_code 0 git diff --quiet a >>> "$TRASH_DIRECTORY/test-outside/non/git/matching-file" >>> >>> but this fails: >>> >>> test_expect_code 0 git diff --quiet a ../non/git/matching-file >>> >>> This surprised me, but I have not investigated any further. >> >> The problem is that path_outside_repo in diff-no-index.c does not bother >> handling relative paths at all, and just assumes they are inside the >> repository. This is obviously not true if the path starts with "..", in >> which case you would need to compare the number of ".." with the current >> depth in the repository. >> >> prefix_path already does this (and is what generates the later >> "../non/git/matching-file is not in the repository" message). We could >> perhaps get rid of path_outside_repo and just re-use prefix_path's >> logic, something like (not tested): > > With your patch applied, I was able to use relative paths in my tests. > I also confirmed that all the t4*.sh tests pass. > > For what its worth, your patch looks correct to me. Existing > consumers of 'prefix_path' should get the same results as before and > the one added xmalloc is paired with a free. Jeff, Are you planning to send this patch to the list? If not, can I include it as 1 of 2 before my patch? If we go that route, I'm not sure how to properly show you as the author... Also, in an earlier email [1] you mentioned that it may be a good idea to rename 'found_changes' to something like 'xdiff_found_changes'. I like the idea...I could submit this change as another patch in the series, if you have no objections. Thanks again for your review and help. [1]: http://thread.gmane.org/gmane.comp.version-control.git/200160/focus=200163 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html