@matheusbsbs / matheus-bagliano / commits / 098aa86762

rev-parse: fix several options when running in a subdirectory

In addition to making git_path() aware of certain file names that need to be handled differently e.g. when running in worktrees, the commit 557bd833bb (git_path(): be aware of file relocation in $GIT_DIR, 2014-11-30) also snuck in a new option for `git rev-parse`: `--git-path`. On the face of it, there is no obvious bug in that commit's diff: it faithfully calls git_path() on the argument and prints it out, i.e. `git rev-parse --git-path <filename>` has the same precise behavior as calling `git_path("<filename>")` in C. The problem lies deeper, much deeper. In hindsight (which is always unfair), implementing the .git/ directory discovery in `setup_git_directory()` by changing the working directory may have allowed us to avoid passing around a struct that contains information about the current repository, but it bought us many, many problems. In this case, when being called in a subdirectory, `git rev-parse` changes the working directory to the top-level directory before calling `git_path()`. In the new working directory, the result is correct. But in the working directory of the calling script, it is incorrect. Example: when calling `git rev-parse --git-path HEAD` in, say, the Documentation/ subdirectory of Git's own source code, the string `.git/HEAD` is printed. Side note: that bug is hidden when running in a subdirectory of a worktree that was added by the `git worktree` command: in that case, the (correct) absolute path of the `HEAD` file is printed. In the interest of time, this patch does not go the "correct" route to introduce a struct with repository information (and removing global state in the process), instead this patch chooses to detect when the command was called in a subdirectory and forces the result to be an absolute path. While at it, we are also fixing the output of --git-common-dir and --shared-index-path. Lastly, please note that we reuse the same strbuf for all of the relative_path() calls; this avoids frequent allocation (and duplicated code), and it does not risk memory leaks, for two reasons: 1) the cmd_rev_parse() function does not return anywhere between the use of the new strbuf instance and its final release, and 2) git-rev-parse is one of these "one-shot" programs in Git, i.e. it exits after running for a very short time, meaning that all allocated memory is released with the exit() call anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Feb 17, 2017 at 17:59 UTC 098aa867626ef2444ef14a92b428a6ca26d83e60

4 files changed +16 -9

builtin/rev-parse.c

+11 -4

@@ -545,6 +545,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

545 unsigned int flags = 0;

546 const char *name = NULL;

547 struct object_context unused;

548 + struct strbuf buf = STRBUF_INIT;

549

550 if (argc > 1 && !strcmp("--parseopt", argv[1]))

551 return cmd_parseopt(argc - 1, argv + 1, prefix);

@@ -599,7 +600,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

600 if (!strcmp(arg, "--git-path")) {

601 if (!argv[i + 1])

602 die("--git-path requires an argument");

602 - puts(git_path("%s", argv[i + 1]));

603 + strbuf_reset(&buf);

604 + puts(relative_path(git_path("%s", argv[i + 1]),

605 + prefix, &buf));

606 i++;

607 continue;

608 }

@@ -821,8 +824,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

824 continue;

825 }

826 if (!strcmp(arg, "--git-common-dir")) {

824 - const char *pfx = prefix ? prefix : "";

825 - puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));

827 + strbuf_reset(&buf);

828 + puts(relative_path(get_git_common_dir(),

829 + prefix, &buf));

830 continue;

831 }

832 if (!strcmp(arg, "--is-inside-git-dir")) {

@@ -845,7 +849,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

849 die(_("Could not read the index"));

850 if (the_index.split_index) {

851 const unsigned char *sha1 = the_index.split_index->base_sha1;

848 - puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));

852 + const char *path = git_path("sharedindex.%s", sha1_to_hex(sha1));

853 + strbuf_reset(&buf);

854 + puts(relative_path(path, prefix, &buf));

855 }

856 continue;

857 }

@@ -897,6 +903,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

903 continue;

904 verify_filename(prefix, arg, 1);

905 }

906 + strbuf_release(&buf);

907 if (verify) {

908 if (revs_count == 1) {

909 show_rev(type, sha1, name);

t/t1500-rev-parse.sh

+2 -2

@@ -93,7 +93,7 @@ test_expect_success 'git-common-dir from worktree root' '

93 test_cmp expect actual

94 '

96 -test_expect_failure 'git-common-dir inside sub-dir' '

96 +test_expect_success 'git-common-dir inside sub-dir' '

97 mkdir -p path/to/child &&

98 test_when_finished "rm -rf path" &&

99 echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&

@@ -107,7 +107,7 @@ test_expect_success 'git-path from worktree root' '

107 test_cmp expect actual

108 '

109

110 -test_expect_failure 'git-path inside sub-dir' '

110 +test_expect_success 'git-path inside sub-dir' '

111 mkdir -p path/to/child &&

112 test_when_finished "rm -rf path" &&

113 echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&

t/t1700-split-index.sh

+1 -1

@@ -200,7 +200,7 @@ EOF

200 test_cmp expect actual

201 '

202

203 -test_expect_failure 'rev-parse --shared-index-path' '

203 +test_expect_success 'rev-parse --shared-index-path' '

204 test_create_repo split-index &&

205 (

206 cd split-index &&

t/t2027-worktree-list.sh

+2 -2

@@ -8,7 +8,7 @@ test_expect_success 'setup' '

8 test_commit init

9 '

11 -test_expect_failure 'rev-parse --git-common-dir on main worktree' '

11 +test_expect_success 'rev-parse --git-common-dir on main worktree' '

12 git rev-parse --git-common-dir >actual &&

13 echo .git >expected &&

14 test_cmp expected actual &&

@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main worktree' '

18 test_cmp expected2 actual2

19 '

21 -test_expect_failure 'rev-parse --git-path objects linked worktree' '

21 +test_expect_success 'rev-parse --git-path objects linked worktree' '

22 echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&

23 test_when_finished "rm -rf linked-tree && git worktree prune" &&

24 git worktree add --detach linked-tree master &&