[PATCH v4 0/2] rev-parse: fix some options when executed from subpath of main tree

[Michael Rappazzo]

Changes since v3 [1]:
 - Rebased onto 'pu' which includes the cleanup of t1500 by Eric Sunshine
 - Fixed a memory leak due to misusing xstrfmt()

[1] http://thread.gmane.org/gmane.comp.version-control.git/293778

Michael Rappazzo (2):
  rev-parse tests: add tests executed from a subdirectory
  rev-parse: fix some options when executed from subpath of main tree

 builtin/rev-parse.c      | 22 +++++++++++++++++-----
 t/t1500-rev-parse.sh     | 28 ++++++++++++++++++++++++++++
 t/t1700-split-index.sh   | 17 +++++++++++++++++
 t/t2027-worktree-list.sh | 10 +++++++++-
 4 files changed, 71 insertions(+), 6 deletions(-)

--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Michael Rappazzo]

t2027-worktree-list has an incorrect expectation for --git-common-dir
which has been adjusted and marked to expect failure.

Some of the tests added have been marked to expect failure.  These
demonstrate a problem with the way that some options to git rev-parse
behave when executed from a subdirectory of the main worktree.

Signed-off-by: Michael Rappazzo <[hidden email]>
---
 t/t1500-rev-parse.sh     | 28 ++++++++++++++++++++++++++++
 t/t1700-split-index.sh   | 17 +++++++++++++++++
 t/t2027-worktree-list.sh | 12 ++++++++++--
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c..f39f783 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -87,4 +87,32 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru
 
 test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
 
+test_expect_success 'git-common-dir from worktree root' '
+ echo .git >expect &&
+ git rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-common-dir inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+ git -C path/to/child rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+ echo .git/objects >expect &&
+ git rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-path inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
+ git -C path/to/child rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
 test_done
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8aef49f..8ca21bd 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,21 @@ EOF
  test_cmp expect actual
 '
 
+test_expect_failure 'rev-parse --shared-index-path' '
+ rm -rf .git &&
+ test_create_repo . &&
+ git update-index --split-index &&
+ ls -t .git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual &&
+ mkdir work &&
+ test_when_finished "rm -rf work" &&
+ (
+ cd work &&
+ ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual
+ )
+'
+
 test_done
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..53cc5d3 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,16 +8,24 @@ test_expect_success 'setup' '
  test_commit init
 '
 
-test_expect_success 'rev-parse --git-common-dir on main worktree' '
+test_expect_failure 'rev-parse --git-common-dir on main worktree' '
  git rev-parse --git-common-dir >actual &&
  echo .git >expected &&
  test_cmp expected actual &&
  mkdir sub &&
  git -C sub rev-parse --git-common-dir >actual2 &&
- echo sub/.git >expected2 &&
+ echo ../.git >expected2 &&
  test_cmp expected2 actual2
 '
 
+test_expect_failure 'rev-parse --git-path objects linked worktree' '
+ echo "$(git rev-parse --show-toplevel)/.git/worktrees/linked-tree/objects" >expect &&
+ test_when_finished "rm -rf linked-tree && git worktree prune" &&
+ git worktree add --detach linked-tree master &&
+ git -C linked-tree rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
 test_expect_success '"list" all worktrees from main' '
  echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
  test_when_finished "rm -rf here && git worktree prune" &&
--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Michael Rappazzo]

Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,
or `--shared-index-path` from the root of the main worktree results in
a relative path to the git dir.

When executed from a subdirectory of the main tree, it can incorrectly
return a path which starts 'sub/path/.git'.  Change this to return the
proper relative path to the git directory.

Related tests marked to expect failure are updated to expect success

