Re: [PATCH] support: git show --follow-symlinks HEAD:symlink

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Apr 13 2018, Michael Vogt wrote:

> Add support for the `--follow-symlinks` options to git-show. This
> allows to write:
>
>     git show --follow-symlink HEAD:path-a-symlink
>
> to get the content of the symlinked file.

Thanks. Commit message would be better as something like:

    show: add --follow-symlinks option for <rev>:<path>

    Add a --follow-symlinks option that'll resolve symlinks to their
    targets when the target is of the form <rev>:<path>.

    Without it, git will show the path of the link itself if the symlink
    is the leaf node of <path>, or otherwise an error if some component
    of <path> is a symlink to another location in the repository. With
    the new --follow-symlinks option both will be resolved to their
    target, and its content shown instead.

I.e. start with "<command>: " ("show" in this case), and explain how it
impacts the dirlink case.

> Signed-off-by: Michael Vogt <mvo@ubuntu.com>
> ---
>  Documentation/git-show.txt |  6 +++++
>  builtin/log.c              |  7 ++++--
>  revision.c                 |  2 ++
>  revision.h                 |  1 +
>  t/t1800-git-show.sh        | 46 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 60 insertions(+), 2 deletions(-)
>  create mode 100755 t/t1800-git-show.sh
>
> diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
> index e73ef5401..d9f7fd90c 100644
> --- a/Documentation/git-show.txt
> +++ b/Documentation/git-show.txt
> @@ -39,6 +39,12 @@ OPTIONS
>  	For a more complete list of ways to spell object names, see
>  	"SPECIFYING REVISIONS" section in linkgit:gitrevisions[7].
>
> +--follow-symlinks::
> +	Follow symlinks inside the repository when requesting objects
> +	in extended revision syntax of the form tree-ish:path-in-tree.
> +	Instead of output about the link itself, provide output about
> +	the linked-to object.
> +
>  include::pretty-options.txt[]

This needs to document the dirlink case I noted above.

> diff --git a/builtin/log.c b/builtin/log.c
> index 94ee177d5..e92af4fc7 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -142,7 +142,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  			 struct rev_info *rev, struct setup_revision_opt *opt)
>  {
>  	struct userformat_want w;
> -	int quiet = 0, source = 0, mailmap = 0;
> +	int quiet = 0, source = 0, mailmap = 0, follow_symlinks = 0;
>  	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
>  	static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
>  	static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
> @@ -162,6 +162,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  		OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
>  			     N_("Process line range n,m in file, counting from 1"),
>  			     log_line_range_callback),
> +		OPT_BOOL(0, "follow-symlinks", &follow_symlinks,
> +			 N_("follow in-tree symlinks (used when showing file content)")),
>  		OPT_END()
>  	};
>
> @@ -173,9 +175,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  			     builtin_log_options, builtin_log_usage,
>  			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
>  			     PARSE_OPT_KEEP_DASHDASH);
> -

Stray line deletion here.

>  	if (quiet)
>  		rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> +	if (follow_symlinks)
> +		rev->follow_symlinks = 1;
>  	argc = setup_revisions(argc, argv, rev, opt);
>
>  	/* Any arguments at this point are not recognized */
> diff --git a/revision.c b/revision.c
> index b42c836d7..4ab22313f 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1678,6 +1678,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
>
>  	if (revarg_opt & REVARG_COMMITTISH)
>  		get_sha1_flags |= GET_OID_COMMITTISH;
> +	if (revs && revs->follow_symlinks)
> +		get_sha1_flags |= GET_OID_FOLLOW_SYMLINKS;
>
>  	if (get_oid_with_context(arg, get_sha1_flags, &oid, &oc))
>  		return revs->ignore_missing ? 0 : -1;
> diff --git a/revision.h b/revision.h
> index b8c47b98e..060f1038a 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -122,6 +122,7 @@ struct rev_info {
>  			first_parent_only:1,
>  			line_level_traverse:1,
>  			tree_blobs_in_commit_order:1,
> +			follow_symlinks:1,
>
>  			/* for internal use only */
>  			exclude_promisor_objects:1;
> diff --git a/t/t1800-git-show.sh b/t/t1800-git-show.sh
> new file mode 100755
> index 000000000..85541b4db
> --- /dev/null
> +++ b/t/t1800-git-show.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh
> +
> +test_description='Test git show works'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'verify git show HEAD:foo works' '
> +    printf "foo content\n" >foo &&
> +    git add foo &&
> +    git commit -m "added foo" &&
> +    git show HEAD:foo >actual &&
> +    printf "foo content\n" >expected &&
> +    test_cmp expected actual
> +'
> +
> +test_expect_success 'verify git show HEAD:symlink shows symlink points to foo' '
> +    printf "foo content\n" >foo &&
> +    ln -s foo symlink &&
> +    git add foo symlink &&
> +    git commit -m "added foo and a symlink to foo" &&
> +    git show HEAD:foo >actual &&
> +    printf "foo content\n" >expected &&
> +    test_cmp expected actual &&
> +    git show HEAD:symlink >actual &&
> +    printf "foo" >expected &&
> +    test_cmp expected actual
> +'
> +
> +test_expect_success 'verify git show --follow-symlinks HEAD:symlink shows foo' '
> +    git show --follow-symlinks HEAD:symlink >actual &&
> +    printf "foo content\n" >expected &&
> +    test_cmp expected actual
> +'
> +
> +test_expect_success 'verify git show --follow-symlinks HEAD:symlink works with subdirs' '
> +    mkdir dir &&
> +    ln -s dir symlink-to-dir &&
> +    printf "bar content\n" >dir/bar &&
> +    git add dir symlink-to-dir &&
> +    git commit -m "add dir and symlink-to-dir" &&
> +    git show --follow-symlinks HEAD:symlink-to-dir/bar >actual &&
> +    printf "bar content\n" >expected &&
> +    test_cmp expected actual
> +'
> +
> +test_done

There's a few issues with this test code I noticed:

 1) Doesn't indent with tabs
 2) Doesn't use the test_commit helper, less verbose
 3) Your 2nd test is just repeating the setup of the 1st test, which is
    not needed
 4) You don't test without --follow-symlinks in the dirlink case
 5) You don't guard with the SYMLINKS prereq (this test will fail on Windows et al)
 6) printf "foo\n" instead of just "echo foo"

