On Sun, Apr 3, 2016 at 9:42 PM, Michael Rappazzo <rappazzo@xxxxxxxxx> wrote:
> Executing `git-rev-parse --git-common-dir` from the root of the main
> worktree results in '.git', which is the relative path to the git dir.
> When executed from a subpath of the main tree it returned somthing like:
s/returned/returns/
s/somthing/something/
It would be clearer if you stated explicitly that the subpath result
is incorrect. For instance:
When executed from a subdirectory of the main tree,
however, it incorrectly returns:
More below...
> 'sub/path/.git'. Change this to return the proper relative path to the
> git directory (similar to `--show-cdup`).
>
> Add as test to t1500-rev-parse.sh for this case and adjust another test
> in t2027-worktree-list.sh to use this expectation.
>
> Signed-off-by: Michael Rappazzo <rappazzo@xxxxxxxxx>
> ---
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> @@ -787,8 +787,18 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> if (!strcmp(arg, "--git-common-dir")) {
> - const char *pfx = prefix ? prefix : "";
> - puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
> + const char *git_common_dir = get_git_common_dir();
> + if (prefix && !is_absolute_path(git_common_dir)) {
> + const char *pfx = prefix;
> + while (pfx) {
> + pfx = strchr(pfx, '/');
> + if (pfx) {
> + pfx++;
> + printf("../");
> + }
> + }
> + }
> + printf("%s\n", git_common_dir);
How about simplifying this entire chunk of code to?
struct strbuf sb = STRBUF_INIT;
puts(relative_path(get_git_common_dir(), prefix, &sb));
strbuf_release(&sb);
No need to check NULL prefix or absolute common dir.
> continue;
> }
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> @@ -3,6 +3,16 @@
> test_description='test git rev-parse'
> . ./test-lib.sh
>
> +test_expect_success 'git-common-dir inside sub-dir' '
> + (
Strange indentation. Use TAB rather than spaces.
> + mkdir -p path/to/child &&
Nit: Although functionally the same, typically 'mkdir' is outside the subshell.
> + cd path/to/child &&
> + echo "$(git rev-parse --show-cdup).git" >expect &&
> + git rev-parse --git-common-dir >actual &&
> + test_cmp expect actual
> + )
> +'
I guess you added this new test to the top of the script rather than
the bottom (as is more customary) because this script is ancient and
cd's all around the place with wild abandon and leaks environment
variables; thus you avoided having to prefix this new test with:
cd .. || exit 1
sane_unset GIT_DIR GIT_CONFIG
which would have been needed had it been added to at the bottom of the script.
It probably wouldn't hurt to add tests to also verify correct behavior
at the root of the main tree, as well as at the root and within a
subdirectory of a linked worktree (though the latter two tests would
probably go in a worktree-related test script).
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html