{"thread":{"id":"39305","subject":"[PATCH v5 1/3] tree-walk: learn get_tree_entry_follow_symlinks","startedAt":"2015-05-11T22:50:42Z","lastAt":"2015-05-14T20:06:39Z","messageCount":20,"participants":["dturner@twopensource.com","Johannes Sixt","Junio C Hamano","David Turner"],"isPatch":true,"patchVersion":5,"patchTotal":3},"messages":[{"id":"261028","messageId":"1431384645-17276-1-git-send-email-dturner@twopensource.com","threadId":"39305","inReplyTo":null,"subject":"[PATCH v5 0/3] cat-file --follow-symlinks","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-11T22:50:42Z","receivedAt":"2015-05-11T22:50:42Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"This version improves the documentation for git cat-file\n--follow-symlinks.  Only Patch 3/3 is changed.\n\nThanks to Junio Hamano for suggestions.\n"},{"id":"261025","messageId":"1431384645-17276-2-git-send-email-dturner@twopensource.com","threadId":"39305","inReplyTo":"1431384645-17276-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 1/3] tree-walk: learn get_tree_entry_follow_symlinks","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-11T22:50:43Z","receivedAt":"2015-05-11T22:50:43Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner <dturner@twitter.com>\n\nAdd a new function, get_tree_entry_follow_symlinks, to tree-walk.[ch].\nThe function is not yet used.  It will be used to implement git\ncat-file --batch --follow-symlinks.\n\nThe function locates an object by path, following symlinks in the\nrepository.  If the symlinks lead outside the repository, the function\nreports this to the caller.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n tree-walk.c | 193 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n tree-walk.h |   2 +\n 2 files changed, 195 insertions(+)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 5dd9a71..3d088b6 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -415,6 +415,12 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)\n \treturn error;\n }\n \n+struct dir_state {\n+\tvoid *tree;\n+\tunsigned long size;\n+\tunsigned char sha1[20];\n+};\n+\n static int find_tree_entry(struct tree_desc *t, const char *name, unsigned char *result, unsigned *mode)\n {\n \tint namelen = strlen(name);\n@@ -478,6 +484,193 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch\n \treturn retval;\n }\n \n+/*\n+ * This is Linux's built-in max for the number of symlinks to follow.\n+ * That limit, of course, does not affect git, but it's a reasonable\n+ * choice.\n+ */\n+#define GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS 40\n+\n+/**\n+ * Find a tree entry by following symlinks in tree_sha (which is\n+ * assumed to be the root of the repository).  In the event that a\n+ * symlink points outside the repository (e.g. a link to /foo or a\n+ * root-level link to ../foo), the portion of the link which is\n+ * outside the repository will be returned in result_path, and *mode\n+ * will be set to 0.  It is assumed that result_path is uninitialized.\n+ * If there are no symlinks, or the end result of the symlink chain\n+ * points to an object inside the repository, result will be filled in\n+ * with the sha1 of the found object, and *mode will hold the mode of\n+ * the object.  If there is a symlink loop, -1 will be returned.\n+ */\n+int get_tree_entry_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, struct strbuf *result_path, unsigned *mode)\n+{\n+\tint retval = -1;\n+\tstruct dir_state *parents = NULL;\n+\tsize_t parents_alloc = 0;\n+\tssize_t parents_nr = 0;\n+\tunsigned char current_tree_sha1[20];\n+\tstruct strbuf namebuf = STRBUF_INIT;\n+\tstruct tree_desc t = {0};\n+\tint follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;\n+\tint i;\n+\n+\tresult_path->buf = 0;\n+\tresult_path->alloc = 0;\n+\tresult_path->len = 0;\n+\tstrbuf_addstr(&namebuf, name);\n+\thashcpy(current_tree_sha1, tree_sha1);\n+\n+\twhile (1) {\n+\t\tint find_result;\n+\t\tchar *first_slash;\n+\t\tchar *remainder = NULL;\n+\n+\t\tif (!t.buffer) {\n+\t\t\tvoid *tree;\n+\t\t\tunsigned char root[20];\n+\t\t\tunsigned long size;\n+\t\t\ttree = read_object_with_reference(current_tree_sha1,\n+\t\t\t\t\t\t\t  tree_type, &size,\n+\t\t\t\t\t\t\t  root);\n+\t\t\tif (!tree)\n+\t\t\t\tgoto done;\n+\n+\t\t\tALLOC_GROW(parents, parents_nr + 1, parents_alloc);\n+\t\t\tparents[parents_nr].tree = tree;\n+\t\t\tparents[parents_nr].size = size;\n+\t\t\thashcpy(parents[parents_nr].sha1, root);\n+\t\t\tparents_nr++;\n+\n+\t\t\tif (namebuf.buf[0] == '\\0') {\n+\t\t\t\thashcpy(result, root);\n+\t\t\t\tretval = 0;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n+\t\t\tif (!size)\n+\t\t\t\tgoto done;\n+\n+\t\t\t/* descend */\n+\t\t\tinit_tree_desc(&t, tree, size);\n+\t\t}\n+\n+\t\t/* Handle symlinks to e.g. a//b by removing leading slashes */\n+\t\twhile (namebuf.buf[0] == '/') {\n+\t\t\tstrbuf_remove(&namebuf, 0, 1);\n+\t\t}\n+\n+\t\t/* Split namebuf into a first component and a remainder */\n+\t\tif ((first_slash = strchr(namebuf.buf, '/'))) {\n+\t\t\t*first_slash = 0;\n+\t\t\tremainder = first_slash + 1;\n+\t\t}\n+\n+\t\tif (!strcmp(namebuf.buf, \"..\")) {\n+\t\t\tstruct dir_state *parent;\n+\t\t\t/*\n+\t\t\t * We could end up with .. in the namebuf if it\n+\t\t\t * appears in a symlink.\n+\t\t\t */\n+\n+\t\t\tif (parents_nr == 1) {\n+\t\t\t\tif (remainder)\n+\t\t\t\t\t*first_slash = '/';\n+\t\t\t\tstrbuf_add(result_path, namebuf.buf,\n+\t\t\t\t\t   namebuf.len);\n+\t\t\t\t*mode = 0;\n+\t\t\t\tretval = 0;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t\tparent = &parents[parents_nr - 1];\n+\t\t\tfree(parent->tree);\n+\t\t\tparents_nr--;\n+\t\t\tparent = &parents[parents_nr - 1];\n+\t\t\tinit_tree_desc(&t, parent->tree, parent->size);\n+\t\t\tstrbuf_remove(&namebuf, 0, remainder ? 3 : 2);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* We could end up here via a symlink to dir/.. */\n+\t\tif (namebuf.buf[0] == '\\0') {\n+\t\t\thashcpy(result, parents[parents_nr - 1].sha1);\n+\t\t\tretval = 0;\n+\t\t\tgoto done;\n+\t\t}\n+\n+\t\t/* Look up the first (or only) path component in the tree. */\n+\t\tfind_result = find_tree_entry(&t, namebuf.buf,\n+\t\t\t\t\t      current_tree_sha1, mode);\n+\t\tif (find_result) {\n+\t\t\tretval = find_result;\n+\t\t\tgoto done;\n+\t\t}\n+\n+\t\tif (S_ISDIR(*mode)) {\n+\t\t\tif (!remainder) {\n+\t\t\t\thashcpy(result, current_tree_sha1);\n+\t\t\t\tretval = 0;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t\t/* Descend the tree */\n+\t\t\tt.buffer = NULL;\n+\t\t\tstrbuf_remove(&namebuf, 0,\n+\t\t\t\t      1 + first_slash - namebuf.buf);\n+\t\t} else if (S_ISREG(*mode)) {\n+\t\t\tif (!remainder) {\n+\t\t\t\thashcpy(result, current_tree_sha1);\n+\t\t\t\tretval = 0;\n+\t\t\t}\n+\t\t\tgoto done;\n+\t\t} else if (S_ISLNK(*mode)) {\n+\t\t\t/* Follow a symlink */\n+\t\t\tsize_t link_len, len;\n+\t\t\tchar *contents, *contents_start;\n+\t\t\tstruct dir_state *parent;\n+\t\t\tenum object_type type;\n+\n+\t\t\tif (follows_remaining-- == 0)\n+\t\t\t\t/* Too many symlinks followed */\n+\t\t\t\tgoto done;\n+\n+\t\t\tcontents = read_sha1_file(current_tree_sha1, &type,\n+\t\t\t\t\t\t  &link_len);\n+\n+\t\t\tif (!contents)\n+\t\t\t\tgoto done;\n+\n+\t\t\tif (contents[0] == '/') {\n+\t\t\t\tstrbuf_addstr(result_path, contents);\n+\t\t\t\t*mode = 0;\n+\t\t\t\tretval = 0;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n+\t\t\tif (remainder)\n+\t\t\t\tlen = first_slash - namebuf.buf;\n+\t\t\telse\n+\t\t\t\tlen = namebuf.len;\n+\n+\t\t\tcontents_start = contents;\n+\n+\t\t\tparent = &parents[parents_nr - 1];\n+\t\t\tinit_tree_desc(&t, parent->tree, parent->size);\n+\t\t\tstrbuf_splice(&namebuf, 0, len,\n+\t\t\t\t      contents_start, link_len);\n+\t\t\tif (remainder)\n+\t\t\t\tnamebuf.buf[link_len] = '/';\n+\t\t\tfree(contents);\n+\t\t}\n+\t}\n+done:\n+\tfor (i = 0; i < parents_nr; i++)\n+\t\tfree(parents[i].tree);\n+\tfree(parents);\n+\n+\tstrbuf_release(&namebuf);\n+\treturn retval;\n+}\n+\n static int match_entry(const struct pathspec_item *item,\n \t\t       const struct name_entry *entry, int pathlen,\n \t\t       const char *match, int matchlen,\ndiff --git a/tree-walk.h b/tree-walk.h\nindex ae7fb3a..d1670c6 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -40,6 +40,8 @@ struct traverse_info;\n typedef int (*traverse_callback_t)(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *);\n int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info);\n \n+int get_tree_entry_follow_symlinks(unsigned char *tree_sha1, const char *name, unsigned char *result, struct strbuf *result_path, unsigned *mode);\n+\n struct traverse_info {\n \tstruct traverse_info *prev;\n \tstruct name_entry name;\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"261026","messageId":"1431384645-17276-3-git-send-email-dturner@twopensource.com","threadId":"39305","inReplyTo":"1431384645-17276-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 2/3] sha1_name: get_sha1_with_context learns to follow symlinks","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-11T22:50:44Z","receivedAt":"2015-05-11T22:50:44Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner <dturner@twitter.com>\n\nWire up get_sha1_with_context to call get_tree_entry_follow_symlinks\nwhen GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS\nis incompatible with G_S_ONLY_TO_DIE because the diagnosis\nthat ONLY_TO_DIE triggers does not at present consider symlinks, and\nit would be a significant amount of additional code to allow it to\ndo so.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n cache.h     | 20 +++++++++++++-------\n sha1_name.c | 20 +++++++++++++++-----\n 2 files changed, 28 insertions(+), 12 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 3d3244b..65505d1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -922,15 +922,21 @@ struct object_context {\n \tunsigned char tree[20];\n \tchar path[PATH_MAX];\n \tunsigned mode;\n+\t/*\n+\t * symlink_path is only used by get_tree_entry_follow_symlinks,\n+\t * and only for symlinks that point outside the repository.\n+\t */\n+\tstruct strbuf symlink_path;\n };\n \n-#define GET_SHA1_QUIETLY        01\n-#define GET_SHA1_COMMIT         02\n-#define GET_SHA1_COMMITTISH     04\n-#define GET_SHA1_TREE          010\n-#define GET_SHA1_TREEISH       020\n-#define GET_SHA1_BLOB\t       040\n-#define GET_SHA1_ONLY_TO_DIE 04000\n+#define GET_SHA1_QUIETLY           01\n+#define GET_SHA1_COMMIT            02\n+#define GET_SHA1_COMMITTISH        04\n+#define GET_SHA1_TREE             010\n+#define GET_SHA1_TREEISH          020\n+#define GET_SHA1_BLOB             040\n+#define GET_SHA1_FOLLOW_SYMLINKS 0100\n+#define GET_SHA1_ONLY_TO_DIE    04000\n \n extern int get_sha1(const char *str, unsigned char *sha1);\n extern int get_sha1_commit(const char *str, unsigned char *sha1);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 6d10f05..0c26515 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,\n \t\t\tnew_filename = resolve_relative_path(filename);\n \t\t\tif (new_filename)\n \t\t\t\tfilename = new_filename;\n-\t\t\tret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);\n-\t\t\tif (ret && only_to_die) {\n-\t\t\t\tdiagnose_invalid_sha1_path(prefix, filename,\n-\t\t\t\t\t\t\t   tree_sha1,\n-\t\t\t\t\t\t\t   name, len);\n+\t\t\tif (flags & GET_SHA1_FOLLOW_SYMLINKS) {\n+\t\t\t\tret = get_tree_entry_follow_symlinks(tree_sha1,\n+\t\t\t\t\tfilename, sha1, &oc->symlink_path,\n+\t\t\t\t\t&oc->mode);\n+\t\t\t} else {\n+\t\t\t\tret = get_tree_entry(tree_sha1, filename,\n+\t\t\t\t\t\t     sha1, &oc->mode);\n+\t\t\t\tif (ret && only_to_die) {\n+\t\t\t\t\tdiagnose_invalid_sha1_path(prefix,\n+\t\t\t\t\t\t\t\t   filename,\n+\t\t\t\t\t\t\t\t   tree_sha1,\n+\t\t\t\t\t\t\t\t   name, len);\n+\t\t\t\t}\n \t\t\t}\n \t\t\thashcpy(oc->tree, tree_sha1);\n \t\t\tstrlcpy(oc->path, filename, sizeof(oc->path));\n@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)\n \n int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)\n {\n+\tif (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)\n+\t\tdie(\"BUG: incompatible flags for get_sha1_with_context\");\n \treturn get_sha1_with_context_1(str, flags, NULL, sha1, orc);\n }\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"261027","messageId":"1431384645-17276-4-git-send-email-dturner@twopensource.com","threadId":"39305","inReplyTo":"1431384645-17276-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-11T22:50:45Z","receivedAt":"2015-05-11T22:50:45Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner <dturner@twitter.com>\n\nThis wires the in-repo-symlink following code through to the cat-file\nbuiltin.  In the event of an out-of-repo link, cat-file will print\nthe link in a new format.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n Documentation/git-cat-file.txt |  54 +++++++++++-\n builtin/cat-file.c             |  24 +++++-\n t/t1006-cat-file.sh            | 189 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 262 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f6a16f4..9bdfced 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git cat-file' (-t | -s | -e | -p | <type> | --textconv ) <object>\n-'git cat-file' (--batch | --batch-check) < <list-of-objects>\n+'git cat-file' (--batch | --batch-check) [--follow-symlinks] < <list-of-objects>\n \n DESCRIPTION\n -----------\n@@ -69,6 +69,46 @@ OPTIONS\n \tnot be combined with any other options or arguments.  See the\n \tsection `BATCH OUTPUT` below for details.\n \n+--follow-symlinks::\n+\tFollow symlinks inside the repository when requesting objects\n+\twith extended SHA-1 expressions of the form tree-ish:path-in-tree.\n+\tInstead of providing output about the link itself, provide output\n+\tabout the linked-to object.  This option requires --batch or\n+\t--batch-check.  In the event of a symlink loop (or more than\n+\t40 symlinks in a symlink resolution chain), the file will be\n+\ttreated as missing.  If a symlink points outside the tree-ish\n+\t(e.g. a link to /foo or a root-level link to ../foo), the\n+\tportion of the link which is outside the tree will be printed.\n+\tFollow-symlinks will be silently turned off if <object>\n+\tspecifies an object in the index rather than one in the object\n+\tdatabase.\n+\n+\tFor example, consider the a git repository containing:\n+\tf: a file containing \"hello\\n\"\n+\tlink: a symlink to f\n+\tdir/link: a symlink to ../f\n+\tplink: a symlink to ../f\n+\talink: a symlink to /etc/passwd\n+\n+\techo HEAD:f |git cat-file --batch --follow-symlinks would print\n+\tce013625030ba8dba906f756967f9e9ca394464a blob 6\n+\n+\techo HEAD:link |git cat-file --batch --follow-symlinks would print\n+\tthe same thing, as would HEAD:dir/link.\n+\tWithout follow-symlinks, these would print data about the\n+\tsymlink itself.  In the case of HEAD:link, you would see\n+\t4d1ae35ba2c8ec712fa2a379db44ad639ca277bd blob 1\n+\n+\tBoth plink and alink point outside the tree, so they would\n+\trespectively print:\n+\tsymlink 4\n+\t../f\n+\n+\tsymlink 11\n+\t/etc/passwd\n+\n+\n+\n OUTPUT\n ------\n If '-t' is specified, one of the <type>.\n@@ -148,6 +188,18 @@ the repository, then `cat-file` will ignore any custom format and print:\n <object> SP missing LF\n ------------\n \n+If --follow-symlinks is used, and a symlink in the repository points\n+outside the repository, then `cat-file` will ignore any custom format\n+and print:\n+\n+------------\n+symlink SP <size> LF <symlink> LF\n+------------\n+\n+The symlink will either be absolute (beginning with a /), or relative\n+to the tree root.  For instance, if dir/link points to ../../foo, then\n+<symlink> will be ../foo.  <size> is the size of the symlink in bytes.\n+\n \n CAVEATS\n -------\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex df99df4..6bcf9a0 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -224,6 +224,7 @@ static void print_object_or_die(int fd, struct expand_data *data)\n \n struct batch_options {\n \tint enabled;\n+\tint follow_symlinks;\n \tint print_contents;\n \tconst char *format;\n };\n@@ -232,16 +233,25 @@ static int batch_one_object(const char *obj_name, struct batch_options *opt,\n \t\t\t    struct expand_data *data)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct object_context ctx;\n+\tint flags = opt->follow_symlinks ? GET_SHA1_FOLLOW_SYMLINKS : 0;\n \n \tif (!obj_name)\n \t   return 1;\n \n-\tif (get_sha1(obj_name, data->sha1)) {\n+\tif (get_sha1_with_context(obj_name, flags, data->sha1, &ctx)) {\n \t\tprintf(\"%s missing\\n\", obj_name);\n \t\tfflush(stdout);\n \t\treturn 0;\n \t}\n \n+\tif (ctx.mode == 0) {\n+\t\tprintf(\"symlink %\"PRIuMAX\"\\n%s\\n\",\n+\t\t       (uintmax_t)ctx.symlink_path.len,\n+\t\t       ctx.symlink_path.buf);\n+\t\treturn 0;\n+\t}\n+\n \tif (sha1_object_info_extended(data->sha1, &data->info, LOOKUP_REPLACE_OBJECT) < 0) {\n \t\tprintf(\"%s missing\\n\", obj_name);\n \t\tfflush(stdout);\n@@ -342,9 +352,8 @@ static int batch_option_callback(const struct option *opt,\n {\n \tstruct batch_options *bo = opt->value;\n \n-\tif (unset) {\n-\t\tmemset(bo, 0, sizeof(*bo));\n-\t\treturn 0;\n+\tif (bo->enabled) {\n+\t\treturn 1;\n \t}\n \n \tbo->enabled = 1;\n@@ -369,6 +378,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT('p', NULL, &opt, N_(\"pretty-print object's content\"), 'p'),\n \t\tOPT_SET_INT(0, \"textconv\", &opt,\n \t\t\t    N_(\"for blob objects, run textconv on object's content\"), 'c'),\n+\t\tOPT_SET_INT(0, \"follow-symlinks\", &batch.follow_symlinks,\n+\t\t\tN_(\"follow in-repo symlinks; report out-of-repo symlinks (requires --batch or --batch-check)\"),\n+\t\t\t    1),\n \t\t{ OPTION_CALLBACK, 0, \"batch\", &batch, \"format\",\n \t\t\tN_(\"show info and content of objects fed from the standard input\"),\n \t\t\tPARSE_OPT_OPTARG, batch_option_callback },\n@@ -402,6 +414,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(cat_file_usage, options);\n \t}\n \n+\tif (batch.follow_symlinks && !batch.enabled) {\n+\t\tusage_with_options(cat_file_usage, options);\n+\t}\n+\n \tif (batch.enabled)\n \t\treturn batch_objects(&batch);\n \ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex ab36b1e..500cfa9 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -189,6 +189,13 @@ do\n     '\n done\n \n+for opt in t s e p\n+do\n+    test_expect_success \"Passing -$opt with --follow-symlinks fails\" '\n+\t    test_must_fail git cat-file --follow-symlinks -$opt $hello_sha1\n+\t'\n+done\n+\n test_expect_success \"--batch-check for a non-existent named object\" '\n     test \"foobar42 missing\n foobar84 missing\" = \\\n@@ -296,4 +303,186 @@ test_expect_success '%(deltabase) reports packed delta bases' '\n \t}\n '\n \n+# Tests for git cat-file --follow-symlinks\n+test_expect_success 'prep for symlink tests' '\n+\techo_without_newline \"$hello_content\" >morx &&\n+\ttest_ln_s_add morx same-dir-link &&\n+\ttest_ln_s_add ../fleem out-of-repo-link &&\n+\ttest_ln_s_add .. out-of-repo-link-dir &&\n+\ttest_ln_s_add same-dir-link link-to-link &&\n+\ttest_ln_s_add nope broken-same-dir-link &&\n+\tmkdir dir &&\n+\ttest_ln_s_add ../morx dir/parent-dir-link &&\n+\ttest_ln_s_add .. dir/link-dir &&\n+\ttest_ln_s_add ../../escape dir/out-of-repo-link &&\n+\ttest_ln_s_add ../.. dir/out-of-repo-link-dir &&\n+\ttest_ln_s_add nope dir/broken-link-in-dir &&\n+\tmkdir dir/subdir &&\n+\ttest_ln_s_add ../../morx dir/subdir/grandparent-dir-link &&\n+\ttest_ln_s_add ../../../great-escape dir/subdir/out-of-repo-link &&\n+\ttest_ln_s_add ../../.. dir/subdir/out-of-repo-link-dir &&\n+\ttest_ln_s_add ../../../ dir/subdir/out-of-repo-link-dir-trailing &&\n+\ttest_ln_s_add ../parent-dir-link dir/subdir/parent-dir-link-to-link &&\n+\techo_without_newline \"$hello_content\" >dir/subdir/ind2 &&\n+\techo_without_newline \"$hello_content\" >dir/ind1 &&\n+\ttest_ln_s_add dir dirlink &&\n+\ttest_ln_s_add dir/subdir subdirlink &&\n+\ttest_ln_s_add subdir/ind2 dir/link-to-child &&\n+\ttest_ln_s_add dir/link-to-child link-to-down-link &&\n+\ttest_ln_s_add dir/.. up-down &&\n+\ttest_ln_s_add dir/../ up-down-trailing &&\n+\ttest_ln_s_add dir/../morx up-down-file &&\n+\ttest_ln_s_add dir/../../morx up-up-down-file &&\n+\ttest_ln_s_add subdirlink/../../morx up-two-down-file &&\n+\ttest_ln_s_add loop1 loop2 &&\n+\ttest_ln_s_add loop2 loop1 &&\n+\tgit add morx dir/subdir/ind2 dir/ind1 &&\n+\tgit commit -am \"test\"\n+\techo $hello_sha1 blob $hello_size >found\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for non-links' '\n+\techo HEAD:morx | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo HEAD:nope missing >expect &&\n+\techo HEAD:nope | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for in-repo, same-dir links' '\n+\techo HEAD:same-dir-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for broken in-repo, same-dir links' '\n+\techo HEAD:broken-same-dir-link missing >expect &&\n+\techo HEAD:broken-same-dir-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for same-dir links-to-links' '\n+\techo HEAD:link-to-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for parent-dir links' '\n+\techo HEAD:dir/parent-dir-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo HEAD:dir/parent-dir-link/nope missing >expect &&\n+\techo HEAD:dir/parent-dir-link/nope | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for .. links' '\n+\techo HEAD:dir/link-dir/nope missing >expect &&\n+\techo HEAD:dir/link-dir/nope | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:dir/link-dir/morx | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo HEAD:dir/broken-link-in-dir missing >expect &&\n+\techo HEAD:dir/broken-link-in-dir | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for ../.. links' '\n+\techo HEAD:dir/subdir/grandparent-dir-link/nope missing >expect &&\n+\techo HEAD:dir/subdir/grandparent-dir-link/nope | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:dir/subdir/grandparent-dir-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo HEAD:dir/subdir/parent-dir-link-to-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir/ links' '\n+\techo HEAD:dirlink/morx missing >expect &&\n+\techo HEAD:dirlink/morx | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo $hello_sha1 blob $hello_size >expect &&\n+\techo HEAD:dirlink/ind1 | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir/subdir links' '\n+\techo HEAD:subdirlink/morx missing >expect &&\n+\techo HEAD:subdirlink/morx | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:subdirlink/ind2 | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir ->subdir links' '\n+\techo HEAD:dir/link-to-child/morx missing >expect &&\n+\techo HEAD:dir/link-to-child/morx | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:dir/link-to-child | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo HEAD:link-to-down-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks' '\n+\techo symlink 8 >expect &&\n+\techo ../fleem >>expect &&\n+\techo HEAD:out-of-repo-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo symlink 2 >expect &&\n+\techo .. >>expect &&\n+\techo HEAD:out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks in dirs' '\n+\techo symlink 9 >expect &&\n+\techo ../escape >>expect &&\n+\techo HEAD:dir/out-of-repo-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo symlink 2 >expect &&\n+\techo .. >>expect &&\n+\techo HEAD:dir/out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-of-repo symlinks in subdirs' '\n+\techo symlink 15 >expect &&\n+\techo ../great-escape >>expect &&\n+\techo HEAD:dir/subdir/out-of-repo-link | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo symlink 2 >expect &&\n+\techo .. >>expect &&\n+\techo HEAD:dir/subdir/out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo symlink 3 >expect &&\n+\techo ../ >>expect &&\n+\techo HEAD:dir/subdir/out-of-repo-link-dir-trailing | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlinks works for symlinks with internal ..' '\n+\techo HEAD: | git cat-file --batch-check >expect &&\n+\techo HEAD:up-down | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:up-down-trailing | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:up-down-file | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual &&\n+\techo symlink 7 >expect &&\n+\techo ../morx >>expect &&\n+\techo HEAD:up-up-down-file | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual &&\n+\techo HEAD:up-two-down-file | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp found actual\n+'\n+\n+test_expect_success 'git cat-file --batch-check --follow-symlink breaks loops' '\n+\techo HEAD:loop1 missing >expect &&\n+\techo HEAD:loop1 | git cat-file --batch-check --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch --follow-symlink returns correct sha and mode' '\n+\techo HEAD:morx | git cat-file --batch >expect &&\n+\techo HEAD:morx | git cat-file --batch --follow-symlinks >actual &&\n+\ttest_cmp expect actual\n+'\n test_done\n-- \n2.0.4.315.gad8727a-twtrsrc\n"},{"id":"261055","messageId":"5552385D.1050303@kdbg.org","threadId":"39305","inReplyTo":"1431384645-17276-2-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 1/3] tree-walk: learn get_tree_entry_follow_symlinks","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-05-12T17:29:01Z","receivedAt":"2015-05-12T17:29:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.05.2015 um 00:50 schrieb dturner@twopensource.com:\n> +\t\t} else if (S_ISLNK(*mode)) {\n> +\t\t\t/* Follow a symlink */\n> +\t\t\tsize_t link_len, len;\n> +\t\t\tchar *contents, *contents_start;\n> +\t\t\tstruct dir_state *parent;\n> +\t\t\tenum object_type type;\n> +\n> +\t\t\tif (follows_remaining-- == 0)\n> +\t\t\t\t/* Too many symlinks followed */\n> +\t\t\t\tgoto done;\n> +\n> +\t\t\tcontents = read_sha1_file(current_tree_sha1, &type,\n> +\t\t\t\t\t\t  &link_len);\n\nIn this line, I get:\n\ntree-walk.c: In function 'get_tree_entry_follow_symlinks':\ntree-walk.c:637: warning: passing argument 3 of 'read_sha1_file' from \nincompatible pointer type\ncache.h:885: note: expected 'long unsigned int *' but argument is of \ntype 'size_t *'\n\n-- Hannes\n"},{"id":"261057","messageId":"5552398C.20700@kdbg.org","threadId":"39305","inReplyTo":"1431384645-17276-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-05-12T17:34:04Z","receivedAt":"2015-05-12T17:34:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.05.2015 um 00:50 schrieb dturner@twopensource.com:\n> +# Tests for git cat-file --follow-symlinks\n> +test_expect_success 'prep for symlink tests' '\n> +\techo_without_newline \"$hello_content\" >morx &&\n> +\ttest_ln_s_add morx same-dir-link &&\n> +\ttest_ln_s_add ../fleem out-of-repo-link &&\n> +\ttest_ln_s_add .. out-of-repo-link-dir &&\n> +\ttest_ln_s_add same-dir-link link-to-link &&\n> +\ttest_ln_s_add nope broken-same-dir-link &&\n> +\tmkdir dir &&\n> +\ttest_ln_s_add ../morx dir/parent-dir-link &&\n> +\ttest_ln_s_add .. dir/link-dir &&\n> +\ttest_ln_s_add ../../escape dir/out-of-repo-link &&\n> +\ttest_ln_s_add ../.. dir/out-of-repo-link-dir &&\n> +\ttest_ln_s_add nope dir/broken-link-in-dir &&\n> +\tmkdir dir/subdir &&\n> +\ttest_ln_s_add ../../morx dir/subdir/grandparent-dir-link &&\n> +\ttest_ln_s_add ../../../great-escape dir/subdir/out-of-repo-link &&\n> +\ttest_ln_s_add ../../.. dir/subdir/out-of-repo-link-dir &&\n> +\ttest_ln_s_add ../../../ dir/subdir/out-of-repo-link-dir-trailing &&\n> +\ttest_ln_s_add ../parent-dir-link dir/subdir/parent-dir-link-to-link &&\n> +\techo_without_newline \"$hello_content\" >dir/subdir/ind2 &&\n> +\techo_without_newline \"$hello_content\" >dir/ind1 &&\n> +\ttest_ln_s_add dir dirlink &&\n> +\ttest_ln_s_add dir/subdir subdirlink &&\n> +\ttest_ln_s_add subdir/ind2 dir/link-to-child &&\n> +\ttest_ln_s_add dir/link-to-child link-to-down-link &&\n> +\ttest_ln_s_add dir/.. up-down &&\n> +\ttest_ln_s_add dir/../ up-down-trailing &&\n> +\ttest_ln_s_add dir/../morx up-down-file &&\n> +\ttest_ln_s_add dir/../../morx up-up-down-file &&\n> +\ttest_ln_s_add subdirlink/../../morx up-two-down-file &&\n> +\ttest_ln_s_add loop1 loop2 &&\n> +\ttest_ln_s_add loop2 loop1 &&\n> +\tgit add morx dir/subdir/ind2 dir/ind1 &&\n> +\tgit commit -am \"test\"\n\nBroken && chain.\n\n> +\techo $hello_sha1 blob $hello_size >found\n> +'\n\n-- Hannes\n"},{"id":"261062","messageId":"xmqqoalpzn3s.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431384645-17276-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T18:07:03Z","receivedAt":"2015-05-12T18:07:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"dturner@twopensource.com writes:\n\n> +--follow-symlinks::\n> +\tFollow symlinks inside the repository when requesting objects\n> +\twith extended SHA-1 expressions of the form tree-ish:path-in-tree.\n> +\tInstead of providing output about the link itself, provide output\n> +\tabout the linked-to object.  This option requires --batch or\n> +\t--batch-check.  In the event of a symlink loop (or more than\n> +\t40 symlinks in a symlink resolution chain), the file will be\n> +\ttreated as missing.  If a symlink points outside the tree-ish\n> +\t(e.g. a link to /foo or a root-level link to ../foo), the\n> +\tportion of the link which is outside the tree will be printed.\n> +\tFollow-symlinks will be silently turned off if <object>\n> +\tspecifies an object in the index rather than one in the object\n> +\tdatabase.\n> +\n> +\tFor example, consider the a git repository containing:\n> +\tf: a file containing \"hello\\n\"\n> +\tlink: a symlink to f\n> +\tdir/link: a symlink to ../f\n> +\tplink: a symlink to ../f\n> +\talink: a symlink to /etc/passwd\n> +\n> +\techo HEAD:f |git cat-file --batch --follow-symlinks would print\n> +\tce013625030ba8dba906f756967f9e9ca394464a blob 6\n> +\n> +\techo HEAD:link |git cat-file --batch --follow-symlinks would print\n> +\tthe same thing, as would HEAD:dir/link.\n> +\tWithout follow-symlinks, these would print data about the\n> +\tsymlink itself.  In the case of HEAD:link, you would see\n> +\t4d1ae35ba2c8ec712fa2a379db44ad639ca277bd blob 1\n> +\n> +\tBoth plink and alink point outside the tree, so they would\n> +\trespectively print:\n> +\tsymlink 4\n> +\t../f\n> +\n> +\tsymlink 11\n> +\t/etc/passwd\n> +\n> +\n> +\n\nA few points I noticed:\n\n * It is not clear that this is (currently) only for --batch and\n   --batch-check until you read four lines into the description.\n\n   Perhaps start the description like this instead?\n\n   --follow-symlinks::\n           When answering `--batch` or `--batch-check` request,\n           follow symlinks inside the repository when requesting objects\n           with extended SHA-1 expressions of the form tree-ish:path-in-tree.\n\n   Also I'd lose the \"This option requires ...\" sentence in the middle\n   (I'll come back to the reason why later).\n\n * Is it fundamental that this is only for --batch family, or is it\n   just lack of need by the current implementor and implementation?\n   \"git cat-file --follow-symlinks blob :RelNotes\" does not sound\n   a nonsense request to me.\n\n * I am not sure if HEAD:link that points at HEAD:link should be\n   reported as \"missing\".  It may be better to report the original\n   without any dereferencing just like a link that points at outside\n   the tree? i.e. \"symlink 4 LF link\".\n\n * I think \"echo :RelNotes | git cat-file --batch --follow-symlinks\"\n   that does not follow a symlink is a BUG.  Unless there is\n   something fundamental that in-index object should never support\n   this feature, that is.  But I do not think of a good reason\n   why---it feels that this is just the lack of implementation that\n   can be addressed by somebody else in the future who finds the\n   need for the support.\n\nI do not necessarily think the latter three need to be addressed in\nthis 3-patch series, but they should be listed as known bugs, I\nwould think.  That would invite others to fix them and save time for\nusers to file unnecessary bug reports.\n\nSo (now I came back) the last part of the description may want to\nbecome more like this:\n\n\t...\n\tportion of the link which is outside the tree will be printed.\n\n\tThis option (currently) cannot be used unless `--batch` or\n\t`--batch-check` is used.\n\n        Also the option does not (currently) work correctly when an\n\tobject in the index is specified (e.g. `:link` instead of\n\t`HEAD:link`) rather than one in the tree.\n\nWe need to also say something about the \"missing\" vs \"loop\" case, if\nwe choose to leave that part broken.  I'd rather see it fixed, but\nthat is not a very strong preference.\n\nBy the way, the text after your patch would not format well thru\nAsciiDoc.  See attached for a suggested mark-up fix that can be\nsquashed.\n\nThanks.\n\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 9bdfced..3226f3e 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -82,31 +82,43 @@ OPTIONS\n \tFollow-symlinks will be silently turned off if <object>\n \tspecifies an object in the index rather than one in the object\n \tdatabase.\n-\n-\tFor example, consider the a git repository containing:\n++\n+For example, consider a git repository containing:\n++\n+--\n \tf: a file containing \"hello\\n\"\n \tlink: a symlink to f\n \tdir/link: a symlink to ../f\n \tplink: a symlink to ../f\n \talink: a symlink to /etc/passwd\n-\n-\techo HEAD:f |git cat-file --batch --follow-symlinks would print\n+--\n++\n+For a regular file `f`, `echo HEAD:f | git cat-file --batch` would print\n++\n+--\n \tce013625030ba8dba906f756967f9e9ca394464a blob 6\n-\n-\techo HEAD:link |git cat-file --batch --follow-symlinks would print\n-\tthe same thing, as would HEAD:dir/link.\n-\tWithout follow-symlinks, these would print data about the\n-\tsymlink itself.  In the case of HEAD:link, you would see\n+--\n++\n+And `echo HEAD:link | git cat-file --batch --follow-symlinks` would print\n+the same thing, as would `HEAD:dir/link`, as they both point at `HEAD:f`.\n++\n+Without `--follow-symlinks`, these would print data about the\n+symlink itself.  In the case of `HEAD:link`, you would see\n++\n+--\n \t4d1ae35ba2c8ec712fa2a379db44ad639ca277bd blob 1\n-\n-\tBoth plink and alink point outside the tree, so they would\n-\trespectively print:\n+--\n++\n+Both `plink` and `alink` point outside the tree, so they would\n+respectively print:\n++\n+--\n \tsymlink 4\n \t../f\n \n \tsymlink 11\n \t/etc/passwd\n-\n+--\n \n \n OUTPUT\n-- \n2.4.0-363-gef77c54\n"},{"id":"261063","messageId":"1431455779.16652.20.camel@ubuntu","threadId":"39305","inReplyTo":"xmqqoalpzn3s.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-12T18:36:19Z","receivedAt":"2015-05-12T18:36:19Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-05-12 at 11:07 -0700, Junio C Hamano wrote:\n> > +\tBoth plink and alink point outside the tree, so they would\n> > +\trespectively print:\n> > +\tsymlink 4\n> > +\t../f\n> > +\n> > +\tsymlink 11\n> > +\t/etc/passwd\n> > +\n> > +\n> > +\n> \n> A few points I noticed:\n> \n>  * It is not clear that this is (currently) only for --batch and\n>    --batch-check until you read four lines into the description.\n> \n>    Perhaps start the description like this instead?\n> \n>    --follow-symlinks::\n>            When answering `--batch` or `--batch-check` request,\n>            follow symlinks inside the repository when requesting objects\n>            with extended SHA-1 expressions of the form tree-ish:path-in-tree.\n\nWill rearrange.\n\n>    Also I'd lose the \"This option requires ...\" sentence in the middle\n>    (I'll come back to the reason why later).\n> \n>  * Is it fundamental that this is only for --batch family, or is it\n>    just lack of need by the current implementor and implementation?\n>    \"git cat-file --follow-symlinks blob :RelNotes\" does not sound\n>    a nonsense request to me.\n\nThe reason that --follow-symlinks doesn't work for non-batch requests is\nthat it is impossible to distinguish out-of-tree symlinks from valid\noutput in non-batch output. I will add text explaining this. \n\n>  * I am not sure if HEAD:link that points at HEAD:link should be\n>    reported as \"missing\".  It may be better to report the original\n>    without any dereferencing just like a link that points at outside\n>    the tree? i.e. \"symlink 4 LF link\".\n\nUnfortunately, a symlink loop might include relative symlinks\n(e.g. ../a).  If we return a relative symlink, the user will\nnot be able to distinguish it from a non-loop, out-of-tree symlink.  So\nI think we may not return symlink 4 LF ../a for these cases.  \n\nWe could, I guess, have a separate output like loop <size> LF link\n<LF>\", but, unless we always save and output the first link in the\nchain, we won't know what any link is relative to.  Since reasonable\npeople do not create symlink loops, and since there are other mechanisms\nfor symlink loop debugging (e.g. plain cat-file), I think it is OK not\nto put special effort into handling loops.\n\n>  * I think \"echo :RelNotes | git cat-file --batch --follow-symlinks\"\n>    that does not follow a symlink is a BUG.  Unless there is\n>    something fundamental that in-index object should never support\n>    this feature, that is.  But I do not think of a good reason\n>    why---it feels that this is just the lack of implementation that\n>    can be addressed by somebody else in the future who finds the\n>    need for the support.\n\nYes, this should definitely be addressed in the future.  I didn't see a\nstraightforward way to generalize this code to also address the index,\nso a new version of this function would have to be written.  That's why\nI didn't add that feature yet.  The lack of it is definitely a bug,\nthough.  \n\n>         Also the option does not (currently) work correctly when an\n> \tobject in the index is specified (e.g. `:link` instead of\n> \t`HEAD:link`) rather than one in the tree.\n> \n> We need to also say something about the \"missing\" vs \"loop\" case, if\n> we choose to leave that part broken.  I'd rather see it fixed, but\n> that is not a very strong preference.\n\nWill add an example.\n\n> By the way, the text after your patch would not format well thru\n> AsciiDoc.  See attached for a suggested mark-up fix that can be\n> squashed.\n\nI'll squash that in when I re-roll.  Thanks for the formatting.\n"},{"id":"261064","messageId":"xmqqk2wdzlfm.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431455779.16652.20.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T18:43:09Z","receivedAt":"2015-05-12T18:43:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n>> We need to also say something about the \"missing\" vs \"loop\" case, if\n>> we choose to leave that part broken.  I'd rather see it fixed, but\n>> that is not a very strong preference.\n>\n> Will add an example.\n\nI do not think we need an example.  By \"also say\", I meant in\naddition to \"This and that does not currently work\", we also need to\nsay that loops do not work well.  In other words, it is enough to\njust mention that it is a current limitation (or a bug, whichever we\nchoose to call) that loops are reported as missing.\n\nThanks.\n"},{"id":"261066","messageId":"1431456922.16652.26.camel@ubuntu","threadId":"39305","inReplyTo":"xmqqk2wdzlfm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-12T18:55:22Z","receivedAt":"2015-05-12T18:55:22Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-05-12 at 11:43 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> >> We need to also say something about the \"missing\" vs \"loop\" case, if\n> >> we choose to leave that part broken.  I'd rather see it fixed, but\n> >> that is not a very strong preference.\n> >\n> > Will add an example.\n> \n> I do not think we need an example.  By \"also say\", I meant in\n> addition to \"This and that does not currently work\", we also need to\n> say that loops do not work well.  In other words, it is enough to\n> just mention that it is a current limitation (or a bug, whichever we\n> choose to call) that loops are reported as missing.\n\nThe version of the patch that we are commenting on contained the text: \n> +     --batch-check.  In the event of a symlink loop (or more than\n> +     40 symlinks in a symlink resolution chain), the file will be\n> +     treated as missing.  If a symlink points outside the tree-ish\n\nIs that sufficient?  \n\nActually, we could simply have a separate output for broken links.\nInstead of [original path] SP missing, [original path] SP loop.\n\nI would probably implement this with a special error return code\n(SYMLINK_LOOP=-2 instead of the usual -1). \n\nDoes that seem reasonable to you?\n"},{"id":"261072","messageId":"xmqqlhgtftw0.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431456922.16652.26.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T20:00:47Z","receivedAt":"2015-05-12T20:00:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Tue, 2015-05-12 at 11:43 -0700, Junio C Hamano wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>> \n>> >> We need to also say something about the \"missing\" vs \"loop\" case, if\n>> >> we choose to leave that part broken.  I'd rather see it fixed, but\n>> >> that is not a very strong preference.\n>> >\n>> > Will add an example.\n>> \n>> I do not think we need an example.  By \"also say\", I meant in\n>> addition to \"This and that does not currently work\", we also need to\n>> say that loops do not work well.  In other words, it is enough to\n>> just mention that it is a current limitation (or a bug, whichever we\n>> choose to call) that loops are reported as missing.\n>\n> The version of the patch that we are commenting on contained the text: \n>> +     --batch-check.  In the event of a symlink loop (or more than\n>> +     40 symlinks in a symlink resolution chain), the file will be\n>> +     treated as missing.  If a symlink points outside the tree-ish\n>\n> Is that sufficient?  \n\nNot really, as I think people would say \"treated as missing\" is a\nbug, and it would be better to mark it as \"currently does not work\"\nin the sense that \"you cannot use this feature to tell HEAD:link\nthat is not in HEAD and HEAD:link that is a symbolic link that\npoints to itself apart\".\n\n> Actually, we could simply have a separate output for broken links.\n> Instead of [original path] SP missing, [original path] SP loop.\n\nHmm, I do not quite see where this resistance against keeping the\noriginal request in order to say \"You gave HEAD:link to me, but that\nis a symbolic link that leads to 'link'\" comes from.  After all,\nwouldn't that be more consistent with what you already show for a\nlink that cannot be resolved, i.e. \"You gave HEAD:link to me, that\nis a link that points at 'nosuch'\" when I do this:\n\n    $ ln -s nosuch link\n    $ git add link\n    $ echo \"$(git write-tree):link\" |\n      git cat-file --batch --follow-symlinks\n\nAhh, that would also give us \"missing\", so in that sense you are\nbeing consistent.\n\nBut I do not think that consistency is useful.  Showing just\n\"missing\" instead is losing information and that is what bothers me.\n\nShowing \"symlink 6 nosuch\" to this \"link points at a target that\nwould be in-tree but there is no such object in the tree\" symbolic\nlink instead of \"missing\" would make it more useful, and I do not\noffhand think of a downside, but maybe I am missing something.\n\nFor a link that points outside, the code already gives\n\n    $ ln -s ../outside outlink\n    $ git add outlink\n    $ echo \"$(git write-tree):outlink\" |\n      git cat-file --batch --follow-symlinks\n\n\"symlink ../outside\", so the script reading from the batch output\nalready has to be prepared to handle \"symlink\" and understand it as\nsaying \"the link does not point an object that is inside the tree\".\n"},{"id":"261073","messageId":"xmqqh9rhftlh.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431455779.16652.20.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T20:07:06Z","receivedAt":"2015-05-12T20:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n>>  * I am not sure if HEAD:link that points at HEAD:link should be\n>>    reported as \"missing\".  It may be better to report the original\n>>    without any dereferencing just like a link that points at outside\n>>    the tree? i.e. \"symlink 4 LF link\".\n>\n> Unfortunately, a symlink loop might include relative symlinks\n> (e.g. ../a).  If we return a relative symlink, the user will\n> not be able to distinguish it from a non-loop, out-of-tree symlink.  So\n> I think we may not return symlink 4 LF ../a for these cases.  \n\nI do not follow.  Let's start from a shared example.\n\n    HEAD:sub/link is a symbolic link whose value is ../nextlink\n    HEAD:nextlink is a symbolic link whose value is sub/link\n\nThat's a loop.  Now, I think what I am sugesting is\n\n\t$ git cat-file --batch-check --follow-symlinks <<\\EOF\n\tHEAD:sub/link\n        HEAD:nextlink\n\tEOF\n\tsymlink ../nextlink\n        symlink sub/link\n\nIf you asked about sub/link and then got ../nextlink back, then\nisn't it clear for the reading script that it is about nextlink\nat the top-level?  Why can't it tell it from out-of-tree link?\n\n\n\t\n"},{"id":"261074","messageId":"xmqqd225fsw8.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"xmqqlhgtftw0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T20:22:15Z","receivedAt":"2015-05-12T20:22:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Ahh, that would also give us \"missing\", so in that sense you are\n> being consistent.\n>\n> But I do not think that consistency is useful.  Showing just\n> \"missing\" instead is losing information and that is what bothers me.\n>\n> Showing \"symlink 6 nosuch\" to this \"link points at a target that\n> would be in-tree but there is no such object in the tree\" symbolic\n> link instead of \"missing\" would make it more useful, and I do not\n> offhand think of a downside, but maybe I am missing something.\n>\n> For a link that points outside, the code already gives\n>\n>     $ ln -s ../outside outlink\n>     $ git add outlink\n>     $ echo \"$(git write-tree):outlink\" |\n>       git cat-file --batch --follow-symlinks\n>\n> \"symlink ../outside\", so the script reading from the batch output\n> already has to be prepared to handle \"symlink\" and understand it as\n> saying \"the link does not point an object that is inside the tree\".\n\nHaving said all that, I think we can make readers' life easier by\nclassifying various reasons why --follow-symlinks cannot pretend\nas if the in-tree pointee were given as its input and signal it\ndifferently:\n\n * A link points outside the original tree (or the index, once we\n   support \"cat-file :RelNotes\").\n\n   I think your \"symlink <size> LF <target> LF\" already does this.\n\n * A link points (directly or indirectly) at itself.\n\n   This would be your \"loop <size> LF <target> LF\", which I think is\n   good.\n\n * A link does not step outside the original tree, but the pointee\n   does not exist in the original tree.\n\n   I think you currently say \"<object name for HEAD:link> missing\",\n   but I do not think it is useful.  That object does exist, but it\n   merely cannot be dereferenced.  Perhaps introducing a new one,\n   e.g. \"dangling <size> LF <target> LF\" similar to symlink/loop,\n   would help the reader better?\n\nAre there other cases?  The only other case I think of is when the\nlink resolves cleanly inside the tree, which you already handle.\n\nThanks.\n"},{"id":"261087","messageId":"1431466646.16652.32.camel@ubuntu","threadId":"39305","inReplyTo":"xmqqh9rhftlh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-12T21:37:26Z","receivedAt":"2015-05-12T21:37:26Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-05-12 at 13:07 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> >>  * I am not sure if HEAD:link that points at HEAD:link should be\n> >>    reported as \"missing\".  It may be better to report the original\n> >>    without any dereferencing just like a link that points at outside\n> >>    the tree? i.e. \"symlink 4 LF link\".\n> >\n> > Unfortunately, a symlink loop might include relative symlinks\n> > (e.g. ../a).  If we return a relative symlink, the user will\n> > not be able to distinguish it from a non-loop, out-of-tree symlink.  So\n> > I think we may not return symlink 4 LF ../a for these cases.  \n> \n> I do not follow.  Let's start from a shared example.\n> \n>     HEAD:sub/link is a symbolic link whose value is ../nextlink\n>     HEAD:nextlink is a symbolic link whose value is sub/link\n> \n> That's a loop.  Now, I think what I am sugesting is\n> \n> \t$ git cat-file --batch-check --follow-symlinks <<\\EOF\n> \tHEAD:sub/link\n>         HEAD:nextlink\n> \tEOF\n> \tsymlink ../nextlink\n>         symlink sub/link\n> \n> If you asked about sub/link and then got ../nextlink back, then\n> isn't it clear for the reading script that it is about nextlink\n> at the top-level?  Why can't it tell it from out-of-tree link?\n\nBecause maybe sub/link was actually a link to ../../nextlink.  Or maybe\nsub/link was actually a link to ../outlink, and ../outlink was a link\nto ../nextlink. \n\nBut I think I'm OK with implementing your proposed solution[1] of\ncategorizing all of the possible cases.\n\n\n[1] http://www.spinics.net/lists/git/msg250893.html\nxmqqd225fsw8.fsf@gitster.dls.corp.google.com \n"},{"id":"261090","messageId":"xmqqiobxealk.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431466646.16652.32.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T21:42:47Z","receivedAt":"2015-05-12T21:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Tue, 2015-05-12 at 13:07 -0700, Junio C Hamano wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>> \n>> >>  * I am not sure if HEAD:link that points at HEAD:link should be\n>> >>    reported as \"missing\".  It may be better to report the original\n>> >>    without any dereferencing just like a link that points at outside\n>> >>    the tree? i.e. \"symlink 4 LF link\".\n>> >\n>> > Unfortunately, a symlink loop might include relative symlinks\n>> > (e.g. ../a).  If we return a relative symlink, the user will\n>> > not be able to distinguish it from a non-loop, out-of-tree symlink.  So\n>> > I think we may not return symlink 4 LF ../a for these cases.  \n>> \n>> I do not follow.  Let's start from a shared example.\n>> \n>>     HEAD:sub/link is a symbolic link whose value is ../nextlink\n>>     HEAD:nextlink is a symbolic link whose value is sub/link\n>> \n>> That's a loop.  Now, I think what I am sugesting is\n>> \n>> \t$ git cat-file --batch-check --follow-symlinks <<\\EOF\n>> \tHEAD:sub/link\n>>         HEAD:nextlink\n>> \tEOF\n>> \tsymlink ../nextlink\n>>         symlink sub/link\n>> \n>> If you asked about sub/link and then got ../nextlink back, then\n>> isn't it clear for the reading script that it is about nextlink\n>> at the top-level?  Why can't it tell it from out-of-tree link?\n>\n> Because maybe sub/link was actually a link to ../../nextlink.\n\nThen the output would have said \"symlink ../../nextlink\" and you can\ntell these two cases apart, no?  Same for the other case.\n\n> But I think I'm OK with implementing your proposed solution[1] of\n> categorizing all of the possible cases.\n\nI think that is the cleanest I can come up with offhand, and would\ngive the easiest input to scripters.  At least I think so for now,\nbut others may have even better ideas ;-)\n\nThanks.\n"},{"id":"261093","messageId":"1431467609.16652.35.camel@ubuntu","threadId":"39305","inReplyTo":"xmqqiobxealk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-12T21:53:29Z","receivedAt":"2015-05-12T21:53:29Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-05-12 at 14:42 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > On Tue, 2015-05-12 at 13:07 -0700, Junio C Hamano wrote:\n> >> David Turner <dturner@twopensource.com> writes:\n> >> \n> >> >>  * I am not sure if HEAD:link that points at HEAD:link should be\n> >> >>    reported as \"missing\".  It may be better to report the original\n> >> >>    without any dereferencing just like a link that points at outside\n> >> >>    the tree? i.e. \"symlink 4 LF link\".\n> >> >\n> >> > Unfortunately, a symlink loop might include relative symlinks\n> >> > (e.g. ../a).  If we return a relative symlink, the user will\n> >> > not be able to distinguish it from a non-loop, out-of-tree symlink.  So\n> >> > I think we may not return symlink 4 LF ../a for these cases.  \n> >> \n> >> I do not follow.  Let's start from a shared example.\n> >> \n> >>     HEAD:sub/link is a symbolic link whose value is ../nextlink\n> >>     HEAD:nextlink is a symbolic link whose value is sub/link\n> >> \n> >> That's a loop.  Now, I think what I am sugesting is\n> >> \n> >> \t$ git cat-file --batch-check --follow-symlinks <<\\EOF\n> >> \tHEAD:sub/link\n> >>         HEAD:nextlink\n> >> \tEOF\n> >> \tsymlink ../nextlink\n> >>         symlink sub/link\n> >> \n> >> If you asked about sub/link and then got ../nextlink back, then\n> >> isn't it clear for the reading script that it is about nextlink\n> >> at the top-level?  Why can't it tell it from out-of-tree link?\n> >\n> > Because maybe sub/link was actually a link to ../../nextlink.\n> \n> Then the output would have said \"symlink ../../nextlink\" and you can\n> tell these two cases apart, no?  Same for the other case.\n\nNo.  Successfully resolved out-of-tree symlinks must be relative to the\ntree root.  Jeff King and I discussed this in the original RFC:\notherwise, after a chain of symlinks which might move you all over the\ntree, you have no idea what \"../\" is relative to.\n"},{"id":"261096","messageId":"1431470204.16652.52.camel@ubuntu","threadId":"39305","inReplyTo":"xmqqd225fsw8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-05-12T22:36:44Z","receivedAt":"2015-05-12T22:36:44Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-05-12 at 13:22 -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Ahh, that would also give us \"missing\", so in that sense you are\n> > being consistent.\n> >\n> > But I do not think that consistency is useful.  Showing just\n> > \"missing\" instead is losing information and that is what bothers me.\n> >\n> > Showing \"symlink 6 nosuch\" to this \"link points at a target that\n> > would be in-tree but there is no such object in the tree\" symbolic\n> > link instead of \"missing\" would make it more useful, and I do not\n> > offhand think of a downside, but maybe I am missing something.\n> >\n> > For a link that points outside, the code already gives\n> >\n> >     $ ln -s ../outside outlink\n> >     $ git add outlink\n> >     $ echo \"$(git write-tree):outlink\" |\n> >       git cat-file --batch --follow-symlinks\n> >\n> > \"symlink ../outside\", so the script reading from the batch output\n> > already has to be prepared to handle \"symlink\" and understand it as\n> > saying \"the link does not point an object that is inside the tree\".\n> \n> Having said all that, I think we can make readers' life easier by\n> classifying various reasons why --follow-symlinks cannot pretend\n> as if the in-tree pointee were given as its input and signal it\n> differently:\n> \n>  * A link points outside the original tree (or the index, once we\n>    support \"cat-file :RelNotes\").\n> \n>    I think your \"symlink <size> LF <target> LF\" already does this.\n> \n>  * A link points (directly or indirectly) at itself.\n> \n>    This would be your \"loop <size> LF <target> LF\", which I think is\n>    good.\n> \n>  * A link does not step outside the original tree, but the pointee\n>    does not exist in the original tree.\n> \n>    I think you currently say \"<object name for HEAD:link> missing\",\n>    but I do not think it is useful.  That object does exist, but it\n>    merely cannot be dereferenced.  Perhaps introducing a new one,\n>    e.g. \"dangling <size> LF <target> LF\" similar to symlink/loop,\n>    would help the reader better?\n> \n> Are there other cases?  The only other case I think of is when the\n> link resolves cleanly inside the tree, which you already handle.\n> \n> Thanks.\nOn Tue, 2015-05-12 at 13:22 -0700, Junio C Hamano wrote:\nJunio C Hamano <gitster@pobox.com> writes:\n> \n> > Ahh, that would also give us \"missing\", so in that sense you are\n> > being consistent.\n> >\n> > But I do not think that consistency is useful.  Showing just\n> > \"missing\" instead is losing information and that is what bothers me.\n> >\n> > Showing \"symlink 6 nosuch\" to this \"link points at a target that\n> > would be in-tree but there is no such object in the tree\" symbolic\n> > link instead of \"missing\" would make it more useful, and I do not\n> > offhand think of a downside, but maybe I am missing something.\n> >\n> > For a link that points outside, the code already gives\n> >\n> >     $ ln -s ../outside outlink\n> >     $ git add outlink\n> >     $ echo \"$(git write-tree):outlink\" |\n> >       git cat-file --batch --follow-symlinks\n> >\n> > \"symlink ../outside\", so the script reading from the batch output\n> > already has to be prepared to handle \"symlink\" and understand it as\n> > saying \"the link does not point an object that is inside the tree\".\n> \n> Having said all that, I think we can make readers' life easier by\n> classifying various reasons why --follow-symlinks cannot pretend\n> as if the in-tree pointee were given as its input and signal it\n> differently:\n> \n>  * A link points outside the original tree (or the index, once we\n>    support \"cat-file :RelNotes\").\n> \n>    I think your \"symlink <size> LF <target> LF\" already does this.\n> \n>  * A link points (directly or indirectly) at itself.\n> \n>    This would be your \"loop <size> LF <target> LF\", which I think is\n>    good.\n> \n>  * A link does not step outside the original tree, but the pointee\n>    does not exist in the original tree.\n> \n>    I think you currently say \"<object name for HEAD:link> missing\",\n>    but I do not think it is useful.  That object does exist, but it\n>    merely cannot be dereferenced.  Perhaps introducing a new one,\n>    e.g. \"dangling <size> LF <target> LF\" similar to symlink/loop,\n>    would help the reader better?\n> \n> Are there other cases?  The only other case I think of is when the\n> link resolves cleanly inside the tree, which you already handle.\n\nWhile updating the tests, I noticed another two cases:\n1. HEAD:broken-link/file\n\nI am inclined to describe this as \"dangling\" as well.  (It is not useful\nto tell users that \"file\" is the remaining bit to be resolved, because\ndue to chains of symlinks, users have no idea what file would be\nrelative to).  I think the filesystem returns ENOENT in the equivalent\ncase.\n\n2. HEAD:link-to-file/file\n\nThis should be \"notdir\", I think, in that it is a distinct way of\nfailing that the user might care to know. The filesystem returns ENOTDIR\nin the equivalent case.\n\nIn addition, I would like to have the format for the dangling, loop, and\nnotdir cases match the missing case.  In other words, \"HEAD:link\nmissing\", \"HEAD:link dangling\", etc.  Users already need to parse the\nmissing case, so we might as well make the others match.  It's true that\nthis parsing is impossible in the case of filenames containing newlines,\nbut git cat-file --batch cannot accept those filenames as input anyway\n(presumably, a -0 flag would be needed, if anyone actually used such\nfilenames, and at that point we could make the -0 output unambiguously\nparseable).\n"},{"id":"261097","messageId":"xmqq617xe6x8.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431470204.16652.52.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T23:02:11Z","receivedAt":"2015-05-12T23:02:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Tue, 2015-05-12 at 13:22 -0700, Junio C Hamano wrote:\n>\n>> Are there other cases?  The only other case I think of is when the\n>> link resolves cleanly inside the tree, which you already handle.\n\nFunny double-inclusion ;-)\n\n> While updating the tests, I noticed another two cases:\n> 1. HEAD:broken-link/file\n>\n> I am inclined to describe this as \"dangling\" as well.  (It is not useful\n> to tell users that \"file\" is the remaining bit to be resolved, because\n> due to chains of symlinks, users have no idea what file would be\n> relative to).  I think the filesystem returns ENOENT in the equivalent\n> case.\n>\n> 2. HEAD:link-to-file/file\n>\n> This should be \"notdir\", I think, in that it is a distinct way of\n> failing that the user might care to know. The filesystem returns ENOTDIR\n> in the equivalent case.\n\nThanks.\n\n> In addition, I would like to have the format for the dangling, loop, and\n> notdir cases match the missing case.  In other words, \"HEAD:link\n> missing\", \"HEAD:link dangling\", etc.  Users already need to parse the\n> missing case, so we might as well make the others match.\n\nInteresting to see our opinions differ here, especially because the\nprevious suggestion was coming from \"users already need to parse the\n'symlink' case, so we might as well make the others match\" ;-).\n\nBetween 'missing' and 'symlink', the latter is richer in information\nand easier to parse, I would have thought.\n"},{"id":"261098","messageId":"xmqq1tile6pe.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"xmqq617xe6x8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T23:06:53Z","receivedAt":"2015-05-12T23:06:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Turner <dturner@twopensource.com> writes:\n>\n>> In addition, I would like to have the format for the dangling, loop, and\n>> notdir cases match the missing case.  In other words, \"HEAD:link\n>> missing\", \"HEAD:link dangling\", etc.  Users already need to parse the\n>> missing case, so we might as well make the others match.\n>\n> Interesting to see our opinions differ here, especially because the\n> previous suggestion was coming from \"users already need to parse the\n> 'symlink' case, so we might as well make the others match\" ;-).\n>\n> Between 'missing' and 'symlink', the latter is richer in information\n> and easier to parse, I would have thought.\n\nForgot to conclude the paragraph with final sentence.\n\n... But I do not deeply care either way.\n\nThanks.\n"},{"id":"261273","messageId":"xmqqiobukjow.fsf@gitster.dls.corp.google.com","threadId":"39305","inReplyTo":"1431455779.16652.20.camel@ubuntu","subject":"Re: [PATCH v5 3/3] cat-file: add --follow-symlinks to --batch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-14T20:06:39Z","receivedAt":"2015-05-14T20:06:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Tue, 2015-05-12 at 11:07 -0700, Junio C Hamano wrote:\n>\n>>  * Is it fundamental that this is only for --batch family, or is it\n>>    just lack of need by the current implementor and implementation?\n>>    \"git cat-file --follow-symlinks blob :RelNotes\" does not sound\n>>    a nonsense request to me.\n>\n> The reason that --follow-symlinks doesn't work for non-batch requests is\n> that it is impossible to distinguish out-of-tree symlinks from valid\n> output in non-batch output. I will add text explaining this. \n\nActually, I do not think that is a valid reason not to consider\nsupporting \"git cat-file --follow-symlinks blob :RelNotes\" in future\nversions of Git.  \"--batch\" and \"--batch-check\" needs to continue,\nso it needs to give diagnosis in-line in its output and let the\ndriving script continue talking with it, but a single-shot request\n\"git cat-file --follow-symlinks blob :RelNotes\" can signal that\nthe link does not resolve in-tree (or in-index) by erroring out.\n\nSo I still view it as \"we currently do not need it outside --batch;\nif somebody wants to, feel free to add it to non-batch mode\".  It\ndoes not have to be called a bug but I would say it is a missing\nfeature.  And not handling an in-index path e.g. :RelNotes falls\ninto the same category, I would say.\n\nJust to avoid misunderstanding, I still think this series (at its\n9th iteration) is good from the point of new feature without filling\nthese two missing features.\n"}]}