Below is a local fixup for this that I came up with (also for that line
deletion above), should be squash-able into a v2.

It would also be worth testing for the case where we don't have SYMLINKS
but *do* clone a repo that has symlinks, but I have no idea what git
does in that case, just copy the file?

 builtin/log.c       |  1 +
 t/t1800-git-show.sh | 55 ++++++++++++++++++++++++-----------------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 2990f9b1b9..5d920a893a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -175,6 +175,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			     builtin_log_options, builtin_log_usage,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
+
 	if (quiet)
 		rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
 	if (follow_symlinks)
diff --git a/t/t1800-git-show.sh b/t/t1800-git-show.sh
index 85541b4db3..7a02438ecf 100755
--- a/t/t1800-git-show.sh
+++ b/t/t1800-git-show.sh
@@ -5,42 +5,37 @@ test_description='Test git show works'
 . ./test-lib.sh

 test_expect_success 'verify git show HEAD:foo works' '
-    printf "foo content\n" >foo &&
-    git add foo &&
-    git commit -m "added foo" &&
-    git show HEAD:foo >actual &&
-    printf "foo content\n" >expected &&
-    test_cmp expected actual
+	test_commit A &&
+	git show HEAD:A.t >actual &&
+	echo A >expected &&
+	test_cmp expected actual
 '

-test_expect_success 'verify git show HEAD:symlink shows symlink points to foo' '
-    printf "foo content\n" >foo &&
-    ln -s foo symlink &&
-    git add foo symlink &&
-    git commit -m "added foo and a symlink to foo" &&
-    git show HEAD:foo >actual &&
-    printf "foo content\n" >expected &&
-    test_cmp expected actual &&
-    git show HEAD:symlink >actual &&
-    printf "foo" >expected &&
-    test_cmp expected actual
+test_expect_success SYMLINKS 'verify git show HEAD:symlink shows symlink points to foo' '
+	ln -s A.t A.link &&
+	git add A.link &&
+	git commit -m"Added symlink to A.t" &&
+	git show HEAD:A.link >actual &&
+	printf "%s" A.t >expected &&
+	test_cmp expected actual
 '

-test_expect_success 'verify git show --follow-symlinks HEAD:symlink shows foo' '
-    git show --follow-symlinks HEAD:symlink >actual &&
-    printf "foo content\n" >expected &&
-    test_cmp expected actual
+test_expect_success SYMLINKS 'verify git show --follow-symlinks HEAD:symlink shows foo' '
+	git show --follow-symlinks HEAD:A.link >actual &&
+	echo A >expected &&
+	test_cmp expected actual
 '

-test_expect_success 'verify git show --follow-symlinks HEAD:symlink works with subdirs' '
-    mkdir dir &&
-    ln -s dir symlink-to-dir &&
-    printf "bar content\n" >dir/bar &&
-    git add dir symlink-to-dir &&
-    git commit -m "add dir and symlink-to-dir" &&
-    git show --follow-symlinks HEAD:symlink-to-dir/bar >actual &&
-    printf "bar content\n" >expected &&
-    test_cmp expected actual
+test_expect_success SYMLINKS 'verify git show --follow-symlinks HEAD:symlink works with subdirs' '
+	mkdir dir &&
+	ln -s dir symlink-to-dir &&
+	test_commit dir/B &&
+	git add dir symlink-to-dir &&
+	git commit -m "add dir and symlink-to-dir" &&
+	test_must_fail git show HEAD:symlink-to-dir/B.t >actual &&
+	git show --follow-symlinks HEAD:symlink-to-dir/B.t >actual &&
+	echo dir/B >expected &&
+	test_cmp expected actual
 '

 test_done