Add "git show --follow-symlinks HEAD:symlink"

RFC PATCH In review Michael Vogt 4 participants 12 messages Apr 9, 2018 – Apr 16, 2018 Documentation

lore

Share & tools

lore

Michael Vogt

author

2018-04-09 09:00 UTC #0 lore raw

RFC PATCH 1/1 Add "git show --follow-symlinks HEAD:symlink"

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.

From 616b7f21c057656960cb6b8a266095bbef734122 Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
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(-)

builtin/log.c +5 -2

index 94ee177d5..e92af4fc7 100644
@@ -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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

Signature · 1 line

2.14.1

Ævar Arnfjörð Bjarmason

2018-04-09 09:28 UTC #1 lore raw

Quoted 23 lines · Michael Vogt

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 <[email protected]>
Date: Mon, 9 Apr 2018 10:38:13 +0200
Subject: [PATCH] support: git show --follow-symlinks HEAD:symlink

Quoted 4 lines

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.

Michael Vogt

author

2018-04-13 09:43 UTC #2 lore raw

Hi Ævar,

thanks for your quick reply!

Quoted 1 line · Ævar Arnfjörð Bjarmason

On Mon, Apr 09 2018, Michael Vogt wrote:

[..]

Quoted 11 lines

Quoted 6 lines
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

From 5a9faa9eff00f316fc654c8e3bc85c3ba56ea659 Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
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 <[email protected]>

---
 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

Documentation/git-show.txt +6 -0

index e73ef5401..fa751c35d 100644
@@ -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[]

builtin/log.c +5 -2

index 94ee177d5..e92af4fc7 100644
@@ -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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

t/t1800-git-show.sh new file +41 -0

41 lines not on this page · raw message

Signature · 1 line

2.17.0

Stefan Beller

2018-04-13 17:28 UTC #3 lore raw

Hi Michael,

thanks for the patch,

Quoted 4 lines

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

Michael Vogt

author

2018-04-13 17:48 UTC #4 lore raw

Quoted 1 line · Stefan Beller

Hi Michael,

Hi Stefan,

Quoted 1 line

thanks for the patch,

Thanks for the review.

[..]

Quoted 3 lines

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.

Quoted 2 lines

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

[..]

Makes sense, updated.

Quoted 1 line

There is a typo &e&.

Ups, sorry! Fixed.

Quoted 2 lines

Can we reword the documentation, such that we do not have an occurrence of "extended SHA-1" ?

[..]

Quoted 4 lines

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

From dab10f5e5aea8a31cbee0ab1d5a78204c8c9832a Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
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 <[email protected]>

---
 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

Documentation/git-show.txt +6 -0

index e73ef5401..d9f7fd90c 100644
@@ -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[]

builtin/log.c +5 -2

index 94ee177d5..e92af4fc7 100644
@@ -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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

t/t1800-git-show.sh new file +46 -0

46 lines not on this page · raw message

Signature · 1 line

2.17.0

Ævar Arnfjörð Bjarmason

2018-04-13 19:33 UTC #5 lore raw

Quoted 1 line · Michael Vogt

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.

Michael Vogt

author

2018-04-13 19:50 UTC #6 lore raw

PATCH 1/1 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 <[email protected]>

---
 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

Documentation/git-show.txt +6 -0

index e73ef5401..d9f7fd90c 100644
@@ -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[]

builtin/log.c +5 -2

index 94ee177d5..e92af4fc7 100644
@@ -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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

t/t1800-git-show.sh new file +46 -0

46 lines not on this page · raw message

Signature · 1 line

2.17.0

Ævar Arnfjörð Bjarmason

2018-04-13 21:03 UTC #7 lore raw

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

Quoted 6 lines · 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.

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.

Quoted 15 lines

Signed-off-by Michael Vogt <[email protected]>

---
 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

Documentation/git-show.txt +0 -0

index e73ef5401..d9f7fd90c 100644

Documentation/git-show.txt OPTIONS · line 48

Quoted 10 lines

Documentation/git-show.txt +6 -0

@@ -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.

Quoted 23 lines

builtin/log.c +3 -1

index 94ee177d5..e92af4fc7 100644
@@ -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()
 	};

builtin/log.c static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, · line 176

Quoted 4 lines

builtin/log.c +0 -1

@@ -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.

Quoted 74 lines

builtin/log.c +2 -0

 	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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

t/t1800-git-show.sh new file +36 -0

new file mode 100755
index 000000000..85541b4db
@@ -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 &&

t/t1800-git-show.sh line 46

Quoted 10 lines

t/t1800-git-show.sh new file +10 -0

+    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(-)

builtin/log.c +1 -0

index 2990f9b1b9..5d920a893a 100644
@@ -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)

t/t1800-git-show.sh +25 ~~-30~~

67 lines not on this page · raw message

Michael Vogt

author

2018-04-16 09:39 UTC #8 lore raw

PATCH v2 1/1 show: add --follow-symlinks option for <rev>:<path>

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.

Michael Vogt

author

2018-04-16 09:41 UTC #9 lore raw

PATCH 1/1 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>.

Signed-off-by Michael Vogt <[email protected]>

---
 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

Documentation/git-show.txt +7 -0

index e73ef5401..e2634b27e 100644
@@ -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[]

builtin/log.c +5 -1

index 94ee177d5..7d815b8ea 100644
@@ -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 */

revision.c +2 -0

index b42c836d7..4ab22313f 100644
@@ -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;

revision.h +1 -0

index b8c47b98e..060f1038a 100644
@@ -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;

t/t1800-git-show.sh new file +41 -0

41 lines not on this page · raw message

Signature · 1 line

2.17.0

Junio C Hamano

2018-04-16 23:05 UTC #10 lore raw

Re: [PATCH] show: add --follow-symlinks option for <rev>:<path>

Quoted 2 lines · Michael Vogt

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.

Michael Vogt

author

2018-04-13 19:55 UTC #11 lore raw

In reply to Ævar Arnfjörð Bjarmason

Quoted 7 lines · Ævar Arnfjörð Bjarmason

Quoted 1 line · Michael Vogt
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

[..]

Quoted 4 lines

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