{"thread":{"id":"12225","subject":"Built-in checkout: wrong behaviour in subdirectories","startedAt":"2008-02-20T22:35:20Z","lastAt":"2008-02-21T15:50:42Z","messageCount":6,"participants":["Clemens Buchacher","Daniel Barkalow","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"69413","messageId":"20080220223520.GA23642@localhost","threadId":"12225","inReplyTo":null,"subject":"Built-in checkout: wrong behaviour in subdirectories","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2008-02-20T22:35:20Z","receivedAt":"2008-02-20T22:35:20Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi,\n\nThe following test shows a problem since commit 782c2d6 (Build in checkout):\n\nmkdir testrep\ncd testrep\ngit init\n\n: >file1\ngit add file1\ngit commit -m 'initial'\n\ngit branch dev\n\nmkdir dir1\ncd dir1\n# this creates dir1/file1\ngit checkout dev\n\nRegards,\nClemens\n"},{"id":"69416","messageId":"alpine.LNX.1.00.0802201740050.19024@iabervon.org","threadId":"12225","inReplyTo":"20080220223520.GA23642@localhost","subject":"Re: Built-in checkout: wrong behaviour in subdirectories","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-20T22:44:44Z","receivedAt":"2008-02-20T22:44:44Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 20 Feb 2008, Clemens Buchacher wrote:\n\n> Hi,\n> \n> The following test shows a problem since commit 782c2d6 (Build in checkout):\n> \n> mkdir testrep\n> cd testrep\n> git init\n> \n> : >file1\n> git add file1\n> git commit -m 'initial'\n> \n> git branch dev\n> \n> mkdir dir1\n> cd dir1\n> # this creates dir1/file1\n> git checkout dev\n\nI was just recently wondering why that worked (in the context of trying to \nhave builtin-clone work the same way). Guess that explains it...\n\nJunio: it looks like something used to cause read-tree to be run with \npwd being the working tree root in git-checkout.sh; is that something that \nshould be done in setup for builtin-checkout, or should unpack_trees learn \nhow to deal correctly with a non-pwd work tree root?\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"69424","messageId":"7vhcg32p0i.fsf@gitster.siamese.dyndns.org","threadId":"12225","inReplyTo":"alpine.LNX.1.00.0802201740050.19024@iabervon.org","subject":"Re: Built-in checkout: wrong behaviour in subdirectories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-20T23:21:01Z","receivedAt":"2008-02-20T23:21:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> Junio: it looks like something used to cause read-tree to be run with \n> pwd being the working tree root in git-checkout.sh.\n\n\"Something\"? I think we explicitly cdup when switching\nbranches, and we should do the same in C rewrite.\n\n # We are switching branches and checking out trees, so\n # we *NEED* to be at the toplevel.\n cd_to_toplevel\n"},{"id":"69428","messageId":"7vd4qr2o03.fsf@gitster.siamese.dyndns.org","threadId":"12225","inReplyTo":"7vhcg32p0i.fsf@gitster.siamese.dyndns.org","subject":"Re: Built-in checkout: wrong behaviour in subdirectories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-20T23:42:52Z","receivedAt":"2008-02-20T23:42:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Daniel Barkalow writes:\n>\n>> Junio: it looks like something used to cause read-tree to be run with \n>> pwd being the working tree root in git-checkout.sh.\n>\n> \"Something\"? I think we explicitly cdup when switching\n> branches, and we should do the same in C rewrite.\n>\n> # We are switching branches and checking out trees, so\n> # we *NEED* to be at the toplevel.\n> cd_to_toplevel\n\nIsn't it just the matter of not passing prefix to topts in\nmerge_working_tree()? Your other calls to unpack_trees() do not\nseem to pass bogus prefix to it.\n\nIIRC, the \"prefix\" to unpack_trees() is not about the path\nlimiter, but grafting a tree into a place where it originally\nwas not intended to be (i.e. to support \"read-tree --prefix\").\n"},{"id":"69429","messageId":"7vy79f18vl.fsf@gitster.siamese.dyndns.org","threadId":"12225","inReplyTo":"20080220223520.GA23642@localhost","subject":"[PATCH] checkout: work from a subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-20T23:54:54Z","receivedAt":"2008-02-20T23:54:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When switching branches from a subdirectory, checkout rewritten\nin C extracted the toplevel of the tree in there.\n\nThis should fix it.\n\nSigned-off-by: Junio C Hamano \n---\n\n builtin-checkout.c | 1 -\n t/t7201-co.sh | 34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 1fc1e56..f51b77a 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -237,7 +237,6 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n \t\t\ttopts.dir->show_ignored = 1;\n \t\t\ttopts.dir->exclude_per_dir = \".gitignore\";\n-\t\t\ttopts.prefix = prefix;\n \t\t\ttree = parse_tree_indirect(old->commit->object.sha1);\n \t\t\tinit_tree_desc(&trees[0], tree->buffer, tree->size);\n \t\t\ttree = parse_tree_indirect(new->commit->object.sha1);\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 5492f21..0fa9467 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -263,4 +263,38 @@ test_expect_success 'checkout with ambiguous tag/branch names' '\n \n '\n \n+test_expect_success 'switch branches while in subdirectory' '\n+\n+\tgit reset --hard &&\n+\tgit checkout master &&\n+\n+\tmkdir subs &&\n+\t(\n+\t\tcd subs &&\n+\t\tgit checkout side\n+\t) &&\n+\t! test -f subs/one &&\n+\trm -fr subs\n+\n+'\n+\n+test_expect_success 'checkout specific path while in subdirectory' '\n+\n+\tgit reset --hard &&\n+\tgit checkout side &&\n+\tmkdir subs &&\n+\t>subs/bero &&\n+\tgit add subs/bero &&\n+\tgit commit -m \"add subs/bero\" &&\n+\n+\tgit checkout master &&\n+\tmkdir -p subs &&\n+\t(\n+\t\tcd subs &&\n+\t\tgit checkout side -- bero\n+\t) &&\n+\ttest -f subs/bero\n+\n+'\n+\n test_done\n"},{"id":"69480","messageId":"alpine.LNX.1.00.0802211041190.19024@iabervon.org","threadId":"12225","inReplyTo":"7vd4qr2o03.fsf@gitster.siamese.dyndns.org","subject":"Re: Built-in checkout: wrong behaviour in subdirectories","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-21T15:50:42Z","receivedAt":"2008-02-21T15:50:42Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 20 Feb 2008, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Daniel Barkalow writes:\n> >\n> >> Junio: it looks like something used to cause read-tree to be run with \n> >> pwd being the working tree root in git-checkout.sh.\n> >\n> > \"Something\"? I think we explicitly cdup when switching\n> > branches, and we should do the same in C rewrite.\n> >\n> > # We are switching branches and checking out trees, so\n> > # we *NEED* to be at the toplevel.\n> > cd_to_toplevel\n> \n> Isn't it just the matter of not passing prefix to topts in\n> merge_working_tree()? Your other calls to unpack_trees() do not\n> seem to pass bogus prefix to it.\n\nYou're absolutely right; I think I initially misunderstood the way the \nwrapper sets things up beforehand, and did the later parts of the \nconversion correctly. But we should also drop the bunch of \nnow-unused arguments, too:\n\n--------------\ncommit 267d68e24111bc9054d1f3356842f58106c76f49\nAuthor: Daniel Barkalow \nDate: Thu Feb 21 10:48:46 2008 -0500\n\n Remove unused prefix arguments in switch_branches path\n \n This path doesn't actually care where in the tree you started out,\n since it must change the whole thing anyway. With the gratuitous bug\n removed, the argument is unused.\n \n Signed-off-by: Daniel Barkalow \n---\n builtin-checkout.c | 10 ++++------\n 1 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex cddd3a9..46cb362 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -205,8 +205,7 @@ static void setup_branch_path(struct branch_info *branch)\n }\n \n static int merge_working_tree(struct checkout_opts *opts,\n-\t\t\t struct branch_info *old, struct branch_info *new,\n-\t\t\t const char *prefix)\n+\t\t\t struct branch_info *old, struct branch_info *new)\n {\n \tint ret;\n \tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n@@ -429,8 +428,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,\n \t\tadjust_to_tracking(new, opts);\n }\n \n-static int switch_branches(struct checkout_opts *opts,\n-\t\t\t struct branch_info *new, const char *prefix)\n+static int switch_branches(struct checkout_opts *opts, struct branch_info *new)\n {\n \tint ret = 0;\n \tstruct branch_info old;\n@@ -471,7 +469,7 @@ static int switch_branches(struct checkout_opts *opts,\n \t\topts->force = 1;\n \t}\n \n-\tret = merge_working_tree(opts, &old, new, prefix);\n+\tret = merge_working_tree(opts, &old, new);\n \tif (ret)\n \t\treturn ret;\n \n@@ -565,5 +563,5 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Cannot switch branch to a non-commit.\");\n \t}\n \n-\treturn switch_branches(&opts, &new, prefix);\n+\treturn switch_branches(&opts, &new);\n }\n"}]}