threads / discuss / 12225

Built-in checkout: wrong behaviour in subdirectories

Subject: Built-in checkout: wrong behaviour in subdirectories

## tl;dr

6 messages between Feb 20, 2008 and Feb 21, 2008.

replies: 5people: 3as markdown or json

Clemens Buchacher· Feb 20, 2008, 22:35 UTC · lore
Hi,
The following test shows a problem since commit 782c2d6 (Build in checkout):

mkdir testrep cd testrep git init

: >file1 git add file1 git commit -m 'initial'

git branch dev

mkdir dir1 cd dir1 # this creates dir1/file1 git checkout dev

Regards, Clemens

Daniel Barkalow· Feb 20, 2008, 22:44 UTC · re: Clemens Buchacher · lore

Re: Built-in checkout: wrong behaviour in subdirectories

On Wed, 20 Feb 2008, Clemens Buchacher wrote:
Show 18 quoted lines
> Hi,
> 
> The following test shows a problem since commit 782c2d6 (Build in checkout):
> 
> mkdir testrep
> cd testrep
> git init
> 
> : >file1
> git add file1
> git commit -m 'initial'
> 
> git branch dev
> 
> mkdir dir1
> cd dir1
> # this creates dir1/file1
> git checkout dev

I was just recently wondering why that worked (in the context of trying to have builtin-clone work the same way). Guess that explains it...

Junio: it looks like something used to cause read-tree to be run with 
pwd being the working tree root in git-checkout.sh; is that something that 
should be done in setup for builtin-checkout, or should unpack_trees learn 
how to deal correctly with a non-pwd work tree root?
	-Daniel
*This .sig left intentionally blank*
Junio C Hamano· Feb 20, 2008, 23:21 UTC · re: Daniel Barkalow · lore

Re: Built-in checkout: wrong behaviour in subdirectories

Daniel Barkalow <barkalow@iabervon.org> writes:
> Junio: it looks like something used to cause read-tree to be run with 
> pwd being the working tree root in git-checkout.sh.

"Something"? I think we explicitly cdup when switching branches, and we should do the same in C rewrite.

        # We are switching branches and checking out trees, so
        # we *NEED* to be at the toplevel.
        cd_to_toplevel
Junio C Hamano· Feb 20, 2008, 23:42 UTC · re: Junio C Hamano · lore

Re: Built-in checkout: wrong behaviour in subdirectories

Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
>> Junio: it looks like something used to cause read-tree to be run with 
>> pwd being the working tree root in git-checkout.sh.
>
> "Something"?  I think we explicitly cdup when switching
> branches, and we should do the same in C rewrite.
>
>         # We are switching branches and checking out trees, so
>         # we *NEED* to be at the toplevel.
>         cd_to_toplevel

Isn't it just the matter of not passing prefix to topts in merge_working_tree()? Your other calls to unpack_trees() do not seem to pass bogus prefix to it.

IIRC, the "prefix" to unpack_trees() is not about the path limiter, but grafting a tree into a place where it originally was not intended to be (i.e. to support "read-tree --prefix").

Daniel Barkalow· Feb 21, 2008, 15:50 UTC · re: Junio C Hamano · lore

Re: Built-in checkout: wrong behaviour in subdirectories

On Wed, 20 Feb 2008, Junio C Hamano wrote:
Show 17 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Daniel Barkalow <barkalow@iabervon.org> writes:
> >
> >> Junio: it looks like something used to cause read-tree to be run with 
> >> pwd being the working tree root in git-checkout.sh.
> >
> > "Something"?  I think we explicitly cdup when switching
> > branches, and we should do the same in C rewrite.
> >
> >         # We are switching branches and checking out trees, so
> >         # we *NEED* to be at the toplevel.
> >         cd_to_toplevel
> 
> Isn't it just the matter of not passing prefix to topts in
> merge_working_tree()?  Your other calls to unpack_trees() do not
> seem to pass bogus prefix to it.

You're absolutely right; I think I initially misunderstood the way the wrapper sets things up beforehand, and did the later parts of the conversion correctly. But we should also drop the bunch of now-unused arguments, too:

--------------
commit 267d68e24111bc9054d1f3356842f58106c76f49
Author: Daniel Barkalow <barkalow@iabervon.org>
Date:   Thu Feb 21 10:48:46 2008 -0500
    Remove unused prefix arguments in switch_branches path
    
    This path doesn't actually care where in the tree you started out,
    since it must change the whole thing anyway. With the gratuitous bug
    removed, the argument is unused.
    
    Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 builtin-checkout.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index cddd3a9..46cb362 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -205,8 +205,7 @@ static void setup_branch_path(struct branch_info *branch)
 }
 
 static int merge_working_tree(struct checkout_opts *opts,
-			      struct branch_info *old, struct branch_info *new,
-			      const char *prefix)
+			      struct branch_info *old, struct branch_info *new)
 {
 	int ret;
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
@@ -429,8 +428,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 		adjust_to_tracking(new, opts);
 }
 
-static int switch_branches(struct checkout_opts *opts,
-			   struct branch_info *new, const char *prefix)
+static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 {
 	int ret = 0;
 	struct branch_info old;
@@ -471,7 +469,7 @@ static int switch_branches(struct checkout_opts *opts,
 		opts->force = 1;
 	}
 
-	ret = merge_working_tree(opts, &old, new, prefix);
+	ret = merge_working_tree(opts, &old, new);
 	if (ret)
 		return ret;
 
@@ -565,5 +563,5 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		die("Cannot switch branch to a non-commit.");
 	}
 
-	return switch_branches(&opts, &new, prefix);
+	return switch_branches(&opts, &new);
 }
Junio C Hamano· Feb 20, 2008, 23:54 UTC · re: Clemens Buchacher · lore

[PATCH] checkout: work from a subdirectory

When switching branches from a subdirectory, checkout rewritten in C extracted the toplevel of the tree in there.

This should fix it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |    1 -
 t/t7201-co.sh      |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 1fc1e56..f51b77a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -237,7 +237,6 @@ static int merge_working_tree(struct checkout_opts *opts,
 			topts.dir = xcalloc(1, sizeof(*topts.dir));
 			topts.dir->show_ignored = 1;
 			topts.dir->exclude_per_dir = ".gitignore";
-			topts.prefix = prefix;
 			tree = parse_tree_indirect(old->commit->object.sha1);
 			init_tree_desc(&trees[0], tree->buffer, tree->size);
 			tree = parse_tree_indirect(new->commit->object.sha1);
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 5492f21..0fa9467 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -263,4 +263,38 @@ test_expect_success 'checkout with ambiguous tag/branch names' '
 
 '
 
+test_expect_success 'switch branches while in subdirectory' '
+
+	git reset --hard &&
+	git checkout master &&
+
+	mkdir subs &&
+	(
+		cd subs &&
+		git checkout side
+	) &&
+	! test -f subs/one &&
+	rm -fr subs
+
+'
+
+test_expect_success 'checkout specific path while in subdirectory' '
+
+	git reset --hard &&
+	git checkout side &&
+	mkdir subs &&
+	>subs/bero &&
+	git add subs/bero &&
+	git commit -m "add subs/bero" &&
+
+	git checkout master &&
+	mkdir -p subs &&
+	(
+		cd subs &&
+		git checkout side -- bero
+	) &&
+	test -f subs/bero
+
+'
+
 test_done

← back to recent threads