git-checkout: Support relative paths containing "..".
PATCH 1/1 git-checkout: Support relative paths containing "..".
--- git-checkout.sh | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
git-checkout.sh +3 -3
@@ -133,9 +133,9 @@ Did you intend to checkout '$@' which can not be resolved as commit?" fi # Make sure the request is about existing paths. - git ls-files --error-unmatch -- "$@" >/dev/null || exit - git ls-files -- "$@" | - git checkout-index -f -u --stdin + git ls-files --full-name --error-unmatch -- "$@" >/dev/null || exit + git ls-files --full-name -- "$@" | + (cd_to_toplevel && git checkout-index -f -u --stdin) # Run a post-checkout hook -- the HEAD does not change so the # current HEAD is passed in for both args
Signature · 1 line
1.5.3.1
PATCH 1/1 git-checkout: Test for relative path use.
---
Test 5 in this series fails because of a bug in git-ls-files, where
git-ls-files t/../
(with or without --full-name) returns no files.
t/t2008-checkout-subdir.sh | 79 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 79 insertions(+), 0 deletions(-) create mode 100755 t/t2008-checkout-subdir.sh
t/t2008-checkout-subdir.sh new file +79 -0
@@ -0,0 +1,79 @@ +#!/bin/sh +# +# Copyright (c) 2007 David Symonds + +test_description='git checkout from subdirectories' + +. ./test-lib.sh + +test_expect_success setup ' + + echo "base" > file0 && + git add file0 && + mkdir dir1 && + echo "hello" > dir1/file1 && + git add dir1/file1 && + mkdir dir2 && + echo "bonjour" > dir2/file2 && + git add dir2/file2 && + test_tick && + git commit -m "populate tree" + +' + +test_expect_success 'remove and restore with relative path' ' + + cd dir1 && + rm ../file0 && + git checkout HEAD -- ../file0 && + test "base" = "$(cat ../file0)" && + rm ../dir2/file2 && + git checkout HEAD -- ../dir2/file2 && + test "bonjour" = "$(cat ../dir2/file2)" && + rm ../file0 ./file1 && + git checkout HEAD -- .. && + test "base" = "$(cat ../file0)" && + test "hello" = "$(cat file1)" && + cd - + +' + +test_expect_success 'checkout with empty prefix' ' + + rm file0 && + git checkout HEAD -- file0 && + test "base" = "$(cat file0)" + +' + +test_expect_success 'checkout with simple prefix' ' + + rm dir1/file1 && + git checkout HEAD -- dir1 && + test "hello" = "$(cat dir1/file1)" && + rm dir1/file1 && + git checkout HEAD -- dir1/file1 && + test "hello" = "$(cat dir1/file1)" + +' + +test_expect_success 'checkout with complex relative path' ' + + rm file1 && + git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1 + +' + +test_expect_failure 'relative path outside tree should fail' \ + 'git checkout HEAD -- ../../Makefile' + +test_expect_failure 'incorrect relative path to file should fail (1)' \ + 'git checkout HEAD -- ../file0' + +test_expect_failure 'incorrect relative path should fail (2)' \ + 'cd dir1 && git checkout HEAD -- ./file0' + +test_expect_failure 'incorrect relative path should fail (3)' \ + 'cd dir1 && git checkout HEAD -- ../../file0' + +test_done
Signature · 1 line
1.5.3.1
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 4 lines · David Symonds
t/t2008-checkout-subdir.sh new file +4
-0+test_expect_success 'remove and restore with relative path' ' + + cd dir1 && + rm ../file0 &&
t/t2008-checkout-subdir.sh line 37
Quoted 10 lines
t/t2008-checkout-subdir.sh new file +10
-0+ git checkout HEAD -- ../file0 && + test "base" = "$(cat ../file0)" && + rm ../dir2/file2 && + git checkout HEAD -- ../dir2/file2 && + test "bonjour" = "$(cat ../dir2/file2)" && + rm ../file0 ./file1 && + git checkout HEAD -- .. && + test "base" = "$(cat ../file0)" && + test "hello" = "$(cat file1)" && + cd -
What if this test fails? Then the rest of the tests run from the wrong directory. You should put the test in parenthesis (and drop the cd -).
-- Hannes
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 10 lines · Johannes Sixt
Quoted 9 lines · David Symonds
t/t2008-checkout-subdir.sh new file +9
-0+test_expect_success 'remove and restore with relative path' ' + + cd dir1 && + rm ../file0 && + git checkout HEAD -- ../file0 && + test "base" = "$(cat ../file0)" && + rm ../dir2/file2 && + git checkout HEAD -- ../dir2/file2 && + test "bonjour" = "$(cat ../dir2/file2)" &&
Quoted 8 lines
t/t2008-checkout-subdir.sh line 37
Quoted 5 lines
t/t2008-checkout-subdir.sh new file +5
-0+ rm ../file0 ./file1 && + git checkout HEAD -- .. && + test "base" = "$(cat ../file0)" && + test "hello" = "$(cat file1)" && + cd -What if this test fails? Then the rest of the tests run from the wrong directory. You should put the test in parenthesis (and drop the cd -).
Looking at the existing tests which, when they change directories, don't cd back to where they were; they "cd .." at the start of the next test. I'll add a "cd .." to the relevant bits of my tests.
Dave.
PATCH 1/1 git-checkout: Test for relative path use.
---
Tests that change directories now change back at the start of the next test. I don't know what to do about that last test, though.
t/t2008-checkout-subdir.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 81 insertions(+), 0 deletions(-) create mode 100755 t/t2008-checkout-subdir.sh
t/t2008-checkout-subdir.sh new file +81 -0
@@ -0,0 +1,81 @@ +#!/bin/sh +# +# Copyright (c) 2007 David Symonds + +test_description='git checkout from subdirectories' + +. ./test-lib.sh + +test_expect_success setup ' + + echo "base" > file0 && + git add file0 && + mkdir dir1 && + echo "hello" > dir1/file1 && + git add dir1/file1 && + mkdir dir2 && + echo "bonjour" > dir2/file2 && + git add dir2/file2 && + test_tick && + git commit -m "populate tree" + +' + +test_expect_success 'remove and restore with relative path' ' + + cd dir1 && + rm ../file0 && + git checkout HEAD -- ../file0 && + test "base" = "$(cat ../file0)" && + rm ../dir2/file2 && + git checkout HEAD -- ../dir2/file2 && + test "bonjour" = "$(cat ../dir2/file2)" && + rm ../file0 ./file1 && + git checkout HEAD -- .. && + test "base" = "$(cat ../file0)" && + test "hello" = "$(cat file1)" + +' + +# currently in dir1/ +test_expect_success 'checkout with empty prefix' ' + + cd .. && + rm file0 && + git checkout HEAD -- file0 && + test "base" = "$(cat file0)" + +' + +test_expect_success 'checkout with simple prefix' ' + + rm dir1/file1 && + git checkout HEAD -- dir1 && + test "hello" = "$(cat dir1/file1)" && + rm dir1/file1 && + git checkout HEAD -- dir1/file1 && + test "hello" = "$(cat dir1/file1)" + +' + +test_expect_success 'checkout with complex relative path' ' + + rm file1 && + git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1 + +' + +test_expect_failure 'relative path outside tree should fail' \ + 'git checkout HEAD -- ../../Makefile' + +test_expect_failure 'incorrect relative path to file should fail (1)' \ + 'git checkout HEAD -- ../file0' + +test_expect_failure 'incorrect relative path should fail (2)' \ + 'cd dir1 && git checkout HEAD -- ./file0' + +# currently in dir1/ +test_expect_failure 'incorrect relative path should fail (3)' \ + 'git checkout HEAD -- ../../file0' + +test_done
Signature · 1 line
1.5.3.1
Re: [PATCH] git-checkout: Test for relative path use.
In reply to David SymondsQuoted 3 lines · David Symonds
Looking at the existing tests which, when they change directories, don't cd back to where they were; they "cd .." at the start of the next test. I'll add a "cd .." to the relevant bits of my tests.
Do not follow the bad examples, please.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 7 lines · Junio C Hamano
Quoted 3 lines · David Symonds
Looking at the existing tests which, when they change directories, don't cd back to where they were; they "cd .." at the start of the next test. I'll add a "cd .." to the relevant bits of my tests.
Do not follow the bad examples, please.
So what would you prefer? Bracketing the whole test in parentheses looks ugly, but I can do that if that's the only option. If I look at t5510-fetch.sh (one of yours, Junio), there is no directory restoration in the case of test failure, as in my original patch.
Perhaps test_ok_ and test_failure_ in test-lib.sh should restore the directory?
Dave.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 4 lines · David Symonds
So what would you prefer? Bracketing the whole test in parentheses looks ugly, but I can do that if that's the only option. If I look at
t5510-fetch.sh(one of yours, Junio), there is no directory restoration in the case of test failure, as in my original patch.
Yes, that is what I was referring to as "bad examples". The way t4116 goes down to different directory do not look ugly to me.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 9 lines · Junio C Hamano
Quoted 4 lines · David Symonds
So what would you prefer? Bracketing the whole test in parentheses looks ugly, but I can do that if that's the only option. If I look at
t5510-fetch.sh(one of yours, Junio), there is no directory restoration in the case of test failure, as in my original patch.Yes, that is what I was referring to as "bad examples". The way t4116 goes down to different directory do not look ugly to me.
Okay, thanks -- that's a useful example. I'll resend the patch shortly.
Dave.
PATCH 1/1 git-checkout: Test for relative path use.
--- t/t2008-checkout-subdir.sh | 80 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 80 insertions(+), 0 deletions(-) create mode 100755 t/t2008-checkout-subdir.sh
t/t2008-checkout-subdir.sh new file +80 -0
@@ -0,0 +1,80 @@ +#!/bin/sh +# +# Copyright (c) 2007 David Symonds + +test_description='git checkout from subdirectories' + +. ./test-lib.sh + +test_expect_success setup ' + + echo "base" > file0 && + git add file0 && + mkdir dir1 && + echo "hello" > dir1/file1 && + git add dir1/file1 && + mkdir dir2 && + echo "bonjour" > dir2/file2 && + git add dir2/file2 && + test_tick && + git commit -m "populate tree" + +' + +test_expect_success 'remove and restore with relative path' ' + + ( + cd dir1 && + rm ../file0 && + git checkout HEAD -- ../file0 && + test "base" = "$(cat ../file0)" && + rm ../dir2/file2 && + git checkout HEAD -- ../dir2/file2 && + test "bonjour" = "$(cat ../dir2/file2)" && + rm ../file0 ./file1 && + git checkout HEAD -- .. && + test "base" = "$(cat ../file0)" && + test "hello" = "$(cat file1)" + ) + +' + +test_expect_success 'checkout with empty prefix' ' + + rm file0 && + git checkout HEAD -- file0 && + test "base" = "$(cat file0)" + +' + +test_expect_success 'checkout with simple prefix' ' + + rm dir1/file1 && + git checkout HEAD -- dir1 && + test "hello" = "$(cat dir1/file1)" && + rm dir1/file1 && + git checkout HEAD -- dir1/file1 && + test "hello" = "$(cat dir1/file1)" + +' + +test_expect_success 'checkout with complex relative path' ' + + rm file1 && + git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1 + +' + +test_expect_failure 'relative path outside tree should fail' \ + 'git checkout HEAD -- ../../Makefile' + +test_expect_failure 'incorrect relative path to file should fail (1)' \ + 'git checkout HEAD -- ../file0' + +test_expect_failure 'incorrect relative path should fail (2)' \ + '( cd dir1 && git checkout HEAD -- ./file0 )' + +test_expect_failure 'incorrect relative path should fail (3)' \ + '( cd dir1 && git checkout HEAD -- ../../file0 )' + +test_done
Signature · 1 line
1.5.3.1
Re: [PATCH] git-checkout: Test for relative path use.
In reply to David SymondsQuoted 5 lines · David Symonds
Signed-off-by David Symonds <[email protected]>---
Test 5 in this series fails because of a bug in git-ls-files, where
git-ls-files t/../
(with or without --full-name) returns no files.
Heh, you shouldn't do that ;-)
Seriously, that's a long standing limitation in the code, not to deal with arbitrary combination of ups and downs, but I do not think there is any fundamental reason to disallow something like:
cd Documentation && git ls-files --full-name ../t
Patches welcome.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · David Symonds
Signed-off-by David Symonds <[email protected]>---
Test 5 in this series fails because of a bug in git-ls-files, where
git-ls-files t/../
(with or without --full-name) returns no files.Heh, you shouldn't do that ;-)
Quoted 8 lines
Seriously, that's a long standing limitation in the code, not to deal with arbitrary combination of ups and downs, but I do not think there is any fundamental reason to disallow something like:
cd Documentation && git ls-files --full-name ../tPatches welcome.
So you're otherwise happy with my tests, despite one of them triggering an (unrelated to git-checkout) bug? Or would you prefer I remove that particular failure from the tests and resend?
Dave.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 7 lines · David Symonds
Quoted 5 lines · Junio C Hamano
Seriously, that's a long standing limitation in the code, not to deal with arbitrary combination of ups and downs, but I do not think there is any fundamental reason to disallow something like:
Quoted 7 lines
Quoted 3 lines
cd Documentation && git ls-files --full-name ../tPatches welcome.
So you're otherwise happy with my tests, despite one of them triggering an (unrelated to git-checkout) bug? Or would you prefer I remove that particular failure from the tests and resend?
Are you really asking my preference? A patch to ls-files to make the test pass is my preference, of course ;-).
Haven't read your tests, though, but I see capable people already commented on the initial round so I do not expect it to be problematic.
Re: [PATCH] git-checkout: Test for relative path use.
Quoted 7 lines · Junio C Hamano
Are you really asking my preference? A patch to ls-files to make the test pass is my preference, of course ;-).
Haven't read your tests, though, but I see capable people already commented on the initial round so I do not expect it to be problematic.
I'm going after low-hanging fruit whilst I get familiar with Git's internal structure. I can try to tackle this ls-files problem as a separate thing.
Dave.
Re: [PATCH] git-checkout: Test for relative path use.
In reply to Junio C Hamanofredag 09 november 2007 skrev Junio C Hamano:
Quoted 10 lines
Quoted 5 lines · David Symonds
Signed-off-by David Symonds <[email protected]>---
Test 5 in this series fails because of a bug in git-ls-files, where
git-ls-files t/../
(with or without --full-name) returns no files.Heh, you shouldn't do that ;-)
Quoted 8 lines
Seriously, that's a long standing limitation in the code, not to deal with arbitrary combination of ups and downs, but I do not think there is any fundamental reason to disallow something like:
cd Documentation && git ls-files --full-name ../tPatches welcome.
I'm for allowing it, but then it should really be all over, not just some arbitrary command. Everywhere or not at all.
-- robin