{"thread":{"id":"42455","subject":"[PATCH v4 0/2] rev-parse: fix some options when executed from subpath of main tree","startedAt":"2016-05-26T11:19:14Z","lastAt":"2016-05-27T18:50:35Z","messageCount":5,"participants":["Michael Rappazzo","Mike Hommey","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"287565","messageId":"1464261556-89722-1-git-send-email-rappazzo@gmail.com","threadId":"42455","inReplyTo":null,"subject":"[PATCH v4 0/2] rev-parse: fix some options when executed from subpath of main tree","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-05-26T11:19:14Z","receivedAt":"2016-05-26T11:19:14Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Changes since v3 [1]:\n - Rebased onto 'pu' which includes the cleanup of t1500 by Eric Sunshine\n - Fixed a memory leak due to misusing xstrfmt()\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/293778\n\nMichael Rappazzo (2):\n  rev-parse tests: add tests executed from a subdirectory\n  rev-parse: fix some options when executed from subpath of main tree\n\n builtin/rev-parse.c      | 22 +++++++++++++++++-----\n t/t1500-rev-parse.sh     | 28 ++++++++++++++++++++++++++++\n t/t1700-split-index.sh   | 17 +++++++++++++++++\n t/t2027-worktree-list.sh | 10 +++++++++-\n 4 files changed, 71 insertions(+), 6 deletions(-)\n\n-- \n2.8.0\n"},{"id":"287566","messageId":"1464261556-89722-2-git-send-email-rappazzo@gmail.com","threadId":"42455","inReplyTo":"1464261556-89722-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v4 1/2] rev-parse tests: add tests executed from a subdirectory","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-05-26T11:19:15Z","receivedAt":"2016-05-26T11:19:15Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"t2027-worktree-list has an incorrect expectation for --git-common-dir\nwhich has been adjusted and marked to expect failure.\n\nSome of the tests added have been marked to expect failure.  These\ndemonstrate a problem with the way that some options to git rev-parse\nbehave when executed from a subdirectory of the main worktree.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n t/t1500-rev-parse.sh     | 28 ++++++++++++++++++++++++++++\n t/t1700-split-index.sh   | 17 +++++++++++++++++\n t/t2027-worktree-list.sh | 12 ++++++++++--\n 3 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 038e24c..f39f783 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -87,4 +87,32 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru\n \n test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n \n+test_expect_success 'git-common-dir from worktree root' '\n+\techo .git >expect &&\n+\tgit rev-parse --git-common-dir >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'git-common-dir inside sub-dir' '\n+\tmkdir -p path/to/child &&\n+\ttest_when_finished \"rm -rf path\" &&\n+\techo \"$(git -C path/to/child rev-parse --show-cdup).git\" >expect &&\n+\tgit -C path/to/child rev-parse --git-common-dir >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git-path from worktree root' '\n+\techo .git/objects >expect &&\n+\tgit rev-parse --git-path objects >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'git-path inside sub-dir' '\n+\tmkdir -p path/to/child &&\n+\ttest_when_finished \"rm -rf path\" &&\n+\techo \"$(git -C path/to/child rev-parse --show-cdup).git/objects\" >expect &&\n+\tgit -C path/to/child rev-parse --git-path objects >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 8aef49f..8ca21bd 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -200,4 +200,21 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_failure 'rev-parse --shared-index-path' '\n+\trm -rf .git &&\n+\ttest_create_repo . &&\n+\tgit update-index --split-index &&\n+\tls -t .git/sharedindex* | tail -n 1 >expect &&\n+\tgit rev-parse --shared-index-path >actual &&\n+\ttest_cmp expect actual &&\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\t(\n+\t\tcd work &&\n+\t\tls -t ../.git/sharedindex* | tail -n 1 >expect &&\n+\t\tgit rev-parse --shared-index-path >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 1b1b65a..53cc5d3 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -8,16 +8,24 @@ test_expect_success 'setup' '\n \ttest_commit init\n '\n \n-test_expect_success 'rev-parse --git-common-dir on main worktree' '\n+test_expect_failure 'rev-parse --git-common-dir on main worktree' '\n \tgit rev-parse --git-common-dir >actual &&\n \techo .git >expected &&\n \ttest_cmp expected actual &&\n \tmkdir sub &&\n \tgit -C sub rev-parse --git-common-dir >actual2 &&\n-\techo sub/.git >expected2 &&\n+\techo ../.git >expected2 &&\n \ttest_cmp expected2 actual2\n '\n \n+test_expect_failure 'rev-parse --git-path objects linked worktree' '\n+\techo \"$(git rev-parse --show-toplevel)/.git/worktrees/linked-tree/objects\" >expect &&\n+\ttest_when_finished \"rm -rf linked-tree && git worktree prune\" &&\n+\tgit worktree add --detach linked-tree master &&\n+\tgit -C linked-tree rev-parse --git-path objects >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '\"list\" all worktrees from main' '\n \techo \"$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n \ttest_when_finished \"rm -rf here && git worktree prune\" &&\n-- \n2.8.0\n"},{"id":"287567","messageId":"1464261556-89722-3-git-send-email-rappazzo@gmail.com","threadId":"42455","inReplyTo":"1464261556-89722-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v4 2/2] rev-parse: fix some options when executed from subpath of main tree","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-05-26T11:19:16Z","receivedAt":"2016-05-26T11:19:16Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,\nor `--shared-index-path` from the root of the main worktree results in\na relative path to the git dir.\n\nWhen executed from a subdirectory of the main tree, it can incorrectly\nreturn a path which starts 'sub/path/.git'.  Change this to return the\nproper relative path to the git directory.\n\nRelated tests marked to expect failure are updated to expect success\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n builtin/rev-parse.c      | 22 +++++++++++++++++-----\n t/t1500-rev-parse.sh     |  4 ++--\n t/t1700-split-index.sh   |  2 +-\n t/t2027-worktree-list.sh |  4 ++--\n 4 files changed, 22 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex c961b74..40579e0 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -564,10 +564,15 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (!strcmp(arg, \"--git-path\")) {\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tchar *path;\n \t\t\tif (!argv[i + 1])\n \t\t\t\tdie(\"--git-path requires an argument\");\n-\t\t\tputs(git_path(\"%s\", argv[i + 1]));\n-\t\t\ti++;\n+\n+\t\t\tpath = xstrfmt(\"%s/%s\", get_git_dir(), argv[++i]);\n+\t\t\tputs(relative_path(path, prefix, &sb));\n+\t\t\tstrbuf_release(&sb);\n+\t\t\tfree(path);\n \t\t\tcontinue;\n \t\t}\n \t\tif (as_is) {\n@@ -787,8 +792,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--git-common-dir\")) {\n-\t\t\t\tconst char *pfx = prefix ? prefix : \"\";\n-\t\t\t\tputs(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));\n+\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\tputs(relative_path(get_git_common_dir(), prefix, &sb));\n+\t\t\t\tstrbuf_release(&sb);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--is-inside-git-dir\")) {\n@@ -811,7 +817,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tdie(_(\"Could not read the index\"));\n \t\t\t\tif (the_index.split_index) {\n \t\t\t\t\tconst unsigned char *sha1 = the_index.split_index->base_sha1;\n-\t\t\t\t\tputs(git_path(\"sharedindex.%s\", sha1_to_hex(sha1)));\n+\t\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\t\tchar *path;\n+\n+\t\t\t\t\tpath = xstrfmt(\"%s/sharedindex.%s\", get_git_dir(), sha1_to_hex(sha1));\n+\t\t\t\t\tputs(relative_path(path, prefix, &sb));\n+\t\t\t\t\tstrbuf_release(&sb);\n+\t\t\t\t\tfree(path);\n \t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t}\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex f39f783..d74f09a 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -93,7 +93,7 @@ test_expect_success 'git-common-dir from worktree root' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'git-common-dir inside sub-dir' '\n+test_expect_success 'git-common-dir inside sub-dir' '\n \tmkdir -p path/to/child &&\n \ttest_when_finished \"rm -rf path\" &&\n \techo \"$(git -C path/to/child rev-parse --show-cdup).git\" >expect &&\n@@ -107,7 +107,7 @@ test_expect_success 'git-path from worktree root' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'git-path inside sub-dir' '\n+test_expect_success 'git-path inside sub-dir' '\n \tmkdir -p path/to/child &&\n \ttest_when_finished \"rm -rf path\" &&\n \techo \"$(git -C path/to/child rev-parse --show-cdup).git/objects\" >expect &&\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 8ca21bd..d2d9e02 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -200,7 +200,7 @@ EOF\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'rev-parse --shared-index-path' '\n+test_expect_success 'rev-parse --shared-index-path' '\n \trm -rf .git &&\n \ttest_create_repo . &&\n \tgit update-index --split-index &&\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 53cc5d3..16eec6e 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -8,7 +8,7 @@ test_expect_success 'setup' '\n \ttest_commit init\n '\n \n-test_expect_failure 'rev-parse --git-common-dir on main worktree' '\n+test_expect_success 'rev-parse --git-common-dir on main worktree' '\n \tgit rev-parse --git-common-dir >actual &&\n \techo .git >expected &&\n \ttest_cmp expected actual &&\n@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main worktree' '\n \ttest_cmp expected2 actual2\n '\n \n-test_expect_failure 'rev-parse --git-path objects linked worktree' '\n+test_expect_success 'rev-parse --git-path objects linked worktree' '\n \techo \"$(git rev-parse --show-toplevel)/.git/worktrees/linked-tree/objects\" >expect &&\n \ttest_when_finished \"rm -rf linked-tree && git worktree prune\" &&\n \tgit worktree add --detach linked-tree master &&\n-- \n2.8.0\n"},{"id":"287573","messageId":"20160526132611.GA2736@glandium.org","threadId":"42455","inReplyTo":"1464261556-89722-3-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v4 2/2] rev-parse: fix some options when executed from subpath of main tree","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2016-05-26T13:26:11Z","receivedAt":"2016-05-26T13:26:11Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, May 26, 2016 at 07:19:16AM -0400, Michael Rappazzo wrote:\n> Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,\n> or `--shared-index-path` from the root of the main worktree results in\n> a relative path to the git dir.\n> \n> When executed from a subdirectory of the main tree, it can incorrectly\n> return a path which starts 'sub/path/.git'.  Change this to return the\n> proper relative path to the git directory.\n> \n> Related tests marked to expect failure are updated to expect success\n\nAs mentioned previously (but late in the thread), I don't get why this\none case of --git-common-dir should not return the same thing as\n--git-dir, which is an absolute directory. Especially when there is\nother flag telling you whether you are in the main or another worktree,\nso comparing the output for --git-dir and --git-common-dir is the\neasiest way to do so, but then you have to normalize them on their own\nbecause git returns different values pointing to the same directory.\n\nMike\n"},{"id":"287706","messageId":"xmqqa8jbqqro.fsf@gitster.mtv.corp.google.com","threadId":"42455","inReplyTo":"20160526132611.GA2736@glandium.org","subject":"Re: [PATCH v4 2/2] rev-parse: fix some options when executed from subpath of main tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-27T18:50:35Z","receivedAt":"2016-05-27T18:50:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> On Thu, May 26, 2016 at 07:19:16AM -0400, Michael Rappazzo wrote:\n>> Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,\n>> or `--shared-index-path` from the root of the main worktree results in\n>> a relative path to the git dir.\n>> \n>> When executed from a subdirectory of the main tree, it can incorrectly\n>> return a path which starts 'sub/path/.git'.  Change this to return the\n>> proper relative path to the git directory.\n>> \n>> Related tests marked to expect failure are updated to expect success\n>\n> As mentioned previously (but late in the thread), I don't get why this\n> one case of --git-common-dir should not return the same thing as\n> --git-dir, which is an absolute directory. Especially when there is\n> other flag telling you whether you are in the main or another worktree,\n> so comparing the output for --git-dir and --git-common-dir is the\n> easiest way to do so, but then you have to normalize them on their own\n> because git returns different values pointing to the same directory.\n\nSounds like a sensible line of thought.\n\nA possible/plausible counter-argument from Michael's side that would\nbe equally sensible might run along the lines of:\n\n    An expected use of \"git rev-parse --commit-dir\" is to store the\n    output in $GIT_DIR/$X so that the layout the worktree machinery\n    expects can be set up by scripted Porcelains without using \"git\n    worktree\".  Making the value stored in $GIT_DIR/$X relative to\n    $Y would help for such and such reasons.\n\nWhile making it easier to build a competing UI like that is a\nsensible goal, I do not think of what that $X or $Y are, and I do\nnot think of what that \"such and such reasons\" are, either.\n\nAnd the cost of having to compare absolute --git-dir output with\nrelative --git-common-dir (i.e. the justification for Mike's\nproposal to make --git-common-dir absolute) and the cost of having\nto turn absolute output from --git-common-dir to a path relative to\n$Y (i.e. the justification of making it relative in the hypothetical\ncounter-argument) would be about the same, so it does not sound very\ncompelling after all.\n"}]}