git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
@ 2018-04-09  9:00 Michael Vogt
  2018-04-09  9:28 ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-09  9:00 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

Hi,

I noticed that `git show HEAD:path-to-symlink` does not work and
returns an error like:
"fatal: Path 'debian/changelog' exists on disk, but not in 'HEAD'."

Looking at `git show` it seems there is no way right now to make
git-show show blobs if they are symlinks [1].

It would be nice to have this ability. Attached is a draft patch to
allow to write: `git show --follow-symlinks HEAD:path-to-symlink`.
Tests are missing in the patch, I'm happy to add those if there is a
chance for the feature to get in.

Cheers,
 Michael

[1] Using `git cat-file --follow-symlinks --batch < input` works but
    feels a bit less elegant compared to supporting it directly in
    git-show.

[-- Attachment #2: 0001-support-git-show-follow-symlinks-HEAD-symlink.patch --]
[-- Type: text/x-diff, Size: 2830 bytes --]

From 616b7f21c057656960cb6b8a266095bbef734122 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Mon, 9 Apr 2018 10:38:13 +0200
Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink

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.
---
 builtin/log.c | 7 +++++--
 revision.c    | 2 ++
 revision.h    | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

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);
-
 	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;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-09  9:00 [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt
@ 2018-04-09  9:28 ` Ævar Arnfjörð Bjarmason
  2018-04-13  9:43   ` Michael Vogt
  0 siblings, 1 reply; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-04-09  9:28 UTC (permalink / raw)
  To: Michael Vogt; +Cc: git


On Mon, Apr 09 2018, Michael Vogt wrote:

> I noticed that `git show HEAD:path-to-symlink` does not work and
> returns an error like:
> "fatal: Path 'debian/changelog' exists on disk, but not in 'HEAD'."
>
> Looking at `git show` it seems there is no way right now to make
> git-show show blobs if they are symlinks [1].
>
> It would be nice to have this ability. Attached is a draft patch to
> allow to write: `git show --follow-symlinks HEAD:path-to-symlink`.
> Tests are missing in the patch, I'm happy to add those if there is a
> chance for the feature to get in.
>
> Cheers,
>  Michael
>
> [1] Using `git cat-file --follow-symlinks --batch < input` works but
>     feels a bit less elegant compared to supporting it directly in
>     git-show.
> From 616b7f21c057656960cb6b8a266095bbef734122 Mon Sep 17 00:00:00 2001
> From: Michael Vogt <mvo@ubuntu.com>
> Date: Mon, 9 Apr 2018 10:38:13 +0200
> Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink
>
> Add support for the `--follow-symlinks` options to git-show. This
> allows to write:
>
>     git show --follow-symlink HEAD:path-a-symlink

The patch looks reasonable, but please submit it as described in
Documentation/SubmittingPatches, i.e. inline instead of as an
attachment, and with a signed-off-by line etc. We'd also need some tests
for this.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-09  9:28 ` Ævar Arnfjörð Bjarmason
@ 2018-04-13  9:43   ` Michael Vogt
  2018-04-13 17:28     ` Stefan Beller
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-13  9:43 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

Hi Ævar,

thanks for your quick reply!

On Mon, Apr 09, 2018 at 11:28:45AM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Apr 09 2018, Michael Vogt wrote:
[..]
> > Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink
> >
> > Add support for the `--follow-symlinks` options to git-show. This
> > allows to write:
> >
> >     git show --follow-symlink HEAD:path-a-symlink
> 
> The patch looks reasonable, but please submit it as described in
> Documentation/SubmittingPatches, i.e. inline instead of as an
> attachment, and with a signed-off-by line etc. We'd also need some tests
> for this.

Thanks for the intial reivew. I updated the patch with a test and
documentation for the new option. Happy to merge the test into one of
the existing test files, I read t/README and greping around I did not
find a place that looked like a good fit. 

I added the updated patch as an mutt inline attachment now.

Cheers,
 Michael

[-- Attachment #2: 0001-support-git-show-follow-symlinks-HEAD-symlink.patch --]
[-- Type: text/x-diff, Size: 5076 bytes --]

From 5a9faa9eff00f316fc654c8e3bc85c3ba56ea659 Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Mon, 9 Apr 2018 10:38:13 +0200
Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink

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.

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        | 41 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 55 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..fa751c35d 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
+	with extended SHA-1 expressions 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[]
 
 
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);
-
 	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..86fe8ee02
--- /dev/null
+++ b/t/t1800-git-show.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test git show works'
+
+. ./test-lib.sh
+
+test_expect_success 'verify git show HEAD:foo works' '
+    echo "foo content" > foo &&
+    git add foo &&
+    git commit -m "added foo" &&
+    content=$(git show HEAD:foo) &&
+    test "$content" = "foo content"
+'
+
+test_expect_success 'verify git show HEAD:symlink shows symlink points to foo' '
+    echo "foo content" > foo &&
+    ln -s foo symlink &&
+    git add foo symlink &&
+    git commit -m "added foo and a symlink to foo" &&
+    content=$(git show HEAD:foo) &&
+    test "$content" = "foo content" &&
+    symlink=$(git show HEAD:symlink) &e& 
+    test "$symlink" = "foo"
+'
+
+test_expect_success 'verify git show --follow-symlinks HEAD:symlink shows foo' '
+    content=$(git show --follow-symlinks HEAD:symlink) &&
+    test "$content" = "foo content"
+'
+
+test_expect_success 'verify git show --follow-symlinks HEAD:symlink works with subdirs' '
+    mkdir dir &&
+    ln -s dir symlink-to-dir &&
+    echo "bar content" > dir/bar &&
+    git add dir symlink-to-dir &&
+    git commit -m "add dir and symlink-to-dir" &&
+    content=$(git show --follow-symlinks HEAD:symlink-to-dir/bar) &&
+    test "$content" = "bar content"
+'
+
+test_done
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-13  9:43   ` Michael Vogt
@ 2018-04-13 17:28     ` Stefan Beller
  2018-04-13 17:48       ` Michael Vogt
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2018-04-13 17:28 UTC (permalink / raw)
  To: Michael Vogt; +Cc: Ævar Arnfjörð Bjarmason, git

Hi Michael,

thanks for the patch,

> Thanks for the intial reivew. I updated the patch with a test and
> documentation for the new option. Happy to merge the test into one of
> the existing test files, I read t/README and greping around I did not
> find a place that looked like a good fit.

I think keeping tests as separate as possible is a good idea.
Looking at the patch https://public-inbox.org/git/20180413094314.GA2404@bod/

The patch seems reasonable, apart from minor nits:
In the test we'd prefer no whitespace on the right side of the redirection,
i.e. echo content >foo

Instead of evaluating git commands in shell and assigning it to a variable,
we'd prefer to dump it to files:

  git show HEAD:symlink >actual &&
  echo foo >expect &&
  test_cmp expect actual

(instead of content=$(git show HEAD:foo) && test $content == ...)

The reason for this is that the &&-chain will inspect the return code
of the git command.

There is a typo &e&.

Can we reword the documentation, such that we do not have
an occurrence of "extended SHA-1" ?
(By now the Git community came up with a plan to migrate
away from SHA-1, hence we'd not want to introduce more
dependencies even in the form of documentation for that)

Maybe

Follow symlinks inside the repository when requesting
objects in extended revision syntax of the form tree-ish:path-in-tree.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-13 17:28     ` Stefan Beller
@ 2018-04-13 17:48       ` Michael Vogt
  2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-13 17:48 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, avarab

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Fri, Apr 13, 2018 at 10:28:13AM -0700, Stefan Beller wrote:
> Hi Michael,
Hi Stefan,

> thanks for the patch,
Thanks for the review.

[..]
> The patch seems reasonable, apart from minor nits:
> In the test we'd prefer no whitespace on the right side of the redirection,
> i.e. echo content >foo

Sure, updated.

> Instead of evaluating git commands in shell and assigning it to a variable,
> we'd prefer to dump it to files:
[..]

Makes sense, updated.

> There is a typo &e&.

Ups, sorry! Fixed.

> Can we reword the documentation, such that we do not have
> an occurrence of "extended SHA-1" ?
[..]
> Maybe
> 
> Follow symlinks inside the repository when requesting
> objects in extended revision syntax of the form tree-ish:path-in-tree.

This looks very reasonable, I updated the documentation accordingly.

The update patch is attached as an inline attachement.

Cheers,
 Michael

[-- Attachment #2: 0001-support-git-show-follow-symlinks-HEAD-symlink.patch --]
[-- Type: text/x-diff, Size: 5230 bytes --]

From dab10f5e5aea8a31cbee0ab1d5a78204c8c9832a Mon Sep 17 00:00:00 2001
From: Michael Vogt <mvo@ubuntu.com>
Date: Mon, 9 Apr 2018 10:38:13 +0200
Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink

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.

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[]
 
 
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);
-
 	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
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-13 17:48       ` Michael Vogt
@ 2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
  2018-04-13 19:48           ` [PATCH] support: git show --follow-symlinks HEAD:symlink Michael Vogt
  2018-04-13 19:55           ` [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt
  0 siblings, 2 replies; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-04-13 19:33 UTC (permalink / raw)
  To: Michael Vogt; +Cc: Stefan Beller, git


On Fri, Apr 13 2018, Michael Vogt wrote:

> The update patch is attached as an inline attachement.

Your patch still just shows up as a straight-up attachment in many
E-Mail clients. Note the difference between what your patch
(https://public-inbox.org/git/20180413174819.GA19030@bod/raw) and a
patch that's not an attachment
(https://public-inbox.org/git/0f0942043678fe76f8d654306482ee26fac643f0.1523617836.git.johannes.schindelin@gmx.de/raw)
look like.

Try to "wget" both of those and apply them with "git am" on top of
master, and note how what you're doing results in a broken patch.

This is why Documentation/SubmittingPatches suggests using format-patch
& send-email. You don't *have* to use those tools, and can use something
that's compatible with what's expected on-list, but what you're doing
isn't that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] support: git show --follow-symlinks HEAD:symlink
  2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
@ 2018-04-13 19:48           ` Michael Vogt
  2018-04-13 21:03             ` Ævar Arnfjörð Bjarmason
  2018-04-13 19:55           ` [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-13 19:48 UTC (permalink / raw)
  To: avarab, sbeller, git; +Cc: Michael Vogt

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.

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[]
 
 
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);
-
 	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
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"
  2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
  2018-04-13 19:48           ` [PATCH] support: git show --follow-symlinks HEAD:symlink Michael Vogt
@ 2018-04-13 19:55           ` Michael Vogt
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Vogt @ 2018-04-13 19:55 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Stefan Beller, git

On Fri, Apr 13, 2018 at 09:33:00PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Apr 13 2018, Michael Vogt wrote:
> 
> > The update patch is attached as an inline attachement.
> 
> Your patch still just shows up as a straight-up attachment in many
> E-Mail clients. Note the difference between what your patch
[..]
> This is why Documentation/SubmittingPatches suggests using format-patch
> & send-email. You don't *have* to use those tools, and can use something
> that's compatible with what's expected on-list, but what you're doing
> isn't that.

My apologizes, I resend it using "git send-email".

Cheers,
 Michael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] support: git show --follow-symlinks HEAD:symlink
  2018-04-13 19:48           ` [PATCH] support: git show --follow-symlinks HEAD:symlink Michael Vogt
@ 2018-04-13 21:03             ` Ævar Arnfjörð Bjarmason
  2018-04-16  9:36               ` [PATCH v2] show: add --follow-symlinks option for <rev>:<path> Michael Vogt
  0 siblings, 1 reply; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-04-13 21:03 UTC (permalink / raw)
  To: Michael Vogt; +Cc: sbeller, git


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2] show: add --follow-symlinks option for <rev>:<path>
  2018-04-13 21:03             ` Ævar Arnfjörð Bjarmason
