{"thread":{"id":"41918","subject":"[PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","startedAt":"2016-04-04T01:42:23Z","lastAt":"2016-05-19T14:15:49Z","messageCount":7,"participants":["Michael Rappazzo","Eric Sunshine","Duy Nguyen","Mike Rappazzo","Mike Hommey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"282592","messageId":"1459734143-95832-1-git-send-email-rappazzo@gmail.com","threadId":"41918","inReplyTo":null,"subject":"[PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-04-04T01:42:23Z","receivedAt":"2016-04-04T01:42:23Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Executing `git-rev-parse --git-common-dir` from the root of the main\nworktree results in '.git', which is the relative path to the git dir.\nWhen executed from a subpath of the main tree it returned somthing like:\n'sub/path/.git'.  Change this to return the proper relative path to the\ngit directory (similar to `--show-cdup`).\n\nAdd as test to t1500-rev-parse.sh for this case and adjust another test\nin t2027-worktree-list.sh to use this expectation.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n builtin/rev-parse.c      | 14 ++++++++++++--\n t/t1500-rev-parse.sh     | 10 ++++++++++\n t/t2027-worktree-list.sh |  2 +-\n 3 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex c961b74..c2918e1 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -787,8 +787,18 @@ 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\tconst char *git_common_dir = get_git_common_dir();\n+\t\t\t\tif (prefix && !is_absolute_path(git_common_dir)) {\n+\t\t\t\t\tconst char *pfx = prefix;\n+\t\t\t\t\twhile (pfx) {\n+\t\t\t\t\t\tpfx = strchr(pfx, '/');\n+\t\t\t\t\t\tif (pfx) {\n+\t\t\t\t\t\t\tpfx++;\n+\t\t\t\t\t\t\tprintf(\"../\");\n+\t\t\t\t\t\t}\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tprintf(\"%s\\n\", git_common_dir);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--is-inside-git-dir\")) {\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 48ee077..2023208 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -3,6 +3,16 @@\n test_description='test git rev-parse'\n . ./test-lib.sh\n \n+test_expect_success 'git-common-dir inside sub-dir' '\n+   (\n+\t\tmkdir -p path/to/child &&\n+\t\tcd path/to/child &&\n+\t\techo \"$(git rev-parse --show-cdup).git\" >expect &&\n+\t\tgit rev-parse --git-common-dir >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_rev_parse() {\n \tname=$1\n \tshift\ndiff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh\nindex 1b1b65a..3780b14 100755\n--- a/t/t2027-worktree-list.sh\n+++ b/t/t2027-worktree-list.sh\n@@ -14,7 +14,7 @@ test_expect_success 'rev-parse --git-common-dir on main worktree' '\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-- \n2.8.0\n"},{"id":"282601","messageId":"CAPig+cSDMD_MjRVh534YfWeY0E1VykWW08oML9oe7QVuxRC72g@mail.gmail.com","threadId":"41918","inReplyTo":"1459734143-95832-1-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-04T06:01:04Z","receivedAt":"2016-04-04T06:01:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 3, 2016 at 9:42 PM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n> Executing `git-rev-parse --git-common-dir` from the root of the main\n> worktree results in '.git', which is the relative path to the git dir.\n> When executed from a subpath of the main tree it returned somthing like:\n\ns/returned/returns/\ns/somthing/something/\n\nIt would be clearer if you stated explicitly that the subpath result\nis incorrect. For instance:\n\n    When executed from a subdirectory of the main tree,\n    however, it incorrectly returns:\n\nMore below...\n\n> 'sub/path/.git'.  Change this to return the proper relative path to the\n> git directory (similar to `--show-cdup`).\n>\n> Add as test to t1500-rev-parse.sh for this case and adjust another test\n> in t2027-worktree-list.sh to use this expectation.\n>\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> @@ -787,8 +787,18 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n>                         if (!strcmp(arg, \"--git-common-dir\")) {\n> -                               const char *pfx = prefix ? prefix : \"\";\n> -                               puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));\n> +                               const char *git_common_dir = get_git_common_dir();\n> +                               if (prefix && !is_absolute_path(git_common_dir)) {\n> +                                       const char *pfx = prefix;\n> +                                       while (pfx) {\n> +                                               pfx = strchr(pfx, '/');\n> +                                               if (pfx) {\n> +                                                       pfx++;\n> +                                                       printf(\"../\");\n> +                                               }\n> +                                       }\n> +                               }\n> +                               printf(\"%s\\n\", git_common_dir);\n\nHow about simplifying this entire chunk of code to?\n\n    struct strbuf sb = STRBUF_INIT;\n    puts(relative_path(get_git_common_dir(), prefix, &sb));\n    strbuf_release(&sb);\n\nNo need to check NULL prefix or absolute common dir.\n\n>                                 continue;\n>                         }\n> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> @@ -3,6 +3,16 @@\n>  test_description='test git rev-parse'\n>  . ./test-lib.sh\n>\n> +test_expect_success 'git-common-dir inside sub-dir' '\n> +   (\n\nStrange indentation. Use TAB rather than spaces.\n\n> +               mkdir -p path/to/child &&\n\nNit: Although functionally the same, typically 'mkdir' is outside the subshell.\n\n> +               cd path/to/child &&\n> +               echo \"$(git rev-parse --show-cdup).git\" >expect &&\n> +               git rev-parse --git-common-dir >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n\nI guess you added this new test to the top of the script rather than\nthe bottom (as is more customary) because this script is ancient and\ncd's all around the place with wild abandon and leaks environment\nvariables; thus you avoided having to prefix this new test with:\n\n    cd .. || exit 1\n    sane_unset GIT_DIR GIT_CONFIG\n\nwhich would have been needed had it been added to at the bottom of the script.\n\nIt probably wouldn't hurt to add tests to also verify correct behavior\nat the root of the main tree, as well as at the root and within a\nsubdirectory of a linked worktree (though the latter two tests would\nprobably go in a worktree-related test script).\n\nThanks.\n"},{"id":"282952","messageId":"CACsJy8CgNy8+s0j+UUVVDk1ru702mtkTrO6QsYNpisGY-eAYGw@mail.gmail.com","threadId":"41918","inReplyTo":"1459734143-95832-1-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-08T11:47:12Z","receivedAt":"2016-04-08T11:47:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n> Executing `git-rev-parse --git-common-dir` from the root of the main\n> worktree results in '.git', which is the relative path to the git dir.\n> When executed from a subpath of the main tree it returned somthing like:\n> 'sub/path/.git'.  Change this to return the proper relative path to the\n> git directory (similar to `--show-cdup`).\n\nI faced a similar problem just a couple days ago, I expected \"git\nrev-parse --git-path\" to return a path relative to cwd too, but it\nreturned relative to git dir. The same solution (or Eric's, which is\ncleaner in my opinion) applies. --shared-index-path also does\nputs(git_path(... and has the same problem. Do you want to fix them\ntoo?\n-- \nDuy\n"},{"id":"282953","messageId":"CANoM8SWCiJch-nXq=-=ELKOc6tV-TNg6U-hcc9W69Fz4rz+j+g@mail.gmail.com","threadId":"41918","inReplyTo":"CACsJy8CgNy8+s0j+UUVVDk1ru702mtkTrO6QsYNpisGY-eAYGw@mail.gmail.com","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-04-08T12:35:51Z","receivedAt":"2016-04-08T12:35:51Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n>> Executing `git-rev-parse --git-common-dir` from the root of the main\n>> worktree results in '.git', which is the relative path to the git dir.\n>> When executed from a subpath of the main tree it returned somthing like:\n>> 'sub/path/.git'.  Change this to return the proper relative path to the\n>> git directory (similar to `--show-cdup`).\n>\n> I faced a similar problem just a couple days ago, I expected \"git\n> rev-parse --git-path\" to return a path relative to cwd too, but it\n> returned relative to git dir. The same solution (or Eric's, which is\n> cleaner in my opinion) applies. --shared-index-path also does\n> puts(git_path(... and has the same problem. Do you want to fix them\n> too?\n\nSure, I can do that.  I will try to get it up soon.\n"},{"id":"286964","messageId":"20160519074939.GA22796@glandium.org","threadId":"41918","inReplyTo":"CANoM8SWCiJch-nXq=-=ELKOc6tV-TNg6U-hcc9W69Fz4rz+j+g@mail.gmail.com","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2016-05-19T07:49:39Z","receivedAt":"2016-05-19T07:49:39Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Fri, Apr 08, 2016 at 08:35:51AM -0400, Mike Rappazzo wrote:\n> On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> > On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n> >> Executing `git-rev-parse --git-common-dir` from the root of the main\n> >> worktree results in '.git', which is the relative path to the git dir.\n> >> When executed from a subpath of the main tree it returned somthing like:\n> >> 'sub/path/.git'.  Change this to return the proper relative path to the\n> >> git directory (similar to `--show-cdup`).\n> >\n> > I faced a similar problem just a couple days ago, I expected \"git\n> > rev-parse --git-path\" to return a path relative to cwd too, but it\n> > returned relative to git dir. The same solution (or Eric's, which is\n> > cleaner in my opinion) applies. --shared-index-path also does\n> > puts(git_path(... and has the same problem. Do you want to fix them\n> > too?\n> \n> Sure, I can do that.  I will try to get it up soon.\n\nIf I'm not mistaken, it looks like this fell off your radar. (I haven't\nseen an updated patch, and it doesn't look like the fix made it to any\ngit branch). Would you mind updating?\n\nCheers,\n\nMike\n"},{"id":"286967","messageId":"20160519085049.GA26914@glandium.org","threadId":"41918","inReplyTo":"1459734143-95832-1-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2016-05-19T08:50:49Z","receivedAt":"2016-05-19T08:50:49Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Apr 03, 2016 at 09:42:23PM -0400, Michael Rappazzo wrote:\n> Executing `git-rev-parse --git-common-dir` from the root of the main\n> worktree results in '.git', which is the relative path to the git dir.\n> When executed from a subpath of the main tree it returned somthing like:\n> 'sub/path/.git'.  Change this to return the proper relative path to the\n> git directory (similar to `--show-cdup`).\n\nNote that `git rev-parse --git-dir` returns an absolute path, not a\nrelative one. Shouldn't --git-common-dir \"simply\" return the same as\n--git-dir when not in a worktree?\n\nMike\n"},{"id":"286984","messageId":"CANoM8SUDVtGa12i2LXor6-bEwgH100zztYFR5mmQbs4z-5K4tg@mail.gmail.com","threadId":"41918","inReplyTo":"20160519074939.GA22796@glandium.org","subject":"Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-05-19T14:15:49Z","receivedAt":"2016-05-19T14:15:49Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Thu, May 19, 2016 at 3:49 AM, Mike Hommey <mh@glandium.org> wrote:\n> On Fri, Apr 08, 2016 at 08:35:51AM -0400, Mike Rappazzo wrote:\n>> On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n>> > On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n>> >> Executing `git-rev-parse --git-common-dir` from the root of the main\n>> >> worktree results in '.git', which is the relative path to the git dir.\n>> >> When executed from a subpath of the main tree it returned somthing like:\n>> >> 'sub/path/.git'.  Change this to return the proper relative path to the\n>> >> git directory (similar to `--show-cdup`).\n>> >\n>> > I faced a similar problem just a couple days ago, I expected \"git\n>> > rev-parse --git-path\" to return a path relative to cwd too, but it\n>> > returned relative to git dir. The same solution (or Eric's, which is\n>> > cleaner in my opinion) applies. --shared-index-path also does\n>> > puts(git_path(... and has the same problem. Do you want to fix them\n>> > too?\n>>\n>> Sure, I can do that.  I will try to get it up soon.\n>\n> If I'm not mistaken, it looks like this fell off your radar. (I haven't\n> seen an updated patch, and it doesn't look like the fix made it to any\n> git branch). Would you mind updating?\n>\n> Cheers,\n>\n> Mike\n\nThere is a newer version submitted on May 6th[1].  Eric Sunshine has\nsubmitted a patch [2] which fixes\nup t1500.  It looks like that is in a stable form, so I will rebase my\nv2 onto those changes, and resubmit\nin the near future.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/293778\n[2] http://thread.gmane.org/gmane.comp.version-control.git/294999\n"}]}