threads / patch / 10716

patchgit-checkout: Handle relative paths containing "..".

Subject: [PATCH] git-checkout: Handle relative paths containing "..".

## tl;dr

4 messages between Nov 8, 2007 and Nov 8, 2007. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

David Symonds· Nov 8, 2007, 02:33 UTC · lore

Based on gitte's idea.

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 git-checkout.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Show changes to git-checkout.sh +4 −3

diff --git a/git-checkout.sh b/git-checkout.sh
index 8993920..b2c50aa 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -134,9 +134,10 @@ 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 "$(git-rev-parse --show-cdup)" &&
+		 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.5.1529.g69a1-dirty

Junio C Hamano· Nov 8, 2007, 08:30 UTC · re: David Symonds · lore

Re: [PATCH] git-checkout: Handle relative paths containing "..".

David Symonds <dsymonds@gmail.com> writes:

Show 15 quoted lines

> diff --git a/git-checkout.sh b/git-checkout.sh
> index 8993920..b2c50aa 100755
> --- a/git-checkout.sh
> +++ b/git-checkout.sh
> @@ -134,9 +134,10 @@ 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 "$(git-rev-parse --show-cdup)" &&
> +		 git checkout-index -f -u --stdin)

Have you tested this patch from the toplevel of any tree, where "git-rev-parse --show-cdup" would yield an empty string?

I also wonder how this patch (with an obvious fix to address the above point) would interact with GIT_DIR and/or GIT_WORK_TREE in the environment.

David Symonds· Nov 8, 2007, 13:10 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-checkout: Handle relative paths containing "..".

On Nov 8, 2007 7:30 PM, Junio C Hamano <junio@pobox.com> wrote:

>
> Have you tested this patch from the toplevel of any tree, where
> "git-rev-parse --show-cdup" would yield an empty string?

No, I didn't. Arguably, "git-rev-parse --show-cdup" should always return a path to the top-level, which would make this kind of construction much simpler. Is there anything that relies upon it returning the empty string when you're in the top-level directory?

> I also wonder how this patch (with an obvious fix to address the
> above point) would interact with GIT_DIR and/or GIT_WORK_TREE in
> the environment.

No idea. I'm still learning my way around the git codebase, so I was hoping for some review and feedback from more experienced Gits.

Dave.

Johannes Schindelin· Nov 8, 2007, 14:25 UTC · re: David Symonds · lore

Re: [PATCH] git-checkout: Handle relative paths containing "..".

Hi,

On Fri, 9 Nov 2007, David Symonds wrote:

Show 8 quoted lines

> On Nov 8, 2007 7:30 PM, Junio C Hamano <junio@pobox.com> wrote:
> >
> > Have you tested this patch from the toplevel of any tree, where
> > "git-rev-parse --show-cdup" would yield an empty string?
> 
> No, I didn't. Arguably, "git-rev-parse --show-cdup" should always return 
> a path to the top-level, which would make this kind of construction much 
> simpler.

As it is, we have a convenience function for this, to make it much simpler: cd_to_toplevel in git-sh-setup.

Show 6 quoted lines

> > I also wonder how this patch (with an obvious fix to address the above 
> > point) would interact with GIT_DIR and/or GIT_WORK_TREE in the 
> > environment.
> 
> No idea. I'm still learning my way around the git codebase, so I was
> hoping for some review and feedback from more experienced Gits.

It _should_ work with GIT_DIR/GIT_WORK_TREE, as the full name is relative to the project root, and that's exactly where cd_to_toplevel is jumping to. And the first call to ls-files should make certain that the paths are correct, so there should be no confusion either.

But yes, I have been burnt by that work tree stuff too many times, so I'd appreciate tests for that (both positive and negative ones).

Ciao, Dscho