{"thread":{"id":"7639","subject":"[PATCH 1/3] Add basic infrastructure to assign attributes to paths","startedAt":"2007-04-13T09:01:34Z","lastAt":"2007-04-16T06:21:10Z","messageCount":19,"participants":["Junio C Hamano","Andy Parkins","Linus Torvalds","Brian Gernhardt","Johannes Schindelin","Tom Prince","Raimund Bauer"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"39276","messageId":"7vr6qod4wh.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":null,"subject":"[PATCH 1/3] Add basic infrastructure to assign attributes to paths","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-13T09:01:34Z","receivedAt":"2007-04-13T09:01:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This adds the basic infrastructure to assign attributes to\npaths, in a way similar to what the exclusion mechanism does\nbased on $GIT_DIR/info/exclude and .gitignore files.\n\nAn attribute is just a simple string that does not contain any\nwhitespace.  They can be specified in $GIT_DIR/info/attributes\nfile, and .gitattributes file in each directory.\n\nEach line in these files defines a pattern matching rule.\nSimilar to the exclusion mechanism, a later match overrides an\nearlier match in the same file, and entries from .gitattributes\nfile in the same directory takes precedence over the ones from\nparent directories.  Lines in $GIT_DIR/info/attributes file are\nused as the lowest precedence default rules.\n\nA line is either a comment (an empty line, or a line that begins\nwith a '#'), or a rule, which is a whitespace separated list of\ntokens.  The first token on the line is a shell glob pattern.\nThe rest are names of attributes, each of which can optionally\nbe prefixed with '!'.  Such a line means \"if a path matches this\nglob, this attribute is set (or unset -- if the attribute name\nis prefixed with '!').  For glob matching, the same \"if the\npattern does not have a slash in it, the basename of the path is\nmatched with fnmatch(3) against the pattern, otherwise, the path\nis matched with the pattern with FNM_PATHNAME\" rule as the\nexclusion mechanism is used.\n\nThis does not define what an attribute means.  Tying an\nattribute to various effects it has on git operation for paths\nthat have it (or doesn't) will be specified separately.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n .gitignore           |    1 +\n Makefile             |    5 +-\n attr.c               |  301 ++++++++++++++++++++++++++++++++++++++++++++++++++\n attr.h               |   16 +++\n builtin-check-attr.c |   31 +++++\n builtin.h            |    1 +\n cache.h              |    1 +\n git.c                |    1 +\n 8 files changed, 355 insertions(+), 2 deletions(-)\n create mode 100644 attr.c\n create mode 100644 attr.h\n create mode 100644 builtin-check-attr.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 9229e91..d96f4f0 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -16,6 +16,7 @@ git-blame\n git-branch\n git-bundle\n git-cat-file\n+git-check-attr\n git-check-ref-format\n git-checkout\n git-checkout-index\ndiff --git a/Makefile b/Makefile\nindex b8e6030..ac89d1b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -283,7 +283,7 @@ LIB_H = \\\n \tdiff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \\\n \trun-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \\\n \ttree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \\\n-\tutf8.h reflog-walk.h patch-ids.h\n+\tutf8.h reflog-walk.h attr.h\n \n DIFF_OBJS = \\\n \tdiff.o diff-lib.o diffcore-break.o diffcore-order.o \\\n@@ -305,7 +305,7 @@ LIB_OBJS = \\\n \twrite_or_die.o trace.o list-objects.o grep.o match-trees.o \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n-\tconvert.o\n+\tconvert.o attr.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\n@@ -316,6 +316,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-branch.o \\\n \tbuiltin-bundle.o \\\n \tbuiltin-cat-file.o \\\n+\tbuiltin-check-attr.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n \tbuiltin-commit-tree.o \\\ndiff --git a/attr.c b/attr.c\nnew file mode 100644\nindex 0000000..bdbc4a3\n--- /dev/null\n+++ b/attr.c\n@@ -0,0 +1,301 @@\n+#include \"cache.h\"\n+#include \"attr.h\"\n+\n+/*\n+ * The basic design decision here is that we are not going to have\n+ * insanely large number of attributes.\n+ *\n+ * This is a randomly chosen prime.\n+ */\n+#define HASHSIZE 257\n+\n+struct git_attr {\n+\tstruct git_attr *next;\n+\tunsigned h;\n+\tchar name[FLEX_ARRAY];\n+};\n+\n+static struct git_attr *(git_attr_hash[HASHSIZE]);\n+\n+static unsigned hash_name(const char *name, int namelen)\n+{\n+\tunsigned val = 0;\n+\tunsigned char c;\n+\n+\twhile (namelen--) {\n+\t\tc = *name++;\n+\t\tval = ((val << 7) | (val >> 22)) ^ c;\n+\t}\n+\treturn val;\n+}\n+\n+struct git_attr *git_attr(const char *name, int len)\n+{\n+\tunsigned hval = hash_name(name, len);\n+\tunsigned pos = hval % HASHSIZE;\n+\tstruct git_attr *a;\n+\n+\tfor (a = git_attr_hash[pos]; a; a = a->next) {\n+\t\tif (a->h == hval &&\n+\t\t    !memcmp(a->name, name, len) && !a->name[len])\n+\t\t\treturn a;\n+\t}\n+\n+\ta = xmalloc(sizeof(*a) + len + 1);\n+\tmemcpy(a->name, name, len);\n+\ta->name[len] = 0;\n+\ta->h = hval;\n+\ta->next = git_attr_hash[pos];\n+\tgit_attr_hash[pos] = a;\n+\treturn a;\n+}\n+\n+/*\n+ * .gitattributes file is one line per record, each of which is\n+ *\n+ * (1) glob pattern.\n+ * (2) whitespace\n+ * (3) whitespace separated list of attribute names, each of which\n+ *     could be prefixed with '!' to mean \"not set\".\n+ */\n+\n+struct attr_state {\n+\tint unset;\n+\tstruct git_attr *attr;\n+};\n+\n+struct match_attr {\n+\tchar *pattern;\n+\tunsigned num_attr;\n+\tstruct attr_state state[FLEX_ARRAY];\n+};\n+\n+static const char blank[] = \" \\t\\r\\n\";\n+\n+static struct match_attr *parse_attr_line(char *line)\n+{\n+\tint namelen;\n+\tint num_attr;\n+\tchar *cp, *name;\n+\tstruct match_attr *res = res;\n+\tint pass;\n+\n+\tcp = line + strspn(line, blank);\n+\tif (!*cp || *cp == '#')\n+\t\treturn NULL;\n+\tname = cp;\n+\tnamelen = strcspn(name, blank);\n+\n+\tfor (pass = 0; pass < 2; pass++) {\n+\t\t/* pass 0 counts and allocates, pass 1 fills */\n+\t\tnum_attr = 0;\n+\t\tcp = name + namelen;\n+\t\tcp = cp + strspn(cp, blank);\n+\t\twhile (*cp) {\n+\t\t\tchar *ep;\n+\t\t\tep = cp + strcspn(cp, blank);\n+\t\t\tif (pass) {\n+\t\t\t\tstruct attr_state *e;\n+\n+\t\t\t\te = &(res->state[num_attr]);\n+\t\t\t\tif (*cp == '!') {\n+\t\t\t\t\te->unset = 1;\n+\t\t\t\t\tcp++;\n+\t\t\t\t}\n+\t\t\t\te->attr = git_attr(cp, ep - cp);\n+\t\t\t}\n+\t\t\tnum_attr++;\n+\t\t\tcp = ep + strspn(ep, blank);\n+\t\t}\n+\t\tif (pass)\n+\t\t\tbreak;\n+\t\tres = xcalloc(1,\n+\t\t\t      sizeof(*res) +\n+\t\t\t      sizeof(struct attr_state) * num_attr +\n+\t\t\t      namelen + 1);\n+\t\tres->pattern = (char*)&(res->state[num_attr]);\n+\t\tmemcpy(res->pattern, name, namelen);\n+\t\tres->pattern[namelen] = 0;\n+\t\tres->num_attr = num_attr;\n+\t}\n+\treturn res;\n+}\n+\n+/*\n+ * Like info/exclude and .gitignore, the attribute information can\n+ * come from many places.\n+ *\n+ * (1) .gitattribute file of the same directory;\n+ * (2) .gitattribute file of the parent directory if (1) does not have any match;\n+ *     this goes recursively upwards, just like .gitignore\n+ * (3) perhaps $GIT_DIR/info/attributes, as the final fallback.\n+ *\n+ * In the same file, later entries override the earlier match, so in the\n+ * global list, we would have entries from info/attributes the earliest\n+ * (reading the file from top to bottom), .gitattribute of the root\n+ * directory (again, reading the file from top to bottom) down to the\n+ * current directory, and then scan the list backwards to find the first match.\n+ * This is exactly the same as what excluded() does in dir.c to deal with\n+ * .gitignore\n+ */\n+\n+static struct attr_stack {\n+\tstruct attr_stack *prev;\n+\tchar *origin;\n+\tunsigned num_matches;\n+\tstruct match_attr **attrs;\n+} *attr_stack;\n+\n+static void free_attr_elem(struct attr_stack *e)\n+{\n+\tint i;\n+\tfree(e->origin);\n+\tfor (i = 0; i < e->num_matches; i++)\n+\t\tfree(e->attrs[i]);\n+\tfree(e);\n+}\n+\n+static struct attr_stack *read_attr_from_file(const char *path)\n+{\n+\tFILE *fp;\n+\tstruct attr_stack *res;\n+\tchar buf[2048];\n+\n+\tres = xcalloc(1, sizeof(*res));\n+\tfp = fopen(path, \"r\");\n+\tif (!fp)\n+\t\treturn res;\n+\n+\twhile (fgets(buf, sizeof(buf), fp)) {\n+\t\tstruct match_attr *a = parse_attr_line(buf);\n+\t\tif (!a)\n+\t\t\tcontinue;\n+\t\tres->attrs = xrealloc(res->attrs, res->num_matches + 1);\n+\t\tres->attrs[res->num_matches++] = a;\n+\t}\n+\tfclose(fp);\n+\treturn res;\n+}\n+\n+static void prepare_attr_stack(const char *path, int dirlen)\n+{\n+\tstruct attr_stack *elem;\n+\tint len;\n+\tchar pathbuf[PATH_MAX];\n+\n+\tif (!attr_stack) {\n+\t\telem = read_attr_from_file(git_path(\"info/attributes\"));\n+\t\telem->origin = NULL;\n+\t\tattr_stack = elem;\n+\n+\t\telem = read_attr_from_file(GITATTRIBUTES_FILE);\n+\t\telem->origin = strdup(\"\");\n+\t\telem->prev = attr_stack;\n+\t\tattr_stack = elem;\n+\t}\n+\n+\twhile (attr_stack && attr_stack->origin) {\n+\t\tint namelen = strlen(attr_stack->origin);\n+\t\tint len = (dirlen < namelen) ? dirlen : namelen;\n+\t\tif (!strncmp(attr_stack->origin, path, len))\n+\t\t\tbreak;\n+\t\telem = attr_stack;\n+\t\tattr_stack = elem->prev;\n+\t\tfree_attr_elem(elem);\n+\t}\n+\n+\twhile (1) {\n+\t\tchar *cp;\n+\n+\t\tlen = strlen(attr_stack->origin);\n+\t\tif (dirlen <= len)\n+\t\t\tbreak;\n+\t\tmemcpy(pathbuf, path, dirlen);\n+\t\tmemcpy(pathbuf + dirlen, \"/\", 2);\n+\t\tcp = strchr(pathbuf + len + 1, '/');\n+\t\tstrcpy(cp + 1, GITATTRIBUTES_FILE);\n+\t\telem = read_attr_from_file(pathbuf);\n+\t\t*cp = '\\0';\n+\t\telem->origin = strdup(pathbuf);\n+\t\telem->prev = attr_stack;\n+\t\tattr_stack = elem;\n+\t}\n+}\n+\n+static int path_matches(const char *pathname, int pathlen,\n+\t\t\tconst char *pattern,\n+\t\t\tconst char *base, int baselen)\n+{\n+\tif (!strchr(pattern, '/')) {\n+\t\t/* match basename */\n+\t\tconst char *basename = strrchr(pathname, '/');\n+\t\tbasename = basename ? basename + 1 : pathname;\n+\t\treturn (fnmatch(pattern, basename, 0) == 0);\n+\t}\n+\t/*\n+\t * match with FNM_PATHNAME; the pattern has base implicitly\n+\t * in front of it.\n+\t */\n+\tif (*pattern == '/')\n+\t\tpattern++;\n+\tif (pathlen < baselen ||\n+\t    (baselen && pathname[baselen - 1] != '/') ||\n+\t    strncmp(pathname, base, baselen))\n+\t\treturn 0;\n+\treturn fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;\n+}\n+\n+/*\n+ * I do not like this at all.  Only because we allow individual\n+ * attribute to be set or unset incrementally by individual\n+ * lines in .gitattribute files, we need to do this triple\n+ * loop which looks quite wasteful.\n+ */\n+static int fill(const char *path, int pathlen,\n+\t\tstruct attr_stack *stk, struct git_attr_check *check,\n+\t\tint num, int rem)\n+{\n+\tint i, j, k;\n+\tconst char *base = stk->origin ? stk->origin : \"\";\n+\n+\tfor (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {\n+\t\tstruct match_attr *a = stk->attrs[i];\n+\t\tif (path_matches(path, pathlen,\n+\t\t\t\t a->pattern, base, strlen(base))) {\n+\t\t\tfor (j = 0; j < a->num_attr; j++) {\n+\t\t\t\tstruct git_attr *attr = a->state[j].attr;\n+\t\t\t\tint set = !a->state[j].unset;\n+\t\t\t\tfor (k = 0; k < num; k++) {\n+\t\t\t\t\tif (0 <= check[k].isset ||\n+\t\t\t\t\t    check[k].attr != attr)\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\tcheck[k].isset = set;\n+\t\t\t\t\trem--;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn rem;\n+}\n+\n+int git_checkattr(const char *path, int num, struct git_attr_check *check)\n+{\n+\tstruct attr_stack *stk;\n+\tconst char *cp;\n+\tint dirlen, pathlen, i, rem;\n+\n+\tfor (i = 0; i < num; i++)\n+\t\tcheck[i].isset = -1;\n+\n+\tpathlen = strlen(path);\n+\tcp = strrchr(path, '/');\n+\tif (!cp)\n+\t\tdirlen = 0;\n+\telse\n+\t\tdirlen = cp - path;\n+\tprepare_attr_stack(path, dirlen);\n+\trem = num;\n+\tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev)\n+\t\trem = fill(path, pathlen, stk, check, num, rem);\n+\treturn 0;\n+}\ndiff --git a/attr.h b/attr.h\nnew file mode 100644\nindex 0000000..1e5ab40\n--- /dev/null\n+++ b/attr.h\n@@ -0,0 +1,16 @@\n+#ifndef ATTR_H\n+#define ATTR_H\n+\n+/* An attribute is a pointer to this opaque structure */\n+struct git_attr;\n+\n+struct git_attr *git_attr(const char *, int);\n+\n+struct git_attr_check {\n+\tstruct git_attr *attr;\n+\tint isset;\n+};\n+\n+int git_checkattr(const char *path, int, struct git_attr_check *);\n+\n+#endif /* ATTR_H */\ndiff --git a/builtin-check-attr.c b/builtin-check-attr.c\nnew file mode 100644\nindex 0000000..643f3ba\n--- /dev/null\n+++ b/builtin-check-attr.c\n@@ -0,0 +1,31 @@\n+#include \"builtin.h\"\n+#include \"attr.h\"\n+\n+static const char check_attr_usage[] =\n+\"git-check-attr pathname attr...\";\n+\n+int cmd_check_attr(int argc, const char **argv, const char *prefix)\n+{\n+\n+\tstruct git_attr_check *check;\n+\tint cnt, i;\n+\n+\tif (argc < 3)\n+\t\tusage(check_attr_usage);\n+\n+\tcnt = argc - 2;\n+\tcheck = xcalloc(cnt, sizeof(*check));\n+\tfor (i = 0; i < cnt; i++) {\n+\t\tconst char *name;\n+\t\tname = argv[i+2];\n+\t\tcheck[i].attr = git_attr(name, strlen(name));\n+\t}\n+\tif (git_checkattr(argv[1], cnt, check))\n+\t\tdie(\"git_checkattr died\");\n+\tfor (i = 0; i < cnt; i++)\n+\t\tprintf(\"%s: %s\\n\", argv[i+2],\n+\t\t       (check[i].isset < 0) ? \"unspecified\" :\n+\t\t       (check[i].isset == 0) ? \"unset\" :\n+\t\t       \"set\");\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex af203e9..d3f3a74 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -22,6 +22,7 @@ extern int cmd_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_bundle(int argc, const char **argv, const char *prefix);\n extern int cmd_cat_file(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n+extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\ndiff --git a/cache.h b/cache.h\nindex b1bd9e4..bec1938 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -151,6 +151,7 @@ enum object_type {\n #define CONFIG_ENVIRONMENT \"GIT_CONFIG\"\n #define CONFIG_LOCAL_ENVIRONMENT \"GIT_CONFIG_LOCAL\"\n #define EXEC_PATH_ENVIRONMENT \"GIT_EXEC_PATH\"\n+#define GITATTRIBUTES_FILE \".gitattributes\"\n \n extern int is_bare_repository_cfg;\n extern int is_bare_repository(void);\ndiff --git a/git.c b/git.c\nindex 7def319..f200907 100644\n--- a/git.c\n+++ b/git.c\n@@ -234,6 +234,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)\n \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n \t\t{ \"checkout-index\", cmd_checkout_index, RUN_SETUP },\n \t\t{ \"check-ref-format\", cmd_check_ref_format },\n+\t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NOT_BARE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NOT_BARE },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n-- \n1.5.1.1.784.g95e2\n"},{"id":"39278","messageId":"200704131033.15751.andyparkins@gmail.com","threadId":"7639","inReplyTo":"7vr6qod4wh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Add basic infrastructure to assign attributes to paths","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-13T09:33:12Z","receivedAt":"2007-04-13T09:33:12Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Friday 2007, April 13, Junio C Hamano wrote:\n\n> parent directories.  Lines in $GIT_DIR/info/attributes file are\n> used as the lowest precedence default rules.\n\nShouldn't this be the highest precedence?  This would be important for \nthose cases where I (as a fringe developer) disagree with an attribute \nthat's been assigned in-tree.  I don't want to force my will on every \nother developer, but would want my repository to work how I like it.  \n\nFor example: what if I /can/ read postscript :-)\n\nIncidentally - already I love gitattributes - nicely done Junio.\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39288","messageId":"Pine.LNX.4.64.0704130804020.28042@woody.linux-foundation.org","threadId":"7639","inReplyTo":"7vr6qod4wh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Add basic infrastructure to assign attributes to paths","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-13T15:04:28Z","receivedAt":"2007-04-13T15:04:28Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 13 Apr 2007, Junio C Hamano wrote:\n>\n> This adds the basic infrastructure to assign attributes to\n> paths, in a way similar to what the exclusion mechanism does\n> based on $GIT_DIR/info/exclude and .gitignore files.\n\nMe likee. The patches look much simpler/cleaner than I expected.\n\n\t\tLinus\n"},{"id":"39354","messageId":"7vejmm78qp.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"200704131033.15751.andyparkins@gmail.com","subject":"Re: [PATCH 1/3] Add basic infrastructure to assign attributes to paths","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T00:59:42Z","receivedAt":"2007-04-15T00:59:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n>> parent directories.  Lines in $GIT_DIR/info/attributes file are\n>> used as the lowest precedence default rules.\n>\n> Shouldn't this be the highest precedence?  This would be important for \n> those cases where I (as a fringe developer) disagree with an attribute \n> that's been assigned in-tree.  I don't want to force my will on every \n> other developer, but would want my repository to work how I like it.  \n\nJohannes Sixt <J.Sixt@eudaptics.com> writes:\n>> This makes paths with 'nodiff' attribute not to produce\n>> \"textual\" diffs from 'git-diff' family.\n>\n> If saying \"nodiff\" can be made equivalent to \"!diff\", then I'd strongly\n> prefer an attribute \"diff\" over \"nodiff\". I'm a strong disbeliever in\n> double negation.\n\nBoth of these are good points.\n\nThe only reason I initially made it 'nodiff' was to have a pair\nof examples to demonstrate positive and negative setting of\nattributes, and I agree it makes more sense to say 'diff' in\npositive.\n\nI reshuffled the code to make $GIT_DIR/info/attributes the\nhighest precedence, and unsetting 'diff' attribute to disable\ndiff; the result is in 'next'.\n\nI'll follow this message up with a few more patches in the\nseries.\n"},{"id":"39355","messageId":"7v8xcu78o6.fsf_-_@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"7vejmm78qp.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 1/2] attribute macro support","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T01:01:13Z","receivedAt":"2007-04-15T01:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This adds \"attribute macros\" (for lack of better name).  So far,\nwe have low-level attributes such as crlf and diff, which are\ndefined in operational terms --- setting or unsetting them on a\nparticular path directly affects what is done to the path.  For\nexample, in order to decline diffs or crlf conversions on a\nbinary blob, no diffs on PostScript files, and treat all other\nfiles normally, you would have something like these:\n\n\t*\t\tdiff crlf\n\t*.ps\t\t!diff\n\tproprietary.o\t!diff !crlf\n\nThat is fine as the operation goes, but gets unwieldy rather\nrapidly, when we start adding more low-level attributes that are\ndefined in operational terms.  A near-term example of such an\nattribute would be 'merge-3way' which would control if git\nshould attempt the usual 3-way file-level merge internally, or\nleave merging to a specialized external program of user's\nchoice.  When it is added, we do _not_ want to force the users\nto update the above to:\n\n\t*\t\tdiff crlf merge-3way\n\t*.ps\t\t!diff\n\tproprietary.o\t!diff !crlf !merge-3way\n\nThe way this patch solves this issue is to realize that the\nattributes the user is assigning to paths are not defined in\nterms of operations but in terms of what they are.\n\nAll of the three low-level attributes usually make sense for\nmost of the files that sane SCM users have git operate on (these\nfiles are typically called \"text').  Only a few cases, such as\nbinary blob, need exception to decline the \"usual treatment\ngiven to text files\" -- and people mark them as \"binary\".\n\nSo this allows the $GIT_DIR/info/alternates and .gitattributes\nat the toplevel of the project to also specify attributes that\nassigns other attributes.  The syntax is '[attr]' followed by an\nattribute name followed by a list of attribute names:\n\n\t[attr] binary\t!diff !crlf !merge-3way\n\nWhen \"binary\" attribute is set to a path, if the path has not\ngot diff/crlf/merge-3way attribute set or unset by other rules,\nthis rule unsets the three low-level attributes.\n\nIt is expected that the user level .gitattributes will be\nexpressed mostly in terms of attributes based on what the files\nare, and the above sample would become like this:\n\n\t(built-in attribute configuration)\n\t[attr] binary\t!diff !crlf !merge-3way\n\t*\t\tdiff crlf merge-3way\n\n\t(project specific .gitattributes)\n\tproprietary.o\tbinary\n\n\t(user preference $GIT_DIR/info/attributes)\n\t*.ps\t\t!diff\n\nThere are a few caveats.\n\n * As described above, you can define these macros only in\n   $GIT_DIR/info/attributes and toplevel .gitattributes.\n\n * There is no attempt to detect circular definition of macro\n   attributes, and definitions are evaluated from bottom to top\n   as usual to fill in other attributes that have not yet got\n   values.  The following would work as expected:\n\n\t[attr] text\tdiff crlf\n\t[attr] ps\ttext !diff\n\t*.ps\tps\n\n   while this would most likely not (I haven't tried):\n\n\t[attr] ps\ttext !diff\n\t[attr] text\tdiff crlf\n\t*.ps\tps\n\n * When a macro says \"[attr] A B !C\", saying that a path does\n   not have attribute A does not let you tell anything about\n   attributes B or C.  That is, given this:\n\n\t[attr] text\tdiff crlf\n\t[attr] ps\ttext !diff\n\t*.txt !ps\n\n  path hello.txt, which would match \"*.txt\" pattern, would have\n  \"ps\" attribute set to zero, but that does not make text\n  attribute of hello.txt set to false (nor diff attribute set to\n  true).\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n attr.c  |  140 ++++++++++++++++++++++++++++++++++++++++++++++++---------------\n cache.h |    1 +\n 2 files changed, 108 insertions(+), 33 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 7435d92..3a14df1 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -16,9 +16,12 @@\n struct git_attr {\n \tstruct git_attr *next;\n \tunsigned h;\n+\tint attr_nr;\n \tchar name[FLEX_ARRAY];\n };\n+static int attr_nr;\n \n+static struct git_attr_check *check_all_attr;\n static struct git_attr *(git_attr_hash[HASHSIZE]);\n \n static unsigned hash_name(const char *name, int namelen)\n@@ -50,7 +53,12 @@ struct git_attr *git_attr(const char *name, int len)\n \ta->name[len] = 0;\n \ta->h = hval;\n \ta->next = git_attr_hash[pos];\n+\ta->attr_nr = attr_nr++;\n \tgit_attr_hash[pos] = a;\n+\n+\tcheck_all_attr = xrealloc(check_all_attr,\n+\t\t\t\t  sizeof(*check_all_attr) * attr_nr);\n+\tcheck_all_attr[a->attr_nr].attr = a;\n \treturn a;\n }\n \n@@ -69,26 +77,46 @@ struct attr_state {\n };\n \n struct match_attr {\n-\tchar *pattern;\n+\tunion {\n+\t\tchar *pattern;\n+\t\tstruct git_attr *attr;\n+\t} u;\n+\tchar is_macro;\n \tunsigned num_attr;\n \tstruct attr_state state[FLEX_ARRAY];\n };\n \n static const char blank[] = \" \\t\\r\\n\";\n \n-static struct match_attr *parse_attr_line(const char *line)\n+static struct match_attr *parse_attr_line(const char *line, const char *src,\n+\t\t\t\t\t  int lineno, int macro_ok)\n {\n \tint namelen;\n \tint num_attr;\n \tconst char *cp, *name;\n \tstruct match_attr *res = res;\n \tint pass;\n+\tint is_macro;\n \n \tcp = line + strspn(line, blank);\n \tif (!*cp || *cp == '#')\n \t\treturn NULL;\n \tname = cp;\n \tnamelen = strcspn(name, blank);\n+\tif (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&\n+\t    !prefixcmp(name, ATTRIBUTE_MACRO_PREFIX)) {\n+\t\tif (!macro_ok) {\n+\t\t\tfprintf(stderr, \"%s not allowed: %s:%d\\n\",\n+\t\t\t\tname, src, lineno);\n+\t\t\treturn NULL;\n+\t\t}\n+\t\tis_macro = 1;\n+\t\tname += strlen(ATTRIBUTE_MACRO_PREFIX);\n+\t\tname += strspn(name, blank);\n+\t\tnamelen = strcspn(name, blank);\n+\t}\n+\telse\n+\t\tis_macro = 0;\n \n \tfor (pass = 0; pass < 2; pass++) {\n \t\t/* pass 0 counts and allocates, pass 1 fills */\n@@ -113,13 +141,19 @@ static struct match_attr *parse_attr_line(const char *line)\n \t\t}\n \t\tif (pass)\n \t\t\tbreak;\n+\n \t\tres = xcalloc(1,\n \t\t\t      sizeof(*res) +\n \t\t\t      sizeof(struct attr_state) * num_attr +\n-\t\t\t      namelen + 1);\n-\t\tres->pattern = (char*)&(res->state[num_attr]);\n-\t\tmemcpy(res->pattern, name, namelen);\n-\t\tres->pattern[namelen] = 0;\n+\t\t\t      (is_macro ? 0 : namelen + 1));\n+\t\tif (is_macro)\n+\t\t\tres->u.attr = git_attr(name, namelen);\n+\t\telse {\n+\t\t\tres->u.pattern = (char*)&(res->state[num_attr]);\n+\t\t\tmemcpy(res->u.pattern, name, namelen);\n+\t\t\tres->u.pattern[namelen] = 0;\n+\t\t}\n+\t\tres->is_macro = is_macro;\n \t\tres->num_attr = num_attr;\n \t}\n \treturn res;\n@@ -167,10 +201,13 @@ static struct attr_stack *read_attr_from_array(const char **list)\n {\n \tstruct attr_stack *res;\n \tconst char *line;\n+\tint lineno = 0;\n \n \tres = xcalloc(1, sizeof(*res));\n \twhile ((line = *(list++)) != NULL) {\n-\t\tstruct match_attr *a = parse_attr_line(line);\n+\t\tstruct match_attr *a;\n+\n+\t\ta = parse_attr_line(line, \"[builtin]\", ++lineno, 1);\n \t\tif (!a)\n \t\t\tcontinue;\n \t\tres->attrs = xrealloc(res->attrs, res->num_matches + 1);\n@@ -179,11 +216,12 @@ static struct attr_stack *read_attr_from_array(const char **list)\n \treturn res;\n }\n \n-static struct attr_stack *read_attr_from_file(const char *path)\n+static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n {\n \tFILE *fp;\n \tstruct attr_stack *res;\n \tchar buf[2048];\n+\tint lineno = 0;\n \n \tres = xcalloc(1, sizeof(*res));\n \tfp = fopen(path, \"r\");\n@@ -191,7 +229,9 @@ static struct attr_stack *read_attr_from_file(const char *path)\n \t\treturn res;\n \n \twhile (fgets(buf, sizeof(buf), fp)) {\n-\t\tstruct match_attr *a = parse_attr_line(buf);\n+\t\tstruct match_attr *a;\n+\n+\t\ta = parse_attr_line(buf, path, ++lineno, macro_ok);\n \t\tif (!a)\n \t\t\tcontinue;\n \t\tres->attrs = xrealloc(res->attrs, res->num_matches + 1);\n@@ -206,11 +246,17 @@ static void debug_info(const char *what, struct attr_stack *elem)\n {\n \tfprintf(stderr, \"%s: %s\\n\", what, elem->origin ? elem->origin : \"()\");\n }\n+static void debug_set(const char *what, const char *match, struct git_attr *attr, int set)\n+{\n+\tfprintf(stderr, \"%s: %s => %d (%s)\\n\",\n+\t\twhat, attr->name, set, match);\n+}\n #define debug_push(a) debug_info(\"push\", (a))\n #define debug_pop(a) debug_info(\"pop\", (a))\n #else\n #define debug_push(a) do { ; } while (0)\n #define debug_pop(a) do { ; } while (0)\n+#define debug_set(a,b,c,d) do { ; } while (0)\n #endif\n \n static void prepare_attr_stack(const char *path, int dirlen)\n@@ -238,13 +284,13 @@ static void prepare_attr_stack(const char *path, int dirlen)\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n \n-\t\telem = read_attr_from_file(GITATTRIBUTES_FILE);\n+\t\telem = read_attr_from_file(GITATTRIBUTES_FILE, 1);\n \t\telem->origin = strdup(\"\");\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n \t\tdebug_push(elem);\n \n-\t\telem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE));\n+\t\telem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);\n \t\telem->origin = NULL;\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n@@ -286,7 +332,7 @@ static void prepare_attr_stack(const char *path, int dirlen)\n \t\tmemcpy(pathbuf + dirlen, \"/\", 2);\n \t\tcp = strchr(pathbuf + len + 1, '/');\n \t\tstrcpy(cp + 1, GITATTRIBUTES_FILE);\n-\t\telem = read_attr_from_file(pathbuf);\n+\t\telem = read_attr_from_file(pathbuf, 0);\n \t\t*cp = '\\0';\n \t\telem->origin = strdup(pathbuf);\n \t\telem->prev = attr_stack;\n@@ -324,31 +370,26 @@ static int path_matches(const char *pathname, int pathlen,\n \treturn fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;\n }\n \n-/*\n- * I do not like this at all.  Only because we allow individual\n- * attribute to be set or unset incrementally by individual\n- * lines in .gitattribute files, we need to do this triple\n- * loop which looks quite wasteful.\n- */\n-static int fill(const char *path, int pathlen,\n-\t\tstruct attr_stack *stk, struct git_attr_check *check,\n-\t\tint num, int rem)\n+static int fill(const char *path, int pathlen, struct attr_stack *stk, int rem)\n {\n-\tint i, j, k;\n \tconst char *base = stk->origin ? stk->origin : \"\";\n+\tint i, j;\n+\tstruct git_attr_check *check = check_all_attr;\n \n \tfor (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {\n \t\tstruct match_attr *a = stk->attrs[i];\n+\t\tif (a->is_macro)\n+\t\t\tcontinue;\n \t\tif (path_matches(path, pathlen,\n-\t\t\t\t a->pattern, base, strlen(base))) {\n-\t\t\tfor (j = 0; j < a->num_attr; j++) {\n+\t\t\t\t a->u.pattern, base, strlen(base))) {\n+\t\t\tfor (j = 0; 0 < rem && j < a->num_attr; j++) {\n \t\t\t\tstruct git_attr *attr = a->state[j].attr;\n \t\t\t\tint set = !a->state[j].unset;\n-\t\t\t\tfor (k = 0; k < num; k++) {\n-\t\t\t\t\tif (0 <= check[k].isset ||\n-\t\t\t\t\t    check[k].attr != attr)\n-\t\t\t\t\t\tcontinue;\n-\t\t\t\t\tcheck[k].isset = set;\n+\t\t\t\tint *n = &(check[attr->attr_nr].isset);\n+\n+\t\t\t\tif (*n < 0) {\n+\t\t\t\t\tdebug_set(\"fill\", a->u.pattern, attr, set);\n+\t\t\t\t\t*n = set;\n \t\t\t\t\trem--;\n \t\t\t\t}\n \t\t\t}\n@@ -357,14 +398,40 @@ static int fill(const char *path, int pathlen,\n \treturn rem;\n }\n \n+static int macroexpand(struct attr_stack *stk, int rem)\n+{\n+\tint i, j;\n+\tstruct git_attr_check *check = check_all_attr;\n+\n+\tfor (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {\n+\t\tstruct match_attr *a = stk->attrs[i];\n+\t\tif (!a->is_macro)\n+\t\t\tcontinue;\n+\t\tif (check[a->u.attr->attr_nr].isset < 0)\n+\t\t\tcontinue;\n+\t\tfor (j = 0; 0 < rem && j < a->num_attr; j++) {\n+\t\t\tstruct git_attr *attr = a->state[j].attr;\n+\t\t\tint set = !a->state[j].unset;\n+\t\t\tint *n = &(check[attr->attr_nr].isset);\n+\n+\t\t\tif (*n < 0) {\n+\t\t\t\tdebug_set(\"expand\", a->u.attr->name, attr, set);\n+\t\t\t\t*n = set;\n+\t\t\t\trem--;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn rem;\n+}\n+\n int git_checkattr(const char *path, int num, struct git_attr_check *check)\n {\n \tstruct attr_stack *stk;\n \tconst char *cp;\n \tint dirlen, pathlen, i, rem;\n \n-\tfor (i = 0; i < num; i++)\n-\t\tcheck[i].isset = -1;\n+\tfor (i = 0; i < attr_nr; i++)\n+\t\tcheck_all_attr[i].isset = -1;\n \n \tpathlen = strlen(path);\n \tcp = strrchr(path, '/');\n@@ -373,8 +440,15 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)\n \telse\n \t\tdirlen = cp - path;\n \tprepare_attr_stack(path, dirlen);\n-\trem = num;\n+\trem = attr_nr;\n+\tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev)\n+\t\trem = fill(path, pathlen, stk, rem);\n+\n \tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev)\n-\t\trem = fill(path, pathlen, stk, check, num, rem);\n+\t\trem = macroexpand(stk, rem);\n+\n+\tfor (i = 0; i < num; i++)\n+\t\tcheck[i].isset = check_all_attr[check[i].attr->attr_nr].isset;\n+\n \treturn 0;\n }\ndiff --git a/cache.h b/cache.h\nindex 63af43f..38ad006 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -153,6 +153,7 @@ enum object_type {\n #define EXEC_PATH_ENVIRONMENT \"GIT_EXEC_PATH\"\n #define GITATTRIBUTES_FILE \".gitattributes\"\n #define INFOATTRIBUTES_FILE \"info/attributes\"\n+#define ATTRIBUTE_MACRO_PREFIX \"[attr]\"\n \n extern int is_bare_repository_cfg;\n extern int is_bare_repository(void);\n-- \n1.5.1.1.810.gac3a\n"},{"id":"39356","messageId":"7vvefy5tzo.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"7vejmm78qp.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 2/2] Define a few built-in attribute rules.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T01:03:39Z","receivedAt":"2007-04-15T01:03:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This adds an obviously sane pair of default attribute rules as built-ins.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n attr.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 3a14df1..9068c2e 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -194,6 +194,8 @@ static void free_attr_elem(struct attr_stack *e)\n }\n \n static const char *builtin_attr[] = {\n+\t\"[attr]binary !diff !crlf\",\n+\t\"* diff crlf\",\n \tNULL,\n };\n \n-- \n1.5.1.1.810.gac3a\n"},{"id":"39358","messageId":"Pine.LNX.4.64.0704141839030.5473@woody.linux-foundation.org","threadId":"7639","inReplyTo":"7vvefy5tzo.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-15T01:41:59Z","receivedAt":"2007-04-15T01:41:59Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 14 Apr 2007, Junio C Hamano wrote:\n>\n> This adds an obviously sane pair of default attribute rules as built-ins.\n\nI'm not sure.\n\n> +\t\"[attr]binary !diff !crlf\",\n> +\t\"* diff crlf\",\n\nWhy would \n\n\t* diff crlf\n\nbe \"obviously sane\"?\n\nIn fact, I'd call it obviously insane.\n\nWe do *not* want to default crlf to all files. We want the default to be \n\"automatic crlf depending on content\". \n\nThen, on top of that, you can *explicitly* specify crlf or !crlf on some \nparticular filename pattern bases.\n\n(Side thought - I have to concur with whoever suggested \"-\" instead of \n\"!\". It just reads better, I think)\n\n\t\tLinus\n"},{"id":"39359","messageId":"0D85864F-E9E9-4F22-93E6-B5B8F9A6C383@silverinsanity.com","threadId":"7639","inReplyTo":"Pine.LNX.4.64.0704141839030.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-04-15T01:48:42Z","receivedAt":"2007-04-15T01:48:42Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Apr 14, 2007, at 9:41 PM, Linus Torvalds wrote:\n\n> (Side thought - I have to concur with whoever suggested \"-\" instead of\n> \"!\". It just reads better, I think)\n\nThe \"diff\" vs \"nodiff\" argument reminds me of Vim option setting.   \nMarking a path \"diff\" means a diff is useful.  Marking it \"nodiff\"  \nmarks it non-useful, same with \"crlf\" and \"nocrlf\".  Just an idea to  \nthrow out there.  It's fairly unambiguous.\n\n~~ Brian G.\n"},{"id":"39361","messageId":"7vr6qm5r73.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"Pine.LNX.4.64.0704141839030.5473@woody.linux-foundation.org","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T02:04:00Z","receivedAt":"2007-04-15T02:04:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 14 Apr 2007, Junio C Hamano wrote:\n>>\n>> This adds an obviously sane pair of default attribute rules as built-ins.\n>\n> I'm not sure.\n>\n>> +\t\"[attr]binary !diff !crlf\",\n>> +\t\"* diff crlf\",\n>\n> Why would \n>\n> \t* diff crlf\n>\n> be \"obviously sane\"?\n>\n> In fact, I'd call it obviously insane.\n>\n> We do *not* want to default crlf to all files. We want the default to be \n> \"automatic crlf depending on content\". \n\nYou do not have to worry.\n\nThat's how \"crlf\" is defined.  Paths you explicitly say !crlf\nwill _not_ go through the existing core.autocrlf mechanism.\n\n\"* crlf\" just says, by default everybody is subject to core.autocrlf,\nand on sane platforms, core.autocrlf is by default off, hence you will\nnot get LF <-> CRLF applied.\n"},{"id":"39362","messageId":"7v8xcu5ps7.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"7vr6qm5r73.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T02:34:32Z","receivedAt":"2007-04-15T02:34:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> Why would \n>>\n>> \t* diff crlf\n>>\n>> be \"obviously sane\"?\n>>\n>> In fact, I'd call it obviously insane.\n>>\n>> We do *not* want to default crlf to all files. We want the default to be \n>> \"automatic crlf depending on content\". \n>\n> You do not have to worry.\n>\n> That's how \"crlf\" is defined.  Paths you explicitly say !crlf\n> will _not_ go through the existing core.autocrlf mechanism.\n>\n> \"* crlf\" just says, by default everybody is subject to core.autocrlf,\n> and on sane platforms, core.autocrlf is by default off, hence you will\n> not get LF <-> CRLF applied.\n\nHaving said that, if we really wanted to, we could introduce a\nway to explicitly say \"Even if the contents do not look like\ntext, apply line ending conversion, always\", by redefining the\nmeaning of 'crlf' attribute.\n\nBut I do not know if that makes much sense.  Being able to turn\n_off_ would be a good thing because a particular file that looks\nlike CRLF terminated text might not be text.  But the other way\naround?  IOW, I do not think of a case where a file that does\nnot even look like a text wants CRLF conversion.\n\n---\n\ndiff --git a/convert.c b/convert.c\nindex 20c744a..f9e5d63 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -191,7 +191,7 @@ static void setup_crlf_check(struct git_attr_check *check)\n \tcheck->attr = attr_crlf;\n }\n \n-static int git_path_is_binary(const char *path)\n+static int git_path_check_crlf(const char *path)\n {\n \tstruct git_attr_check attr_crlf_check;\n \n@@ -202,20 +202,31 @@ static int git_path_is_binary(const char *path)\n \t * disable autocrlf only when crlf attribute is explicitly\n \t * unset.\n \t */\n-\treturn (!git_checkattr(path, 1, &attr_crlf_check) &&\n-\t\t(0 == attr_crlf_check.isset));\n+\tif (!git_checkattr(path, 1, &attr_crlf_check))\n+\t\treturn -1;\n+\treturn attr_crlf_check.isset;\n }\n \n int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n {\n-\tif (git_path_is_binary(path))\n+\tswitch (git_path_check_crlf(path)) {\n+\tcase 0:\n \t\treturn 0;\n-\treturn autocrlf_to_git(path, bufp, sizep);\n+\tcase 1:\n+\t\treturn forcecrlf_to_git(path, bufp, sizep);\n+\tdefault:\n+\t\treturn autocrlf_to_git(path, bufp, sizep);\n+\t}\n }\n \n int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n {\n-\tif (git_path_is_binary(path))\n+\tswitch (git_path_check_crlf(path)) {\n+\tcase 0:\n \t\treturn 0;\n-\treturn autocrlf_to_working_tree(path, bufp, sizep);\n+\tcase 1:\n+\t\treturn forcecrlf_to_working_tree(path, bufp, sizep);\n+\tdefault:\n+\t\treturn autocrlf_to_working_tree(path, bufp, sizep);\n+\t}\n }\n"},{"id":"39363","messageId":"Pine.LNX.4.64.0704142103210.5473@woody.linux-foundation.org","threadId":"7639","inReplyTo":"7vr6qm5r73.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-15T04:30:37Z","receivedAt":"2007-04-15T04:30:37Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 14 Apr 2007, Junio C Hamano wrote:\n> \n> You do not have to worry.\n\nI do.\n\n> That's how \"crlf\" is defined.  Paths you explicitly say !crlf\n> will _not_ go through the existing core.autocrlf mechanism.\n\nThat's broken.\n\nIt should be:\n\n - \"crlf\": always do crlf.\n - \"!crlf\": never do crlf.\n - no attrbute: guess.\n\nWhy? Because quite frankly, it's quite possible that some file really *is* \ntext, even if the content-based guessing doesn't catch it. \n\nIt boils down to a simple truth: if our content-based guessing is so \nperfect that it never makes mistakes, there's no *point* to having a \n'crlf' attribute in the first place!\n\nHere's a simple example:\n\n\techo -e '\\007Bell!' > bell\n\nand just because we consider the BEL character to be binary, we'll think \nthe file is binary.\n\nCould we add the BEL character? Sure. But that's not the point. The \n*point* is that the whole and only reason for attributes in the first \nplace is to _override_ guessing.\n\nThe guesses should be good enough that hopefully nobody really will ever \nneed attributes. But people do strange things.\n\n\t\t\tLinus\n"},{"id":"39373","messageId":"Pine.LNX.4.64.0704151055530.18846@racer.site","threadId":"7639","inReplyTo":"7vr6qod4wh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Add basic infrastructure to assign attributes to paths","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-15T15:54:07Z","receivedAt":"2007-04-15T15:54:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 13 Apr 2007, Junio C Hamano wrote:\n\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -283,7 +283,7 @@ LIB_H = \\\n>  \tdiff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \\\n>  \trun-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \\\n>  \ttree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \\\n> -\tutf8.h reflog-walk.h patch-ids.h\n> +\tutf8.h reflog-walk.h attr.h\n\nDid you really want to remove \"patch-ids.h\" from the list?\n\nCiao,\nDscho\n"},{"id":"39375","messageId":"Pine.LNX.4.64.0704151819390.18846@racer.site","threadId":"7639","inReplyTo":"7v8xcu5ps7.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-15T16:21:05Z","receivedAt":"2007-04-15T16:21:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Apr 2007, Junio C Hamano wrote:\n\n> [...] if we really wanted to, we could introduce a way to explicitly say \n> \"Even if the contents do not look like text, apply line ending \n> conversion, always\", by redefining the meaning of 'crlf' attribute.\n\nI think it might make more sense to introduce a way to say \"do not even \ncheck; I _know_ that I want crlf on these\".\n\nIt might show performance improvements on large repos, for example.\n\nCiao,\nDscho\n"},{"id":"39388","messageId":"7vwt0d2yw9.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"Pine.LNX.4.64.0704151819390.18846@racer.site","subject":"Re: [PATCH 2/2] Define a few built-in attribute rules.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T19:58:14Z","receivedAt":"2007-04-15T19:58:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Sat, 14 Apr 2007, Junio C Hamano wrote:\n>\n>> [...] if we really wanted to, we could introduce a way to explicitly say \n>> \"Even if the contents do not look like text, apply line ending \n>> conversion, always\", by redefining the meaning of 'crlf' attribute.\n>\n> I think it might make more sense to introduce a way to say \"do not even \n> check; I _know_ that I want crlf on these\".\n\nYou said I wanted to say in the message in clearer words.  We\nare in agreement.\n\nAlso the comments to the two patches in this series you sent are\nvalid; I reworked them before merging to 'next' and pushing them\nout yesterday, but I think the Makefile thing still remains.\nWill fix up.\n"},{"id":"39419","messageId":"7vr6ql1ben.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"Pine.LNX.4.64.0704142103210.5473@woody.linux-foundation.org","subject":"[PATCH] Fix 'crlf' attribute semantics.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T23:10:56Z","receivedAt":"2007-04-15T23:10:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier we said 'crlf lets the path go through core.autocrlf\nprocess while !crlf disables it altogether'.  This fixes the\nsemantics to:\n\n - Lack of 'crlf' attribute makes core.autocrlf to apply\n   (i.e. we guess based on the contents and if platform\n   expresses its desire to have CRLF line endings via\n   core.autocrlf, we do so).\n\n - Setting 'crlf' attribute to true forces CRLF line endings in\n   working tree files, even if blob does not look like text\n   (e.g. contains NUL or other bytes we consider binary).\n\n - Setting 'crlf' attribute to false disables conversion.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n  Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n  > Here's a simple example:\n  >\n  > \techo -e '\\007Bell!' > bell\n  >\n  > and just because we consider the BEL character to be binary, we'll think \n  > the file is binary.\n\n You are right.  This replaces my earlier \"we could do...\" patch.\n\n convert.c |  122 +++++++++++++++++++++++++++++++++++++++----------------------\n 1 files changed, 78 insertions(+), 44 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 20c744a..d0d4b81 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -74,13 +74,13 @@ static int is_binary(unsigned long size, struct text_stat *stats)\n \treturn 0;\n }\n \n-static int autocrlf_to_git(const char *path, char **bufp, unsigned long *sizep)\n+static int crlf_to_git(const char *path, char **bufp, unsigned long *sizep, int guess)\n {\n \tchar *buffer, *nbuf;\n \tunsigned long size, nsize;\n \tstruct text_stat stats;\n \n-\tif (!auto_crlf)\n+\tif (guess && !auto_crlf)\n \t\treturn 0;\n \n \tsize = *sizep;\n@@ -94,19 +94,21 @@ static int autocrlf_to_git(const char *path, char **bufp, unsigned long *sizep)\n \tif (!stats.cr)\n \t\treturn 0;\n \n-\t/*\n-\t * We're currently not going to even try to convert stuff\n-\t * that has bare CR characters. Does anybody do that crazy\n-\t * stuff?\n-\t */\n-\tif (stats.cr != stats.crlf)\n-\t\treturn 0;\n-\n-\t/*\n-\t * And add some heuristics for binary vs text, of course...\n-\t */\n-\tif (is_binary(size, &stats))\n-\t\treturn 0;\n+\tif (guess) {\n+\t\t/*\n+\t\t * We're currently not going to even try to convert stuff\n+\t\t * that has bare CR characters. Does anybody do that crazy\n+\t\t * stuff?\n+\t\t */\n+\t\tif (stats.cr != stats.crlf)\n+\t\t\treturn 0;\n+\n+\t\t/*\n+\t\t * And add some heuristics for binary vs text, of course...\n+\t\t */\n+\t\tif (is_binary(size, &stats))\n+\t\t\treturn 0;\n+\t}\n \n \t/*\n \t * Ok, allocate a new buffer, fill it in, and return true\n@@ -116,28 +118,42 @@ static int autocrlf_to_git(const char *path, char **bufp, unsigned long *sizep)\n \tnbuf = xmalloc(nsize);\n \t*bufp = nbuf;\n \t*sizep = nsize;\n-\tdo {\n-\t\tunsigned char c = *buffer++;\n-\t\tif (c != '\\r')\n-\t\t\t*nbuf++ = c;\n-\t} while (--size);\n+\n+\tif (guess) {\n+\t\tdo {\n+\t\t\tunsigned char c = *buffer++;\n+\t\t\tif (c != '\\r')\n+\t\t\t\t*nbuf++ = c;\n+\t\t} while (--size);\n+\t} else {\n+\t\tdo {\n+\t\t\tunsigned char c = *buffer++;\n+\t\t\tif (! (c == '\\r' && (1 < size && *buffer == '\\n')))\n+\t\t\t\t*nbuf++ = c;\n+\t\t} while (--size);\n+\t}\n \n \treturn 1;\n }\n \n-static int autocrlf_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n+static int autocrlf_to_git(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\treturn crlf_to_git(path, bufp, sizep, 1);\n+}\n+\n+static int forcecrlf_to_git(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\treturn crlf_to_git(path, bufp, sizep, 0);\n+}\n+\n+static int crlf_to_working_tree(const char *path, char **bufp, unsigned long *sizep, int guess)\n {\n \tchar *buffer, *nbuf;\n \tunsigned long size, nsize;\n \tstruct text_stat stats;\n \tunsigned char last;\n \n-\t/*\n-\t * FIXME! Other pluggable conversions should go here,\n-\t * based on filename patterns. Right now we just do the\n-\t * stupid auto-CRLF one.\n-\t */\n-\tif (auto_crlf <= 0)\n+\tif (guess && auto_crlf <= 0)\n \t\treturn 0;\n \n \tsize = *sizep;\n@@ -155,12 +171,14 @@ static int autocrlf_to_working_tree(const char *path, char **bufp, unsigned long\n \tif (stats.lf == stats.crlf)\n \t\treturn 0;\n \n-\t/* If we have any bare CR characters, we're not going to touch it */\n-\tif (stats.cr != stats.crlf)\n-\t\treturn 0;\n+\tif (guess) {\n+\t\t/* If we have any bare CR characters, we're not going to touch it */\n+\t\tif (stats.cr != stats.crlf)\n+\t\t\treturn 0;\n \n-\tif (is_binary(size, &stats))\n-\t\treturn 0;\n+\t\tif (is_binary(size, &stats))\n+\t\t\treturn 0;\n+\t}\n \n \t/*\n \t * Ok, allocate a new buffer, fill it in, and return true\n@@ -182,6 +200,16 @@ static int autocrlf_to_working_tree(const char *path, char **bufp, unsigned long\n \treturn 1;\n }\n \n+static int autocrlf_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\treturn crlf_to_working_tree(path, bufp, sizep, 1);\n+}\n+\n+static int forcecrlf_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n+{\n+\treturn crlf_to_working_tree(path, bufp, sizep, 0);\n+}\n+\n static void setup_crlf_check(struct git_attr_check *check)\n {\n \tstatic struct git_attr *attr_crlf;\n@@ -191,31 +219,37 @@ static void setup_crlf_check(struct git_attr_check *check)\n \tcheck->attr = attr_crlf;\n }\n \n-static int git_path_is_binary(const char *path)\n+static int git_path_check_crlf(const char *path)\n {\n \tstruct git_attr_check attr_crlf_check;\n \n \tsetup_crlf_check(&attr_crlf_check);\n \n-\t/*\n-\t * If crlf is not mentioned, default to autocrlf;\n-\t * disable autocrlf only when crlf attribute is explicitly\n-\t * unset.\n-\t */\n-\treturn (!git_checkattr(path, 1, &attr_crlf_check) &&\n-\t\t(0 == attr_crlf_check.isset));\n+\tif (git_checkattr(path, 1, &attr_crlf_check))\n+\t\treturn -1;\n+\treturn attr_crlf_check.isset;\n }\n \n int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n {\n-\tif (git_path_is_binary(path))\n+\tswitch (git_path_check_crlf(path)) {\n+\tcase 0:\n \t\treturn 0;\n-\treturn autocrlf_to_git(path, bufp, sizep);\n+\tcase 1:\n+\t\treturn forcecrlf_to_git(path, bufp, sizep);\n+\tdefault:\n+\t\treturn autocrlf_to_git(path, bufp, sizep);\n+\t}\n }\n \n int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n {\n-\tif (git_path_is_binary(path))\n+\tswitch (git_path_check_crlf(path)) {\n+\tcase 0:\n \t\treturn 0;\n-\treturn autocrlf_to_working_tree(path, bufp, sizep);\n+\tcase 1:\n+\t\treturn forcecrlf_to_working_tree(path, bufp, sizep);\n+\tdefault:\n+\t\treturn autocrlf_to_working_tree(path, bufp, sizep);\n+\t}\n }\n-- \n1.5.1.1.815.g3e763\n"},{"id":"39420","messageId":"7vlkgt1bck.fsf_-_@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"7vr6ql1ben.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Fix 'diff' attribute semantics.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T23:12:11Z","receivedAt":"2007-04-15T23:12:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is in the same spirit as the previous one.  Earlier 'diff'\nmeant 'do the built-in binary heuristics and disable patch text\ngeneration based on it' while '!diff' meant 'do not guess, do\nnot generate patch text'.  There was no way to say 'do generate\npatch text even when the heuristics says it has NUL in it'.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n * And this is a companion patch to 'crlf' one.\n\n diff.c |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e4efb65..dcea405 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1069,8 +1069,9 @@ static int file_is_binary(struct diff_filespec *one)\n \n \tsetup_diff_attr_check(&attr_diff_check);\n \tif (!git_checkattr(one->path, 1, &attr_diff_check) &&\n-\t    (0 == attr_diff_check.isset))\n-\t\treturn 1;\n+\t    (0 <= attr_diff_check.isset))\n+\t\treturn !attr_diff_check.isset;\n+\n \tif (!one->data) {\n \t\tif (!DIFF_FILE_VALID(one))\n \t\t\treturn 0;\n-- \n1.5.1.1.815.g3e763\n"},{"id":"39425","messageId":"20070415233722.GA20222@hermes","threadId":"7639","inReplyTo":"7vr6ql1ben.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix 'crlf' attribute semantics.","fromName":"Tom Prince","fromEmail":"tom.prince@ualberta.net","sentAt":"2007-04-15T23:37:22Z","receivedAt":"2007-04-15T23:37:22Z","isPatch":true,"sender":{"key":"tom.prince@ualberta.net","avatar":"https://gravatar.com/avatar/a0ad19caee7618876339485106ec994f5202505eecd210ba5c0bd869feaa555a?d=mp&s=160"},"body":"On Sun, Apr 15, 2007 at 04:10:56PM -0700, Junio C Hamano wrote:\n> Earlier we said 'crlf lets the path go through core.autocrlf\n> process while !crlf disables it altogether'.  This fixes the\n> semantics to:\n\nThis change means there is no way to enable the automatic heuristics for a\nspecific pattern once it has been disable for a more generic pattern. Would it\nmake sense to make the attributes more than simply boolean?\n\n  Tom\n"},{"id":"39428","messageId":"7vd52519vj.fsf@assigned-by-dhcp.cox.net","threadId":"7639","inReplyTo":"20070415233722.GA20222@hermes","subject":"Re: [PATCH] Fix 'crlf' attribute semantics.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-15T23:44:00Z","receivedAt":"2007-04-15T23:44:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Prince <tom.prince@ualberta.net> writes:\n\n<offtopic>Please do not rudely point other people with\nmail-followup-to; I did not want to address this message to\nLinus but wanted to talk to YOU specifically, and you stole a\nfew seconds of my time, forcing me to rewrite my To: line\n</offtopic>\n\n> On Sun, Apr 15, 2007 at 04:10:56PM -0700, Junio C Hamano wrote:\n>> Earlier we said 'crlf lets the path go through core.autocrlf\n>> process while !crlf disables it altogether'.  This fixes the\n>> semantics to:\n>\n> This change means there is no way to enable the automatic heuristics for a\n> specific pattern once it has been disable for a more generic pattern. Would it\n> make sense to make the attributes more than simply boolean?\n\nI do not think that is a problem in practice.  Do not set\nsomething to \"false\" explicitly with a generic pattern, if you\nmight want to override it.\n"},{"id":"39471","messageId":"1176704470.5966.16.camel@localhost","threadId":"7639","inReplyTo":"7vd52519vj.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix 'crlf' attribute semantics.","fromName":"Raimund Bauer","fromEmail":"ray007@gmx.net","sentAt":"2007-04-16T06:21:10Z","receivedAt":"2007-04-16T06:21:10Z","isPatch":true,"sender":{"key":"ray007@gmx.net","avatar":null},"body":"On Sun, 2007-04-15 at 16:44 -0700, Junio C Hamano wrote:\n\n> > This change means there is no way to enable the automatic heuristics for a\n> > specific pattern once it has been disable for a more generic pattern. Would it\n> > make sense to make the attributes more than simply boolean?\n> \n> I do not think that is a problem in practice.  Do not set\n> something to \"false\" explicitly with a generic pattern, if you\n> might want to override it.\n\nI also don't think it's a problem, but I think it would generally be a\ngood idea to have values for attributes. So you can say\n\ncrlf=yes|no|auto\ndiff=yes|no|my-xml-diff|...\nmerge=3way|...\n\nIn the yes/no case we could keep the existing syntax on just add the\nattribute=othervalue for those that need more than a boolean decision.\n\n-- \nbest regards\n\n  Ray\n"}]}