# [PATCH] git-checkout: Test for relative path use. 15 messages from 2007-11-09 to 2007-11-09. Participants: David Symonds, Junio C Hamano, Johannes Sixt, Robin Rosenberg. Thread: https://gitlist.dev/t/10743 ## David Symonds, 2007-11-09 00:36 Subject: [PATCH] git-checkout: Support relative paths containing "..". Message-ID: <11945685673280-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11945685673280-git-send-email-dsymonds%40gmail.com ``` Signed-off-by: David Symonds --- git-checkout.sh | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-checkout.sh b/git-checkout.sh index c00cedd..aa724ac 100755 --- a/git-checkout.sh +++ b/git-checkout.sh @@ -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 -- 1.5.3.1 ``` ## David Symonds, 2007-11-09 00:36 Subject: [PATCH] git-checkout: Test for relative path use. Message-ID: <11945685732608-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11945685732608-git-send-email-dsymonds%40gmail.com In-Reply-To: <11945685673280-git-send-email-dsymonds@gmail.com> ``` Signed-off-by: David Symonds --- 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 diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh new file mode 100755 index 0000000..f226511 --- /dev/null +++ b/t/t2008-checkout-subdir.sh @@ -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 -- 1.5.3.1 ``` ## Junio C Hamano, 2007-11-09 01:28 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <7vtznwxl59.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtznwxl59.fsf%40gitster.siamese.dyndns.org In-Reply-To: <11945685732608-git-send-email-dsymonds@gmail.com> ``` David Symonds writes: > Signed-off-by: David Symonds > --- > 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. ``` ## David Symonds, 2007-11-09 01:44 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711081744p5d7b46fo88a582b9f5dbdab8%40mail.gmail.com In-Reply-To: <7vtznwxl59.fsf@gitster.siamese.dyndns.org> ``` On Nov 9, 2007 12:28 PM, Junio C Hamano wrote: > David Symonds writes: > > > Signed-off-by: David Symonds > > --- > > 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. 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. ``` ## Junio C Hamano, 2007-11-09 01:54 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <7vd4ukxjxn.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4ukxjxn.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "David Symonds" writes: > On Nov 9, 2007 12:28 PM, Junio C Hamano wrote: > >> 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. > > 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. ``` ## David Symonds, 2007-11-09 01:57 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711081757l46527904x2c962f77fa539bc%40mail.gmail.com In-Reply-To: <7vd4ukxjxn.fsf@gitster.siamese.dyndns.org> ``` On Nov 9, 2007 12:54 PM, Junio C Hamano wrote: > > 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. ``` ## Johannes Sixt, 2007-11-09 07:13 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <47340895.6000403@viscovery.net> URL: https://gitlist.dev/e/47340895.6000403%40viscovery.net In-Reply-To: <11945685732608-git-send-email-dsymonds@gmail.com> ``` David Symonds schrieb: > +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 - 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 ``` ## David Symonds, 2007-11-09 07:24 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711082324s39a9d441tc05c5a27e6d39f3e%40mail.gmail.com In-Reply-To: <47340895.6000403@viscovery.net> ``` On Nov 9, 2007 6:13 PM, Johannes Sixt wrote: > David Symonds schrieb: > > +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 - > > 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. ``` ## David Symonds, 2007-11-09 07:37 Subject: [PATCH] git-checkout: Test for relative path use. Message-ID: <11945938461226-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11945938461226-git-send-email-dsymonds%40gmail.com In-Reply-To: ``` Signed-off-by: David Symonds --- 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 diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh new file mode 100755 index 0000000..41e76c9 --- /dev/null +++ b/t/t2008-checkout-subdir.sh @@ -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 -- 1.5.3.1 ``` ## Junio C Hamano, 2007-11-09 08:04 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <7v7ikrx2st.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7ikrx2st.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "David Symonds" writes: > 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. ``` ## David Symonds, 2007-11-09 08:14 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711090014qfed56e7y446c014399e47a82%40mail.gmail.com In-Reply-To: <7v7ikrx2st.fsf@gitster.siamese.dyndns.org> ``` On Nov 9, 2007 7:04 PM, Junio C Hamano wrote: > "David Symonds" writes: > > > 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. ``` ## Junio C Hamano, 2007-11-09 09:06 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <7vfxzfvlch.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfxzfvlch.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` "David Symonds" writes: > 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. ``` ## David Symonds, 2007-11-09 09:10 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711090110s5d6c533et5e1e016a95fde943%40mail.gmail.com In-Reply-To: <7vfxzfvlch.fsf@gitster.siamese.dyndns.org> ``` On Nov 9, 2007 8:06 PM, Junio C Hamano wrote: > "David Symonds" writes: > > > 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. ``` ## David Symonds, 2007-11-09 09:12 Subject: [PATCH] git-checkout: Test for relative path use. Message-ID: <11945995483966-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11945995483966-git-send-email-dsymonds%40gmail.com In-Reply-To: ``` Signed-off-by: David Symonds --- t/t2008-checkout-subdir.sh | 80 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 80 insertions(+), 0 deletions(-) create mode 100755 t/t2008-checkout-subdir.sh diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh new file mode 100755 index 0000000..98d8eb3 --- /dev/null +++ b/t/t2008-checkout-subdir.sh @@ -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 -- 1.5.3.1 ``` ## Robin Rosenberg, 2007-11-09 19:48 Subject: Re: [PATCH] git-checkout: Test for relative path use. Message-ID: <200711092048.34868.robin.rosenberg.lists@dewire.com> URL: https://gitlist.dev/e/200711092048.34868.robin.rosenberg.lists%40dewire.com In-Reply-To: <7vtznwxl59.fsf@gitster.siamese.dyndns.org> ``` fredag 09 november 2007 skrev Junio C Hamano: > David Symonds writes: > > > Signed-off-by: David Symonds > > --- > > 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. I'm for allowing it, but then it should really be all over, not just some arbitrary command. Everywhere or not at all. -- robin ```