@ 2018-04-16  9:36               ` Michael Vogt
  2018-04-16  9:36                 ` [PATCH] " Michael Vogt
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-16  9:36 UTC (permalink / raw)
  To: git, sbeller, avarab

Updated version of the `git show --follow-symlink` patch. This version
includes the feedback from Ævar Arnfjörð Bjarmason and Stefan Beller:

- commit message updated
- test fixes merged from Ævar (thanks!)
- Documentation/git-show.txt clarified

It does not include a test for --follow-symlinks in the dirlink case
when a file is behind a dirlink symlink. This this currently fails with
a non-descriptive error message. I hope to find time to improve this
error message at some point and then a test for this will be added.

It also does not include a test for a repo with symlinks when running
on a system without SYMLINKS. I don't have access to such a system,
sorry.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] show: add --follow-symlinks option for <rev>:<path>
  2018-04-16  9:36               ` [PATCH v2] show: add --follow-symlinks option for <rev>:<path> Michael Vogt
@ 2018-04-16  9:36                 ` Michael Vogt
  2018-04-16 23:05                   ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Vogt @ 2018-04-16  9:36 UTC (permalink / raw)
  To: git, sbeller, avarab; +Cc: Michael Vogt

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.

Signed-off-by: Michael Vogt <mvo@ubuntu.com>
---
 Documentation/git-show.txt |  7 +++++++
 builtin/log.c              |  6 +++++-
 revision.c                 |  2 ++
 revision.h                 |  1 +
 t/t1800-git-show.sh        | 41 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100755 t/t1800-git-show.sh

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index e73ef5401..e2634b27e 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -39,6 +39,13 @@ 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.
+	It will resolve any symlinks in <path-in-tree> and shows the
+	content	of the link if the symlink is the leaf node of
+	<path-in-tree>.
+
 include::pretty-options.txt[]
 
 
