[PATCH 1/3] tree-walk: learn get_tree_enty_follow_symlinks

[PATCH 1/3] tree-walk: learn get_tree_enty_follow_symlinks

[dturner]

From: David Turner <dturner@twitter.com>

Add a new function, get_tree_entry_follow_symlinks, to tree-walk.[ch].
The function is not yet used.  It will be used to implement git
cat-file --batch --follow-symlinks.

The function locates an object by path, following symlinks in the
repository.  If the symlinks lead outside the repository, the function
reports this to the caller.

Signed-off-by: David Turner <dturner@twitter.com>
---
 tree-walk.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tree-walk.h |   2 +
 2 files changed, 224 insertions(+)

diff --git a/tree-walk.c b/tree-walk.c
index 5dd9a71..6fb4b7d 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -415,6 +415,228 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 	return error;
 }
 
+static int find_tree_entry_nonrecursive(struct tree_desc *t, char *name, unsigned char *result, unsigned *mode) {
+	int namelen = strlen(name);
+
+	while (t->size) {
+		const char *entry;
+		const unsigned char *sha1;
+		int entrylen, cmp;
+
+		sha1 = tree_entry_extract(t, &entry, mode);
+		entrylen = tree_entry_len(&t->entry);
+		update_tree_entry(t);
+		if (entrylen > namelen)
+			continue;
+		cmp = memcmp(name, entry, entrylen);
+		if (cmp > 0)
+			continue;
+		if (cmp < 0)
+			break;
+		if (entrylen == namelen) {
+			hashcpy(result, sha1);
+			return 0;
+		}
+		if (name[entrylen] != '/')
+			continue;
+		if (!S_ISDIR(*mode))
+			break;
+		hashcpy(result, sha1);
+		return 0;
+	}
+	return -1;
+}
+
+struct dir_state {
+	void *tree;
+	unsigned long size;
+	unsigned char sha1[20];
+};
+
+#define GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS 40
+
+/**
+ * Find a tree entry by following symlinks in tree_sha (which is
+ * assumed to be the root of the repository).  In the event that a
+ * symlink points outside the repository (e.g. a link to /foo or a
+ * root-level link to ../foo), the portion of the link which is
+ * outside the repository will be copied into result_path (which is
+ * assumed to hold at least PATH_MAX bytes), and *mode will be set to
+ * 0.  Otherwise, result will be filled in with the sha1 of the found
+ * object, and *mode will hold the mode of the object.
+ */
+int get_tree_enty_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, unsigned char *result_path, unsigned *mode)
+{
+	int retval = -1;
+	void *tree;
+	struct dir_state *parents = NULL;
+	size_t parents_cap = 0;
+	ssize_t parents_len = 0;
+	unsigned long size;
+	unsigned char root[20];
+	unsigned char current_tree_sha1[20];
+	struct strbuf namebuf = STRBUF_INIT;
+	enum object_type type;
+	int already_have_tree = 0;
+	struct tree_desc t = {0};
+	int follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;
+	int i;
+
+	strbuf_addstr(&namebuf, name);
+	hashcpy(current_tree_sha1, tree_sha1);
+
+	while (1) {
+		char *first_slash;
+		char *remainder = NULL;
+		int find_result;
+
+		if (!t.buffer) {
+			tree = read_object_with_reference(current_tree_sha1,
+							  tree_type, &size,
+							  root);
+			if (!tree)
+				goto done;
+
+			ALLOC_GROW(parents, parents_len + 1, parents_cap);
+			parents[parents_len].tree = tree;
+			parents[parents_len].size = size;
+			hashcpy(parents[parents_len].sha1, root);
+
+			parents_len++;
+
+			if (namebuf.buf[0] == '\0') {
+				hashcpy(result, root);
+				retval = 0;
+				goto done;
+			}
+
+			if (!size)
+				goto done;
+
+			/* descend */
+			init_tree_desc(&t, tree, size);
+		}
+
+		/* Handle symlinks to e.g. a//b by removing leading slashes */
+		while (namebuf.buf[0] == '/') {
+			strbuf_remove(&namebuf, 0, 1);
+		}
+
+		/* Split namebuf into a first component and a
+		 * remainder */
+		if ((first_slash = strchr(namebuf.buf, '/'))) {
+			*first_slash = 0;
+			remainder = first_slash + 1;
+		}
+
+		if (!strcmp(namebuf.buf, "..")) {
+			struct dir_state *parent;
+			/* We could end up with .. in the namebuf if
+			 * it appears in a symlink. */
+
+			if (parents_len == 1) {
+				if (remainder)
+					*first_slash = '/';
+				if (strlcpy(result_path, namebuf.buf,
+					    PATH_MAX) < PATH_MAX) {
+					*mode = 0;
+					retval = 0;
+				}
+				goto done;
+			}
+			parent = &parents[parents_len - 1];
+			free(parent->tree);
+			parents_len--;
+			parent = &parents[parents_len - 1];
+			init_tree_desc(&t, parent->tree, parent->size);
+			strbuf_remove(&namebuf, 0, remainder ? 3 : 2);
+			continue;
+		}
+
+		/* We could end up here via a symlink to dir/.. */
+		if (namebuf.buf[0] == '\0') {
+			hashcpy(result, parents[parents_len - 1].sha1);
+			retval = 0;
+			goto done;
+		}
+
+		/* Look up the first (or only) path component
+		 * in the tree. */
+		find_result = find_tree_entry_nonrecursive(&t, namebuf.buf,
+							   current_tree_sha1,
+							   mode);
+		if (find_result) {
+			retval = find_result;
+			goto done;
+		}
+
+		if (S_ISDIR(*mode)) {
+			if (!remainder) {
+				hashcpy(result, current_tree_sha1);
+				retval = 0;
+				goto done;
+			}
+			/* Descend the tree */
+			t.buffer = NULL;
+			strbuf_remove(&namebuf, 0,
+				      1 + first_slash - namebuf.buf);
+		} else if (S_ISREG(*mode)) {
+			if (!remainder) {
+				hashcpy(result, current_tree_sha1);
+				retval = 0;
+			}
+			goto done;
+		} else if (S_ISLNK(*mode)) {
+			/* Follow a symlink */
+			size_t link_len, len;
+			char *contents, *contents_start;
+			struct dir_state *parent;
+
+			if (follows_remaining-- == 0)
+				/* Too many symlinks followed */
+				goto done;
+
+			contents = read_sha1_file(current_tree_sha1, &type,
+						  &link_len);
+
+			if (!contents)
+				goto done;
+
+			if (contents[0] == '/') {
+				if (strlcpy(result_path,
+					    contents, PATH_MAX) < PATH_MAX) {
+					*mode = 0;
+					retval = 0;
+				}
+				goto done;
+			}
+
+			if (remainder)
+				len = first_slash - namebuf.buf;
+			else
+				len = namebuf.len;
+
+			contents_start = contents;
+
+			parent = &parents[parents_len - 1];
+			init_tree_desc(&t, parent->tree, parent->size);
+			strbuf_splice(&namebuf, 0, len,
+				      contents_start, link_len);
+			if (remainder)
+				namebuf.buf[link_len] = '/';
+			free(contents);
+		}
+	}
+done:
+	for (i = 0; i < parents_len; ++i) {
+		free(parents[i].tree);
+	}
+	free(parents);
+
+	strbuf_release(&namebuf);
+	return retval;
+}
+
 static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char *result, unsigned *mode)
 {
 	int namelen = strlen(name);
diff --git a/tree-walk.h b/tree-walk.h
index ae7fb3a..002e5a9 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -40,6 +40,8 @@ struct traverse_info;
 typedef int (*traverse_callback_t)(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *);
 int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info);
 
