Re: [PATCH] test: some testcases failed if cwd is on a symlink
On 08/19/2012 06:43 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: I just verified that the combination of your two suggestions (i.e., the patch below) fixes the problem for me. Good to know. The only remaining two worries from me are if everybody has working pwd at that early point in the script (I think MINGW replaces pwd with its own), and if the latter one should really be /bin/pwd everywhere. Saying Give the true path to --root when you run it can sidestep the latter issue ;-) Nevertheless, I'm not sure that this is the best solution. The test failures that occur without this change suggest to me that GIT_CEILING_DIRECTORIES is implemented in a fragile way. Hrmph. How would you improve it? chdir() around twice and compare? I believe that the old-school method is to stat(2) the two directories and check whether their device IDs and inode numbers are identical. But I don't know whether that is portable to other allegedly POSIX-compatible OSs, or even works with all modern filesystems (I think there was just a thread about a FUSE filesystem that sometimes changes inode numbers). Another alternative is to write a function that knows how to convert an arbitrary path into an absolute path, including converting relative paths to absolute, resolving symlinks, eliminating redundant / and ., resolving .., and perhaps canonicalizing / vs. \ and who-knows-what-else on Windows. It takes a bit of care to implement this correctly, but it might be a useful function to have around. Python's library function os.path.realpath() is an example [1]. Either approach would avoid chdir()ing around even temporarily, which would anyway be bad form in git proper (as opposed to the test suite). And it would avoid the need to chdir() permanently in the test suite, which can have the effect of making directory names appear in unfamiliar forms. I'm afraid I don't have time to work on this now; I'm still trying to finish the next iteration of the post-receive-email hook script replacement. Michael [1] (Python) source code here: http://hg.python.org/cpython/file/20985f52b65e/Lib/posixfile.py -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
On 08/18/2012 10:36 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: I can work around the problem by using --root=/run/shm. I do not necessarily think it is a work around. http://en.wiktionary.org/wiki/workaround: 2. (computing) A procedure or a temporary fix that bypasses a problem and allows the user to continue working until a better solution can be provided; a kluge. For me that is exactly what it was. A low-impact approach may be to update the part that parses --root option to do root=$(...) root=$( cd $root /bin/pwd ) or something. I just verified that the combination of your two suggestions (i.e., the patch below) fixes the problem for me. Nevertheless, I'm not sure that this is the best solution. The test failures that occur without this change suggest to me that GIT_CEILING_DIRECTORIES is implemented in a fragile way. Michael diff --git a/t/test-lib.sh b/t/test-lib.sh index bb4f886..c7f320f 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/ . +cd $(pwd -P) + # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. case $GIT_TEST_TEE_STARTED, $* in @@ -166,6 +168,7 @@ do shift ;; # was handled already --root=*) root=$(expr z$1 : 'z[^=]*=\(.*\)') + root=$(cd $root /bin/pwd) shift ;; *) echo error: unknown test option '$1' 2; exit 1 ;; -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
Junio C Hamano gitster at pobox.com writes: Jiang Xin worldhello.net at gmail.com writes: Run command 'git rev-parse --git-dir' under subdir will return realpath of '.git' directory. Some test scripts compare this realpath against $TRASH_DIRECTORY, they are not equal if current working directory is on a symlink. In this fix, get realpath of $TRASH_DIRECTORY, store it in $TRASH_REALPATH variable, and use it when necessary. I wonder if running test in a real directory (in other words, fix your cwd) may be a simpler, more robust and generally a better solution, e.g. something silly like... diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..7f6fb0a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/ . +cd $(pwd -P) + # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. case $GIT_TEST_TEE_STARTED, $* in What is the status of this bug? Today I wasted a bunch of time trying to track down a build breakage that was ultimately caused by this problem. I was running the test suite on master with --root=/dev/shm (my usual setting), but this caused tests t4035 and t9903 to fail as described upthread. (It turns out that on my system, /dev/shm is a symlink to /run/shm.) For me, the failure is fixed by Jiang Xin's patch, but it is not fixed by Junio's. In the case of t4035 in failing test git diff --ignore-all-space, both files outside repo, right before git diff is called, PWD=/run/shm/trash directory.t4035-diff-quiet/test-outside/non/git GIT_CEILING_DIRECTORIES=/dev/shm/trash directory.t4035-diff-quiet/test-outside I can work around the problem by using --root=/run/shm. But it would be good to get this problem fixed one way or the other to spare other people the same pain. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
Michael Haggerty mhag...@alum.mit.edu writes: I can work around the problem by using --root=/run/shm. I do not necessarily think it is a work around. A low-impact approach may be to update the part that parses --root option to do root=$(...) root=$( cd $root /bin/pwd ) or something. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
Some grammatical nits about the commit message. I hope this doesn't come across as too picky/annoying ... And you might want to wait for a native to confirm whether these nits are actually all warranted. On 07/24/2012 10:00 AM, Jiang Xin wrote: Run s/Run/Running/ command 'git rev-parse --git-dir' under subdir s/under subdir/under a subdir/. Or even better IMHO, s/under subdir/in a subdir/ will return realpath s/realpath/the realpath/ of '.git' directory. s/of/of the/ Some test scripts compare this realpath against $TRASH_DIRECTORY, they are not equal s/they are not/but they are not/ if current working directory is on a symlink. s/current/the current/ In this fix, get realpath s/realpath/the realpath/ of $TRASH_DIRECTORY, store it in $TRASH_REALPATH variable, and use it when necessary. Signed-off-by: Jiang Xin worldhello@gmail.com --- t/t4035-diff-quiet.sh | 8 +--- t/t9903-bash-prompt.sh | 13 +++-- 2 个文件被修改，插入 12 行(+)，删除 9 行(-) diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index 23141..5855 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh @@ -4,6 +4,8 @@ test_description='Return value of diffs' . ./test-lib.sh +TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) + BTW, the outer quotes are not needed; this is enough: TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
worldhello@gmail.com wrote on Tue, 24 Jul 2012 16:00 +0800: Run command 'git rev-parse --git-dir' under subdir will return realpath of '.git' directory. Some test scripts compare this realpath against $TRASH_DIRECTORY, they are not equal if current working directory is on a symlink. In this fix, get realpath of $TRASH_DIRECTORY, store it in $TRASH_REALPATH variable, and use it when necessary. We have the same problem with p4. I fixed it in t98* in 23bd0c9 (git p4 test: use real_path to resolve p4 client symlinks, 2012-06-27), but maybe an exported TRASH_DIRECTORY_REAL_PATH might be generally useful. +TRASH_REALPATH=$(cd $TRASH_DIRECTORY; pwd -P) There's a portable test helper that does this already: path=$(test-path-utils real_path $path) -- Pete -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: some testcases failed if cwd is on a symlink
2012/7/24 Junio C Hamano gits...@pobox.com: I wonder if running test in a real directory (in other words, fix your cwd) may be a simpler, more robust and generally a better solution, e.g. something silly like... diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..7f6fb0a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/ . +cd $(pwd -P) + Yes, it's much simpler. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