diff --git a/builtin/log.c b/builtin/log.c
index 94ee177d5..7d815b8ea 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()
 	};
 
@@ -176,6 +178,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 
 	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..7a02438ec
--- /dev/null
+++ b/t/t1800-git-show.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test git show works'
+
+. ./test-lib.sh
+
+test_expect_success 'verify git show HEAD:foo works' '
+	test_commit A &&
+	git show HEAD:A.t >actual &&
+	echo A >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 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 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
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] show: add --follow-symlinks option for <rev>:<path>
  2018-04-16  9:36                 ` [PATCH] " Michael Vogt
@ 2018-04-16 23:05                   ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2018-04-16 23:05 UTC (permalink / raw)
  To: Michael Vogt; +Cc: git, sbeller, avarab

Michael Vogt <mvo@ubuntu.com> writes:

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

This not only affects "show" but all in the "log" family of
commands, because the change is made to revision.[ch] that is shared
by them.  I doubt that is desirable.

I offhand do not think of any command other than "show", to which
this feature makes any sense [*1*].  And I certainly do not mind if
the feature is limited to "show" and nothing else for that reason.

But I do mind the implementation that seeps through to other
commands (because "log_init_finish()" is shared with commands in the
family other than "show", and because "struct rev_info" is shared
across all the commands in the "log" famil) and not limited to
"show", which is a sign of typical end-user confusion waiting to
happen.

Thanks.


[Footnote]

For example, "git log" is affected by this patch but I am not sure
what it even means that we can ask this question:

	$ git log -p --follow-symlinks -- RelNotes

Can we see how each update to Documentation/RelNotes/2.17.0.txt as
well as change of RelNotes symlink from 2.17.0.txt to 2.18.0.txt in
the patch form?  If that were the case, it might make some sense to
allow the feature to be triggered by "git log" like your change to
builtin/log.c did, but I somehow do not think that is what your
patch does.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-04-16 23:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-09  9:00 [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt
2018-04-09  9:28 ` Ævar Arnfjörð Bjarmason
2018-04-13  9:43   ` Michael Vogt
2018-04-13 17:28     ` Stefan Beller
2018-04-13 17:48       ` Michael Vogt
2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
2018-04-13 19:48           ` [PATCH] support: git show --follow-symlinks HEAD:symlink Michael Vogt
2018-04-13 21:03             ` Ævar Arnfjörð Bjarmason
2018-04-16  9:36               ` [PATCH v2] show: add --follow-symlinks option for <rev>:<path> Michael Vogt
2018-04-16  9:36                 ` [PATCH] " Michael Vogt
2018-04-16 23:05                   ` Junio C Hamano
2018-04-13 19:55           ` [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).