Signed-off-by: Michael Rappazzo <[hidden email]>
---
 builtin/rev-parse.c      | 22 +++++++++++++++++-----
 t/t1500-rev-parse.sh     |  4 ++--
 t/t1700-split-index.sh   |  2 +-
 t/t2027-worktree-list.sh |  4 ++--
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c961b74..40579e0 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -564,10 +564,15 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  }
 
  if (!strcmp(arg, "--git-path")) {
+ struct strbuf sb = STRBUF_INIT;
+ char *path;
  if (!argv[i + 1])
  die("--git-path requires an argument");
- puts(git_path("%s", argv[i + 1]));
- i++;
+
+ path = xstrfmt("%s/%s", get_git_dir(), argv[++i]);
+ puts(relative_path(path, prefix, &sb));
+ strbuf_release(&sb);
+ free(path);
  continue;
  }
  if (as_is) {
@@ -787,8 +792,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  continue;
  }
  if (!strcmp(arg, "--git-common-dir")) {
- const char *pfx = prefix ? prefix : "";
- puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
+ struct strbuf sb = STRBUF_INIT;
+ puts(relative_path(get_git_common_dir(), prefix, &sb));
+ strbuf_release(&sb);
  continue;
  }
  if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -811,7 +817,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  die(_("Could not read the index"));
  if (the_index.split_index) {
  const unsigned char *sha1 = the_index.split_index->base_sha1;
- puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));
+ struct strbuf sb = STRBUF_INIT;
+ char *path;
+
+ path = xstrfmt("%s/sharedindex.%s", get_git_dir(), sha1_to_hex(sha1));
+ puts(relative_path(path, prefix, &sb));
+ strbuf_release(&sb);
+ free(path);
  }
  continue;
  }
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index f39f783..d74f09a 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -93,7 +93,7 @@ test_expect_success 'git-common-dir from worktree root' '
  test_cmp expect actual
 '
 
-test_expect_failure 'git-common-dir inside sub-dir' '
+test_expect_success 'git-common-dir inside sub-dir' '
  mkdir -p path/to/child &&
  test_when_finished "rm -rf path" &&
  echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
@@ -107,7 +107,7 @@ test_expect_success 'git-path from worktree root' '
  test_cmp expect actual
 '
 
-test_expect_failure 'git-path inside sub-dir' '
+test_expect_success 'git-path inside sub-dir' '
  mkdir -p path/to/child &&
  test_when_finished "rm -rf path" &&
  echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8ca21bd..d2d9e02 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,7 +200,7 @@ EOF
  test_cmp expect actual
 '
 
-test_expect_failure 'rev-parse --shared-index-path' '
+test_expect_success 'rev-parse --shared-index-path' '
  rm -rf .git &&
  test_create_repo . &&
  git update-index --split-index &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 53cc5d3..16eec6e 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,7 +8,7 @@ test_expect_success 'setup' '
  test_commit init
 '
 
-test_expect_failure 'rev-parse --git-common-dir on main worktree' '
+test_expect_success 'rev-parse --git-common-dir on main worktree' '
  git rev-parse --git-common-dir >actual &&
  echo .git >expected &&
  test_cmp expected actual &&
@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main worktree' '
  test_cmp expected2 actual2
 '
 
-test_expect_failure 'rev-parse --git-path objects linked worktree' '
+test_expect_success 'rev-parse --git-path objects linked worktree' '
  echo "$(git rev-parse --show-toplevel)/.git/worktrees/linked-tree/objects" >expect &&
  test_when_finished "rm -rf linked-tree && git worktree prune" &&
  git worktree add --detach linked-tree master &&
--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Mike Hommey-3]

On Thu, May 26, 2016 at 07:19:16AM -0400, Michael Rappazzo wrote:
> Executing `git-rev-parse` with `--git-common-dir`, `--git-path <path>`,
> or `--shared-index-path` from the root of the main worktree results in
> a relative path to the git dir.
>
> When executed from a subdirectory of the main tree, it can incorrectly
> return a path which starts 'sub/path/.git'.  Change this to return the
> proper relative path to the git directory.
>
> Related tests marked to expect failure are updated to expect success

As mentioned previously (but late in the thread), I don't get why this
one case of --git-common-dir should not return the same thing as
--git-dir, which is an absolute directory. Especially when there is
other flag telling you whether you are in the main or another worktree,
so comparing the output for --git-dir and --git-common-dir is the
easiest way to do so, but then you have to normalize them on their own
because git returns different values pointing to the same directory.

Mike
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html