+int get_tree_enty_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, unsigned char *result_path, unsigned *mode);
+
 struct traverse_info {
 	struct traverse_info *prev;
 	struct name_entry name;
-- 
2.0.4.315.gad8727a-twtrsrc

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[dturner]

From: David Turner <dturner@twitter.com>

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis that
ONLY_TO_DIE triggers does not consider symlinks.

Signed-off-by: David Turner <dturner@twitter.com>
---
 cache.h     | 15 ++++++++-------
 sha1_name.c |  8 ++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..16743de 100644
--- a/cache.h
+++ b/cache.h
@@ -924,13 +924,14 @@ struct object_context {
 	unsigned mode;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..23863f7 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,6 +1434,12 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_enty_follow_symlinks(tree_sha1,
+					filename, sha1, oc->path, &oc->mode);
+				free(new_filename);
+				return ret;
+			}
 			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 			if (ret && only_to_die) {
 				diagnose_invalid_sha1_path(prefix, filename,
@@ -1469,5 +1475,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die(_("internal error: bad flags for get_sha1_with_context"));
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Eric Sunshine]

On Fri, May 8, 2015 at 2:13 PM,  <dturner@twopensource.com> wrote:
> Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
> when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
> is incompatible with G_S_ONLY_TO_DIE because the diagnosis that
> ONLY_TO_DIE triggers does not consider symlinks.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---
> diff --git a/sha1_name.c b/sha1_name.c
> index 6d10f05..23863f7 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1434,6 +1434,12 @@ static int get_sha1_with_context_1(const char *name,
>                         new_filename = resolve_relative_path(filename);
>                         if (new_filename)
>                                 filename = new_filename;
> +                       if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
> +                               ret = get_tree_enty_follow_symlinks(tree_sha1,
> +                                       filename, sha1, oc->path, &oc->mode);
> +                               free(new_filename);
> +                               return ret;
> +                       }
>                         ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
>                         if (ret && only_to_die) {
>                                 diagnose_invalid_sha1_path(prefix, filename,
> @@ -1469,5 +1475,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
>
>  int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
>  {
> +       if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
> +               die(_("internal error: bad flags for get_sha1_with_context"));

There may not be much value in marking an "internal error" string for
translation.

>         return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
>  }

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Junio C Hamano]

On Fri, May 8, 2015 at 12:45 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> +       if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
>> +               die(_("internal error: bad flags for get_sha1_with_context"));
>
> There may not be much value in marking an "internal error" string for
> translation.

The whole point of this kind of messages is so that the end users,
when they see it trigger, can notify us and then to allow us to
identify which die() triggered.  It is not just "may not be much
value in", but is actively unproductive to make it translatable,
even if we ignore the cost to i18n/l10n teams to translate such
messages.

By the way, we would want to standardise the string before the
colon, so that we can tell users "If you see an error message that
begins with 'internal error:', please report that to us---it is a
programming error".  I think the majority of existing code uses the
string "BUG:" for that, and I do not mind using 'internal error:'
for that purpose instead, but the important thing is to use just one
single string thoughout the codebase, so that the "please report
when you see this" message we give the users can be simple.

Thanks.

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Junio C Hamano]

Perhaps something like the attached patch is a good idea (I used
"BUG:" for now), I wonder?

 Documentation/CodingGuidelines | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index c6e536f..020370e 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -232,7 +232,12 @@ For C programs:
 	   to be translated, that follows immediately after it */
 	_("Here is a translatable string explained by the above.");
 
+ - Speaking of translations, do not mark the message to die() that
+   should never trigger unless there is an programming error.  E.g.
+
+	die("BUG: frotz() called with NULL pointer to xyzzy parameter");
+
- - Double negation is often harder to understand than no negation
+ - Double negation is not easier to understand than no negation
    at all.
 
  - There are two schools of thought when it comes to comparison,

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Jeff King]

On Fri, May 08, 2015 at 01:25:45PM -0700, Junio C Hamano wrote:

> Perhaps something like the attached patch is a good idea (I used
> "BUG:" for now), I wonder?
> 
>  Documentation/CodingGuidelines | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Yeah, this looks good (and we definitely prefer BUG:, though I am
responsible for over 1/3 of the existing incidences).

I have been tempted to provide a macro or function for it, and have it
actually call abort() to trigger a coredump. I.e., basically assert(),
except unconditional but you get to write a more useful message.

So all of the sites would just become:

  BUG("frotz() called with NULL pointer to xyzzy parameter");

or similar.

-Peff

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, May 08, 2015 at 01:25:45PM -0700, Junio C Hamano wrote:
>
>> Perhaps something like the attached patch is a good idea (I used
>> "BUG:" for now), I wonder?
>> 
>>  Documentation/CodingGuidelines | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> Yeah, this looks good (and we definitely prefer BUG:, though I am
> responsible for over 1/3 of the existing incidences).
>
> I have been tempted to provide a macro or function for it, and have it
> actually call abort() to trigger a coredump. I.e., basically assert(),
> except unconditional but you get to write a more useful message.
>
> So all of the sites would just become:
>
>   BUG("frotz() called with NULL pointer to xyzzy parameter");
>
> or similar.

Yeah, that sounds more sensible (especially the abort() part).

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

On Fri, 2015-05-08 at 13:17 -0700, Junio C Hamano wrote:
> On Fri, May 8, 2015 at 12:45 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> >> +       if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
> >> +               die(_("internal error: bad flags for get_sha1_with_context"));
> >
> > There may not be much value in marking an "internal error" string for
> > translation.
> 
> The whole point of this kind of messages is so that the end users,
> when they see it trigger, can notify us and then to allow us to
> identify which die() triggered.  It is not just "may not be much
> value in", but is actively unproductive to make it translatable,
> even if we ignore the cost to i18n/l10n teams to translate such
> messages.
>
> By the way, we would want to standardise the string before the
> colon, so that we can tell users "If you see an error message that
> begins with 'internal error:', please report that to us---it is a
> programming error".  I think the majority of existing code uses the
> string "BUG:" for that, and I do not mind using 'internal error:'
> for that purpose instead, but the important thing is to use just one
> single string thoughout the codebase, so that the "please report
> when you see this" message we give the users can be simple.

It appears that "BUG:" is preferred to "internal error:" by a margin of
2:1.  So I'll change it.  Fixing the remaining ~40 instances of
"internal error" would be a great project for someone new to git
development.

>From an i18n perspective, I think the correct thing to do would be to
i18n the "BUG" portion and nothing else.  That way, users can understand
what has happened, but if they report it upstream, git developers know
where in the code to look.  But this would probably be complicated to
implement, so maybe not worthwhile.

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> On Fri, May 8, 2015 at 12:45 PM, Eric Sunshine 
> <sunshine@sunshineco.com> wrote:
>>> +       if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & 
>>> GET_SHA1_ONLY_TO_DIE)
>>> +               die(_("internal error: bad flags for 
>>> get_sha1_with_context"));
>>
>> There may not be much value in marking an "internal error" string for
>> translation.
>
> The whole point of this kind of messages is so that the end users,
> when they see it trigger, can notify us and then to allow us to
> identify which die() triggered.  It is not just "may not be much
> value in", but is actively unproductive to make it translatable,
> even if we ignore the cost to i18n/l10n teams to translate such
> messages.
>
> By the way, we would want to standardise the string before the
> colon, so that we can tell users "If you see an error message that
> begins with 'internal error:', please report that to us---it is a
> programming error".  I think the majority of existing code uses the
> string "BUG:" for that, and I do not mind using 'internal error:'
> for that purpose instead, but the important thing is to use just one
> single string thoughout the codebase, so that the "please report
> when you see this" message we give the users can be simple.
>
I would see "BUG:" as being internationally recognised, while "internal 
error:" would need to be translated, and perhaps be written "internal 
error, report this to git@vger.kernel.org:" to ensure that the reader 
knows immediately what to do.
--
Philip 

Re: [PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[Junio C Hamano]

dturner@twopensource.com writes:

> diff --git a/sha1_name.c b/sha1_name.c
> index 6d10f05..23863f7 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1434,6 +1434,12 @@ static int get_sha1_with_context_1(const char *name,
>  			new_filename = resolve_relative_path(filename);
>  			if (new_filename)
>  				filename = new_filename;
> +			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
> +				ret = get_tree_enty_follow_symlinks(tree_sha1,
> +					filename, sha1, oc->path, &oc->mode);
> +				free(new_filename);
> +				return ret;
> +			}
>  			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
>  			if (ret && only_to_die) {
>  				diagnose_invalid_sha1_path(prefix, filename,

In general, I'd prefer to avoid this kind of "I only need this part
of the existing codeflow and I'll ignore rest by returning early".
You cannot anticipate any future needs of callers (e.g. only-to-die
flag might want to be passed when doing follow-symlinks in the
future) and clean-ups that may be added to the later part of the
function that you are skipping with this early return.

In other words,

	if (new_filename)
        	filename = new_filename;
-	ret = get_tree_entry(...);
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+		ret = ... whatever new thing you want to do ...
+	} else {
+		ret = get_tree_entry(... just reindented ... );
+	}
	if (ret && ...)

is what we would prefer to see here.

> @@ -1469,5 +1475,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
>  
>  int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
>  {
> +	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
> +		die(_("internal error: bad flags for get_sha1_with_context"));
>  	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
>  }

[PATCH 3/3] cat-file: add --follow-symlinks to --batch

[dturner]

From: David Turner <dturner@twitter.com>

This wires the in-repo-symlink following code through to the cat-file
builtin.  In the event of an out-of-repo link, cat-file will print
the link in a new format.

Signed-off-by: David Turner <dturner@twitter.com>
---
 Documentation/git-cat-file.txt |  28 ++++++-
 builtin/cat-file.c             |  23 +++++-
 t/t1006-cat-file.sh            | 184 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f6a16f4..18b67a3 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git cat-file' (-t | -s | -e | -p | <type> | --textconv ) <object>
-'git cat-file' (--batch | --batch-check) < <list-of-objects>
+'git cat-file' (--batch | --batch-check) [--follow-symlinks] < <list-of-objects>
 
 DESCRIPTION
 -----------
@@ -69,6 +69,19 @@ OPTIONS
 	not be combined with any other options or arguments.  See the
 	section `BATCH OUTPUT` below for details.
 
+--follow-symlinks::
+	Follow symlinks inside the repository.  Instead of providing
+	output about the link itself, provide output about the linked-to
+	object.  This option requires --batch or --batch-check.  In the
+	event of a symlink loop (or more than 40 symlinks in a symlink
+	resolution chain), the file will be treated as missing.  If a
+	symlink points outside the repository (e.g. a link to /foo or a
+	root-level link to ../foo), the portion of the link which is
+	outside the repository will be printed.  Follow-symlinks will
+	be silently turned off if <object> specifies an object in the
+	index rather than one in the object database.
+
+
 OUTPUT
 ------
 If '-t' is specified, one of the <type>.
@@ -148,6 +161,19 @@ the repository, then `cat-file` will ignore any custom format and print:
 <object> SP missing LF
 ------------
 
+If --follow-symlinks is used, and a symlink in the repository points
+outside the repository, then `cat-file` will ignore any custom format
+and print:
+
+------------
+symlink SP <size> LF <symlink> LF
+------------
+
+The symlink will either be absolute (beginning with a /), or relative
+to the repository root.  For instance, if dir/link points to ../../foo,
+then <symlink> will be ../foo.  <size> is the size of the symlink in
+bytes.
+
 
 CAVEATS
 -------
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df99df4..277af32 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -224,6 +224,7 @@ static void print_object_or_die(int fd, struct expand_data *data)
 
 struct batch_options {
 	int enabled;
+	int follow_symlinks;
 	int print_contents;
 	const char *format;
 };
@@ -232,16 +233,24 @@ static int batch_one_object(const char *obj_name, struct batch_options *opt,
 			    struct expand_data *data)
 {
 	struct strbuf buf = STRBUF_INIT;
+	struct object_context ctx;
+	int flags = opt->follow_symlinks ? GET_SHA1_FOLLOW_SYMLINKS : 0;
 
 	if (!obj_name)
 	   return 1;
 
-	if (get_sha1(obj_name, data->sha1)) {
+	if (get_sha1_with_context(obj_name, flags, data->sha1, &ctx)) {
 		printf("%s missing\n", obj_name);
 		fflush(stdout);
 		return 0;
 	}
 
+	if (ctx.mode == 0) {
+		printf("symlink %"PRIuMAX"\n%s\n", (uintmax_t)strlen(ctx.path),
+		       ctx.path);
+		return 0;
+	}
+
 	if (sha1_object_info_extended(data->sha1, &data->info, LOOKUP_REPLACE_OBJECT) < 0) {
 		printf("%s missing\n", obj_name);
 		fflush(stdout);
@@ -342,9 +351,8 @@ static int batch_option_callback(const struct option *opt,
 {
 	struct batch_options *bo = opt->value;
 
-	if (unset) {
-		memset(bo, 0, sizeof(*bo));
-		return 0;
+	if (bo->enabled) {
+		return 1;
 	}
 
 	bo->enabled = 1;
@@ -369,6 +377,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_SET_INT(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_SET_INT(0, "follow-symlinks", &batch.follow_symlinks,
+			N_("Follow in-repo symlinks; report out-of-repo symlinks (requires --batch or --batch-check)"),
+			    1),
 		{ OPTION_CALLBACK, 0, "batch", &batch, "format",
 			N_("show info and content of objects fed from the standard input"),
 			PARSE_OPT_OPTARG, batch_option_callback },
@@ -402,6 +413,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		usage_with_options(cat_file_usage, options);
 	}
 
+	if (batch.follow_symlinks && !batch.enabled) {
+		usage_with_options(cat_file_usage, options);
+	}
+
 	if (batch.enabled)
 		return batch_objects(&batch);
 
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index ab36b1e..a9ef3a6 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -189,6 +189,13 @@ do
     '
 done
 
+for opt in t s e p
+do
+    test_expect_success "Passing -$opt with --follow-symlinks fails" '
+	    test_must_fail git cat-file --follow-symlinks -$opt $hello_sha1
+	'
+done
+
 test_expect_success "--batch-check for a non-existent named object" '
     test "foobar42 missing
 foobar84 missing" = \
@@ -296,4 +303,181 @@ test_expect_success '%(deltabase) reports packed delta bases' '
 	}
 '
 
+# Tests for git cat-file --follow-symlinks
+test_expect_success 'prep for symlink tests' '
+	echo_without_newline "$hello_content" > morx &&
+	ln -s morx same-dir-link &&
+	ln -s ../fleem out-of-repo-link &&
+	ln -s .. out-of-repo-link-dir &&
+	ln -s same-dir-link link-to-link &&
+	ln -s nope broken-same-dir-link &&
+	mkdir dir &&
+	ln -s ../morx dir/parent-dir-link &&
+	ln -s .. dir/link-dir &&
+	ln -s ../../escape dir/out-of-repo-link &&
+	ln -s ../.. dir/out-of-repo-link-dir &&
+	ln -s nope dir/broken-link-in-dir &&
+	mkdir dir/subdir &&
+	ln -s ../../morx dir/subdir/grandparent-dir-link &&
+	ln -s ../../../great-escape dir/subdir/out-of-repo-link &&
+	ln -s ../../.. dir/subdir/out-of-repo-link-dir &&
+	ln -s ../../../ dir/subdir/out-of-repo-link-dir-trailing &&
+	ln -s ../parent-dir-link dir/subdir/parent-dir-link-to-link &&
+	echo_without_newline "$hello_content" >dir/subdir/ind2 &&
+	echo_without_newline "$hello_content" >dir/ind1 &&
+	ln -s dir dirlink &&
+	ln -s dir/subdir subdirlink &&
+	ln -s subdir/ind2 dir/link-to-child &&
+	ln -s dir/link-to-child link-to-down-link &&
+	ln -s dir/.. up-down &&
+	ln -s dir/../ up-down-trailing &&
+	ln -s dir/../morx up-down-file &&
+	ln -s dir/../../morx up-up-down-file &&
+	ln -s subdirlink/../../morx up-two-down-file &&
+	ln -s loop1 loop2 &&
+	ln -s loop2 loop1 &&
+	git add . &&
+	git commit -am "test"
+'
+
+echo $hello_sha1 blob $hello_size > found
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for non-links' '
+	echo HEAD:morx | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo HEAD:nope missing > expect &&
+	echo HEAD:nope | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for in-repo, same-dir links' '
+	echo HEAD:same-dir-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for broken in-repo, same-dir links' '
+	echo HEAD:broken-same-dir-link missing > expect &&
+	echo HEAD:broken-same-dir-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for same-dir links-to-links' '
+	echo HEAD:link-to-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for parent-dir links' '
+	echo HEAD:dir/parent-dir-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo HEAD:dir/parent-dir-link/nope missing > expect &&
+	echo HEAD:dir/parent-dir-link/nope | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for .. links' '
+	echo HEAD:dir/link-dir/nope missing > expect &&
+	echo HEAD:dir/link-dir/nope | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:dir/link-dir/morx | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo HEAD:dir/broken-link-in-dir missing > expect &&
+	echo HEAD:dir/broken-link-in-dir | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for ../.. links' '
+	echo HEAD:dir/subdir/grandparent-dir-link/nope missing > expect &&
+	echo HEAD:dir/subdir/grandparent-dir-link/nope | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:dir/subdir/grandparent-dir-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo HEAD:dir/subdir/parent-dir-link-to-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir/ links' '
+	echo HEAD:dirlink/morx missing > expect &&
+	echo HEAD:dirlink/morx | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo $hello_sha1 blob $hello_size > expect &&
+	echo HEAD:dirlink/ind1 | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir/subdir links' '
+	echo HEAD:subdirlink/morx missing > expect &&
+	echo HEAD:subdirlink/morx | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:subdirlink/ind2 | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir -> subdir links' '
+	echo HEAD:dir/link-to-child/morx missing > expect &&
+	echo HEAD:dir/link-to-child/morx | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:dir/link-to-child | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo HEAD:link-to-down-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks' '
+	echo symlink 8 > expect &&
+	echo ../fleem >> expect &&
+	echo HEAD:out-of-repo-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo symlink 2 > expect &&
+	echo .. >> expect &&
+	echo HEAD:out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks in dirs' '
+	echo symlink 9 > expect &&
+	echo ../escape >> expect &&
+	echo HEAD:dir/out-of-repo-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo symlink 2 > expect &&
+	echo .. >> expect &&
+	echo HEAD:dir/out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks in subdirs' '
+	echo symlink 15 > expect &&
+	echo ../great-escape >> expect &&
+	echo HEAD:dir/subdir/out-of-repo-link | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo symlink 2 > expect &&
+	echo .. >> expect &&
+	echo HEAD:dir/subdir/out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo symlink 3 > expect &&
+	echo ../ >> expect &&
+	echo HEAD:dir/subdir/out-of-repo-link-dir-trailing | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlinks works for symlinks with internal ..' '
+	echo HEAD: | git cat-file --batch-check > expect &&
+	echo HEAD:up-down | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:up-down-trailing | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:up-down-file | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual &&
+	echo symlink 7 > expect &&
+	echo ../morx >> expect &&
+	echo HEAD:up-up-down-file | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual &&
+	echo HEAD:up-two-down-file | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp found actual
+'
+
+test_expect_success 'git cat-file --batch-check --follow-symlink breaks loops' '
+	echo HEAD:loop1 missing > expect &&
+	echo HEAD:loop1 | git cat-file --batch-check --follow-symlinks > actual &&
+	test_cmp expect actual
+'
 test_done
-- 
2.0.4.315.gad8727a-twtrsrc

Re: [PATCH 3/3] cat-file: add --follow-symlinks to --batch

[Eric Sunshine]

On Fri, May 8, 2015 at 2:13 PM,  <dturner@twopensource.com> wrote:
> This wires the in-repo-symlink following code through to the cat-file
> builtin.  In the event of an out-of-repo link, cat-file will print
> the link in a new format.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index df99df4..277af32 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -369,6 +377,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
>                 OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
>                 OPT_SET_INT(0, "textconv", &opt,
>                             N_("for blob objects, run textconv on object's content"), 'c'),
> +               OPT_SET_INT(0, "follow-symlinks", &batch.follow_symlinks,
> +                       N_("Follow in-repo symlinks; report out-of-repo symlinks (requires --batch or --batch-check)"),
> +                           1),

Capitalization is not used for descriptions of the other options.

>                 { OPTION_CALLBACK, 0, "batch", &batch, "format",
>                         N_("show info and content of objects fed from the standard input"),
>                         PARSE_OPT_OPTARG, batch_option_callback },

Re: [PATCH 1/3] tree-walk: learn get_tree_enty_follow_symlinks

[Junio C Hamano]

dturner@twopensource.com writes:

> From: David Turner <dturner@twitter.com>
>
> Add a new function, get_tree_entry_follow_symlinks, to tree-walk.[ch].
> The function is not yet used.  It will be used to implement git
> cat-file --batch --follow-symlinks.
>
> The function locates an object by path, following symlinks in the
> repository.  If the symlinks lead outside the repository, the function
> reports this to the caller.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---
>  tree-walk.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tree-walk.h |   2 +
>  2 files changed, 224 insertions(+)
>
> diff --git a/tree-walk.c b/tree-walk.c
> index 5dd9a71..6fb4b7d 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -415,6 +415,228 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>  	return error;
>  }
>  
> +static int find_tree_entry_nonrecursive(struct tree_desc *t, char *name, unsigned char *result, unsigned *mode) {
> +	int namelen = strlen(name);
> +
> +	while (t->size) {

Why do you need an almost duplicate of existing find_tree_entry()
here?  The argument "name" above is not const, so isn't that just
the matter of the caller to temporarily replace '/' in name[] before
calling find_tree_entry() if all you wanted to avoid was to prevent
it from falling into get_tree_entry() codepath?  Or are there more
to this function?

> +#define GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS 40

Is 40 just a randomly chosen number?

I do not think 40 is particularly unreasonable, but so is 5 (which I
think is also reasonable and is already used as MAXDEPTH in refs.c
to follow symrefs), and am curious where that number came from.

> +/**
> + * Find a tree entry by following symlinks in tree_sha (which is
> + * assumed to be the root of the repository).  In the event that a
> + * symlink points outside the repository (e.g. a link to /foo or a
> + * root-level link to ../foo), the portion of the link which is
> + * outside the repository will be copied into result_path (which is
> + * assumed to hold at least PATH_MAX bytes), and *mode will be set to
> + * 0.

As the API to this new function is not constrained by existing
callers, you might want to consider using strbuf for result_path,
which would make it easier for both the callers and this function.

> +int get_tree_enty_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, unsigned char *result_path, unsigned *mode)
> +{
> +	int retval = -1;
> +	void *tree;
> +	struct dir_state *parents = NULL;
> +	size_t parents_cap = 0;
> +	ssize_t parents_len = 0;

It is customary to name variables to control an ALLOC_GROW()-managed
array 'foo' as foo_nr and foo_alloc.  Deviating from the convention
makes the patch harder to read by people who are familiar with it
without any benefit, and those who are familiar with the existing
code are the people you want your patch reviewed by.

I am context-switching now; will review the remainder some other
time.

Thanks.

Re: [PATCH 1/3] tree-walk: learn get_tree_enty_follow_symlinks

[David Turner]

On Fri, 2015-05-08 at 12:26 -0700, Junio C Hamano wrote:
> > +static int find_tree_entry_nonrecursive(struct tree_desc *t, char *name, unsigned char *result, unsigned *mode) {
> > +	int namelen = strlen(name);
> > +
> > +	while (t->size) {
> 
> Why do you need an almost duplicate of existing find_tree_entry()
> here?  The argument "name" above is not const, so isn't that just
> the matter of the caller to temporarily replace '/' in name[] before
> calling find_tree_entry() if all you wanted to avoid was to prevent
> it from falling into get_tree_entry() codepath?  Or are there more
> to this function?

Oh, right, actually, we already replaced the '/'!  So I can just use the
existing one.  That was silly.  

> > +#define GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS 40
> 
> Is 40 just a randomly chosen number?
> 
> I do not think 40 is particularly unreasonable, but so is 5 (which I
> think is also reasonable and is already used as MAXDEPTH in refs.c
> to follow symrefs), and am curious where that number came from.

http://lxr.linux.no/linux+v3.6.4/fs/namei.c#L783

I'll add a comment to that effect.

> > +/**
> > + * Find a tree entry by following symlinks in tree_sha (which is
> > + * assumed to be the root of the repository).  In the event that a
> > + * symlink points outside the repository (e.g. a link to /foo or a
> > + * root-level link to ../foo), the portion of the link which is
> > + * outside the repository will be copied into result_path (which is
> > + * assumed to hold at least PATH_MAX bytes), and *mode will be set to
> > + * 0.
> 
> As the API to this new function is not constrained by existing
> callers, you might want to consider using strbuf for result_path,
> which would make it easier for both the callers and this function.

The API is sort-of constrained, because the caller has a fixed-size
buffer to fill in (see the 2nd patch in this series).  

> It is customary to name variables to control an ALLOC_GROW()-managed
> array 'foo' as foo_nr and foo_alloc.  Deviating from the convention
> makes the patch harder to read by people who are familiar with it
> without any benefit, and those who are familiar with the existing
> code are the people you want your patch reviewed by.

Will fix.

> I am context-switching now; will review the remainder some other
> time.

Thanks for the review.

Re: [PATCH 1/3] tree-walk: learn get_tree_enty_follow_symlinks

[Eric Sunshine]

On Fri, May 8, 2015 at 2:13 PM,  <dturner@twopensource.com> wrote:
> tree-walk: learn get_tree_enty_follow_symlinks

s/enty/entry/ here and in several places in the patch itself.

> Add a new function, get_tree_entry_follow_symlinks, to tree-walk.[ch].
> The function is not yet used.  It will be used to implement git
> cat-file --batch --follow-symlinks.
>
> The function locates an object by path, following symlinks in the
> repository.  If the symlinks lead outside the repository, the function
> reports this to the caller.
>
> Signed-off-by: David Turner <dturner@twitter.com>
> ---
> diff --git a/tree-walk.c b/tree-walk.c
> index 5dd9a71..6fb4b7d 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -415,6 +415,228 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>  static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char *result, unsigned *mode)
>  {
> diff --git a/tree-walk.h b/tree-walk.h
> index ae7fb3a..002e5a9 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -40,6 +40,8 @@ struct traverse_info;
>  typedef int (*traverse_callback_t)(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *);
>  int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info);
>
> +int get_tree_enty_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, unsigned char *result_path, unsigned *mode);

[PATCH v8 0/3] git cat-file --follow-symlinks

[David Turner]

This version includes the following:
(a) fix leak in tree-walk.c, thanks to Jeff King
(b) asciidoc now correctly indented when rendered as HTML and when
rendered as a man page.
(c) type casting is now correct
(d) Added a default to switch statement to shut up stupid compilers.

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.

Signed-off-by: David Turner <dturner@twitter.com>
---
 cache.h     | 20 +++++++++++++-------
 sha1_name.c | 20 +++++++++++++++-----
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
 	unsigned char tree[20];
 	char path[PATH_MAX];
 	unsigned mode;
+	/*
+	 * symlink_path is only used by get_tree_entry_follow_symlinks,
+	 * and only for symlinks that point outside the repository.
+	 */
+	struct strbuf symlink_path;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
-			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
-			if (ret && only_to_die) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1,
-							   name, len);
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_entry_follow_symlinks(tree_sha1,
+					filename, sha1, &oc->symlink_path,
+					&oc->mode);
+			} else {
+				ret = get_tree_entry(tree_sha1, filename,
+						     sha1, &oc->mode);
+				if (ret && only_to_die) {
+					diagnose_invalid_sha1_path(prefix,
+								   filename,
+								   tree_sha1,
+								   name, len);
+				}
 			}
 			hashcpy(oc->tree, tree_sha1);
 			strlcpy(oc->path, filename, sizeof(oc->path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die("BUG: incompatible flags for get_sha1_with_context");
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc

[PATCH v9 0/3]

[David Turner]

Oops, forgot to ammend commit before patch v8.  This patch *really*
includes the switch and type casting fixes.

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.

Signed-off-by: David Turner <dturner@twitter.com>
---
 cache.h     | 20 +++++++++++++-------
 sha1_name.c | 20 +++++++++++++++-----
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
 	unsigned char tree[20];
 	char path[PATH_MAX];
 	unsigned mode;
+	/*
+	 * symlink_path is only used by get_tree_entry_follow_symlinks,
+	 * and only for symlinks that point outside the repository.
+	 */
+	struct strbuf symlink_path;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
-			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
-			if (ret && only_to_die) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1,
-							   name, len);
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_entry_follow_symlinks(tree_sha1,
+					filename, sha1, &oc->symlink_path,
+					&oc->mode);
+			} else {
+				ret = get_tree_entry(tree_sha1, filename,
+						     sha1, &oc->mode);
+				if (ret && only_to_die) {
+					diagnose_invalid_sha1_path(prefix,
+								   filename,
+								   tree_sha1,
+								   name, len);
+				}
 			}
 			hashcpy(oc->tree, tree_sha1);
 			strlcpy(oc->path, filename, sizeof(oc->path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die("BUG: incompatible flags for get_sha1_with_context");
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc

[PATCH v10 0/3]

[David Turner]

Change text explaining that --follow-symlinks only works with
--batch{-check,}; now it is presented as a missing feature rather than
a necessary evil.

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.

Signed-off-by: David Turner <dturner@twitter.com>
---
 cache.h     | 20 +++++++++++++-------
 sha1_name.c | 20 +++++++++++++++-----
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
 	unsigned char tree[20];
 	char path[PATH_MAX];
 	unsigned mode;
+	/*
+	 * symlink_path is only used by get_tree_entry_follow_symlinks,
+	 * and only for symlinks that point outside the repository.
+	 */
+	struct strbuf symlink_path;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
-			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
-			if (ret && only_to_die) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1,
-							   name, len);
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_entry_follow_symlinks(tree_sha1,
+					filename, sha1, &oc->symlink_path,
+					&oc->mode);
+			} else {
+				ret = get_tree_entry(tree_sha1, filename,
+						     sha1, &oc->mode);
+				if (ret && only_to_die) {
+					diagnose_invalid_sha1_path(prefix,
+								   filename,
+								   tree_sha1,
+								   name, len);
+				}
 			}
 			hashcpy(oc->tree, tree_sha1);
 			strlcpy(oc->path, filename, sizeof(oc->path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die("BUG: incompatible flags for get_sha1_with_context");
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc

[PATCH v11 0/3] cat-file --follow-symlinks

[David Turner]

Squashes Junio's formatting and text to cat-file.c on top of my doco
fixes from v10.

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.

Signed-off-by: David Turner <dturner@twitter.com>
---
 cache.h     | 20 +++++++++++++-------
 sha1_name.c | 20 +++++++++++++++-----
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
 	unsigned char tree[20];
 	char path[PATH_MAX];
 	unsigned mode;
+	/*
+	 * symlink_path is only used by get_tree_entry_follow_symlinks,
+	 * and only for symlinks that point outside the repository.
+	 */
+	struct strbuf symlink_path;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
-			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
-			if (ret && only_to_die) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1,
-							   name, len);
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_entry_follow_symlinks(tree_sha1,
+					filename, sha1, &oc->symlink_path,
+					&oc->mode);
+			} else {
+				ret = get_tree_entry(tree_sha1, filename,
+						     sha1, &oc->mode);
+				if (ret && only_to_die) {
+					diagnose_invalid_sha1_path(prefix,
+								   filename,
+								   tree_sha1,
+								   name, len);
+				}
 			}
 			hashcpy(oc->tree, tree_sha1);
 			strlcpy(oc->path, filename, sizeof(oc->path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die("BUG: incompatible flags for get_sha1_with_context");
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc

[PATCH v12 0/3] git cat-file --follow-symlinks

[David Turner]

Adjust asciidoc to use +-continuation with non-indented sections
instead of [normal].

[PATCH 2/3] sha1_name: get_sha1_with_context learns to follow symlinks

[David Turner]

From: David Turner <dturner@twitter.com>

Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.

Signed-off-by: David Turner <dturner@twopensource.com>
---
 cache.h     | 20 +++++++++++++-------
 sha1_name.c | 20 +++++++++++++++-----
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
 	unsigned char tree[20];
 	char path[PATH_MAX];
 	unsigned mode;
+	/*
+	 * symlink_path is only used by get_tree_entry_follow_symlinks,
+	 * and only for symlinks that point outside the repository.
+	 */
+	struct strbuf symlink_path;
 };
 
-#define GET_SHA1_QUIETLY        01
-#define GET_SHA1_COMMIT         02
-#define GET_SHA1_COMMITTISH     04
-#define GET_SHA1_TREE          010
-#define GET_SHA1_TREEISH       020
-#define GET_SHA1_BLOB	       040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY           01
+#define GET_SHA1_COMMIT            02
+#define GET_SHA1_COMMITTISH        04
+#define GET_SHA1_TREE             010
+#define GET_SHA1_TREEISH          020
+#define GET_SHA1_BLOB             040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE    04000
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
 			new_filename = resolve_relative_path(filename);
 			if (new_filename)
 				filename = new_filename;
-			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
-			if (ret && only_to_die) {
-				diagnose_invalid_sha1_path(prefix, filename,
-							   tree_sha1,
-							   name, len);
+			if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
+				ret = get_tree_entry_follow_symlinks(tree_sha1,
+					filename, sha1, &oc->symlink_path,
+					&oc->mode);
+			} else {
+				ret = get_tree_entry(tree_sha1, filename,
+						     sha1, &oc->mode);
+				if (ret && only_to_die) {
+					diagnose_invalid_sha1_path(prefix,
+								   filename,
+								   tree_sha1,
+								   name, len);
+				}
 			}
 			hashcpy(oc->tree, tree_sha1);
 			strlcpy(oc->path, filename, sizeof(oc->path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
 
 int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
 {
+	if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
+		die("BUG: incompatible flags for get_sha1_with_context");
 	return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
 }
-- 
2.0.4.315.gad8727a-twtrsrc