Re: [PATCH v2] test: set the realpath of CWD as TRASH_DIRECTORY
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Aug 29, 2012, 04:14 UTC
- Message-ID
- <503D973D.8040402@alum.mit.edu>
- In-Reply-To
- <7vk3wktiph.fsf@alter.siamese.dyndns.org>
On 08/27/2012 06:15 PM, Junio C Hamano wrote:
Show 24 quoted lines
> Jiang Xin <worldhello.net@gmail.com> writes: > >> Some testcases will fail if current work directory is on a symlink. >> >> symlink$ sh ./t4035-diff-quiet.sh >> $ sh ./t4035-diff-quiet.sh --root=/symlink >> $ TEST_OUTPUT_DIRECTORY=/symlink sh ./t4035-diff-quiet.sh >> >> This is because the realpath of ".git" directory will be returned when >> running the command 'git rev-parse --git-dir' in a subdir of the work >> tree, and the realpath may not equal to "$TRASH_DIRECTORY". >> >> In this fix, "$TRASH_DIRECTORY" is determined right after the realpath >> of CWD is resolved. >> >> Signed-off-by: Jiang Xin <worldhello.net@gmail.com> >> Reported-by: Michael Haggerty <mhagger@alum.mit.edu> >> Signed-off-by: Jiang Xin <worldhello.net@gmail.com> >> --- > > I think this is in line with what was discussed in the other thread > Michael brought this up. Thanks for following it through. > > Michael, this looks good to me; anything I missed?
I can confirm that this patch eliminates the test failures that I was seeing in t4035 and t9903.
But it also changes almost 600 *other* tests from "succeed even in the presence of symlinks" to "never tested in the presence of symlinks", and I think that is surrendering more ground than necessary. I would rather see one of the following approaches:
*If* the official policy is that GIT_CEILING_DIRECTORIES is not reliable in the presence of symlinks, then (a) that limitation should be mentioned in the documentation; (b) the affected tests should either be skipped in the case of symlinked directories or they (alone!) should take measures to work around the problem.
*If* the official policy is that GIT_CEILING_DIRECTORIES should work in the presence of symlinks, then (a) we should add a failing test case that specifically documents this bug; (b) other tests that fail as a side effect of this bug even though they are trying to test something else should either be skipped in the case of symlinked directories or they (alone!) should take measures to work around the problem; (c) the bug should be fixed as soon as possible.
In fact, we could even go further:
* The "cd -P" in test-lib.sh could be changed to "cd -L", to avoid masking other problems related to symlinks. If I make that change, I get test failures in 10 files when using --root=/symlinkeddir, and not all of them are obviously related to GIT_CEILING_DIRECTORIES. Some of these might simply be sloppiness in how the test scripts are written, but others might be bugs in git proper.
* We could *intentionally* access the trash directories via a symlink on systems that support symlinks (much like the trash directory names intentionally include a space) to get *systematic* test coverage of symlink issues, rather than occasional testing and mysterious failures when somebody happens to have a symlink in his build path or root.
(But it is still the case that I don't have time to work on this.)
Michael
-- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/