{"thread":{"id":"23968","subject":"[PATCH v4 0/5] Patches to avoid reporting conversion changes.","startedAt":"2010-06-01T14:41:50Z","lastAt":"2010-06-10T19:55:55Z","messageCount":19,"participants":["Henrik Grubbström (Grubba)","Junio C Hamano","Henrik Grubbström","Jonathan Nieder","Finn Arne Gangstad"],"isPatch":true,"patchVersion":4,"patchTotal":5},"messages":[{"id":"142685","messageId":"cover.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":null,"subject":"[PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:50Z","receivedAt":"2010-06-01T14:41:50Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"This is the fourth go at having the git index keep track of\nthe conversion mode for blobs.\n\nThis is useful for repositorys not containing fully normalized files\n(eg containing CRLF's or expanded $Id$ strings), where a later attribute\nchange implies a conversion mode change. Without this set of patches\nthe user would need to recommit semantically unchanged files to get\na clean index.\n\nChanges since last time:\n\n  o The patch set has been rebased upon 0ed6711 (aka eb/core-eol),\n    to be able to take advantage of convert.c:get_output_conversion()\n    and convert.c:determine_action().\n\n  o As a consequence of using those functions, the on-disk format for\n    the CONV extension has changed slightly. The change should have\n    minimal impact, since the index will in most cases self-repair.\n\n  o The t0025-crlf-auto.sh tests have been updated to still test\n    the same behaviour.\n\nJunio: This should be close to what you envisioned in\n       <7vsk6qio1f.fsf@alter.siamese.dyndns.org>.\n\nHenrik Grubbström (Grubba) (5):\n  sha1_file: Add index_blob().\n  strbuf: Add strbuf_add_uint32().\n  cache: Keep track of conversion mode changes.\n  cache: Add index extension \"CONV\".\n  t/t0021: Test that conversion changes are detected.\n\n cache.h               |   12 ++++++\n convert.c             |   46 ++++++++++++++++++++++\n read-cache.c          |  102 ++++++++++++++++++++++++++++++++++++++++++++----\n sha1_file.c           |   19 +++++++++\n strbuf.h              |    4 ++\n t/t0021-conversion.sh |   54 ++++++++++++++++++++++++++\n t/t0025-crlf-auto.sh  |   20 ++++++----\n 7 files changed, 240 insertions(+), 17 deletions(-)\n"},{"id":"142687","messageId":"30b87a410786953aabedc73a68dd3f8f25d2a80a.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"[PATCH v4 1/5] sha1_file: Add index_blob().","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:51Z","receivedAt":"2010-06-01T14:41:51Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"When conversion attributes have changed, it\nis useful to be able to easily reconvert an\nexisting blob.\n\nSigned-off-by: Henrik Grubbström <grubba@grubba.org>\n---\nNo changes since v1.\n\n cache.h     |    1 +\n sha1_file.c |   19 +++++++++++++++++++\n 2 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex ebe71c7..004296d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -482,6 +482,7 @@ extern int ie_match_stat(const struct index_state *, struct cache_entry *, struc\n extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);\n \n extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);\n+extern int index_blob(unsigned char *dst_sha1, const unsigned char *src_sha1, int write_object, const char *path);\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);\n extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);\n extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 657825e..6e7b999 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2434,6 +2434,25 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,\n \treturn ret;\n }\n \n+int index_blob(unsigned char *dst_sha1, const unsigned char *src_sha1,\n+\t       int write_object, const char *path)\n+{\n+\tvoid *buf;\n+\tunsigned long buflen = 0;\n+\tint ret;\n+\n+\tmemcpy(dst_sha1, src_sha1, 20);\n+\tbuf = read_object_with_reference(src_sha1, typename(OBJ_BLOB),\n+\t\t\t\t\t &buflen, dst_sha1);\n+\tif (!buf)\n+\t\treturn 0;\n+\n+\tret = index_mem(dst_sha1, buf, buflen, write_object, OBJ_BLOB, path);\n+\tfree(buf);\n+\n+\treturn ret;\n+}\n+\n int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n \t     enum object_type type, const char *path)\n {\n-- \n1.7.0.4.369.g81e89\n"},{"id":"142686","messageId":"774d7a5951abd425bd0743b14e939eafebe41a4c.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"[PATCH v4 2/5] strbuf: Add strbuf_add_uint32().","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:52Z","receivedAt":"2010-06-01T14:41:52Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"Adds a convenience function for adding an unsigned 32bit\ninteger in network byte-order to a strbuf.\n\nSigned-off-by: Henrik Grubbström <grubba@grubba.org>\n---\n strbuf.h |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/strbuf.h b/strbuf.h\nindex fac2dbc..52cd71e 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -107,6 +107,10 @@ static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2) {\n \tstrbuf_grow(sb, sb2->len);\n \tstrbuf_add(sb, sb2->buf, sb2->len);\n }\n+static inline void strbuf_add_uint32(struct strbuf *sb, uint32_t val) {\n+\tval = htonl(val);\n+\tstrbuf_add(sb, &val, sizeof(val));\n+}\n extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);\n \n typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);\n-- \n1.7.0.4.369.g81e89\n"},{"id":"142688","messageId":"a8e30003e2cb4697316b441d26297eadccfe7f64.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"[PATCH v4 3/5] cache: Keep track of conversion mode changes.","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:53Z","receivedAt":"2010-06-01T14:41:53Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"The index now keeps track of the conversion mode that was active\nwhen the entry was created. This can be used to detect the most\ncommon cases of when the conversion mode has changed.\n\nSigned-off-by: Henrik Grubbström <grubba@grubba.org>\n---\nRebased on 0ed6711 (aka eb/core-eol).\n\nThe conversion mode flags are now based on the EOL_* set of flags,\nand to avoid code duplication convert.c:get_output_conversion() and\nconvert.c:determine_action() are used to determine the eol conversion\nmode.\n\nThe denormalized eol tests in t0025-crlf-auto.sh have been altered to\nstill test the intended eol conversion properties.\n\n cache.h              |   11 +++++++++++\n convert.c            |   46 ++++++++++++++++++++++++++++++++++++++++++++++\n read-cache.c         |   37 ++++++++++++++++++++++++++++++++++---\n t/t0025-crlf-auto.sh |   20 ++++++++++++--------\n 4 files changed, 103 insertions(+), 11 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 004296d..263f4f3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -151,10 +151,20 @@ struct cache_entry {\n \tunsigned int ce_size;\n \tunsigned int ce_flags;\n \tunsigned char sha1[20];\n+\tunsigned int ce_conv_flags;\n \tstruct cache_entry *next;\n \tchar name[FLEX_ARRAY]; /* more */\n };\n \n+#define CONV_EOL_CRLF\t\tEOL_CRLF\n+#define CONV_EOL_LF\t\tEOL_LF\n+#define CONV_GIT_LF\t\t0x0004\n+#define CONV_GIT_AUTO\t\t0x0008\n+#define CONV_IDENT\t\t0x0010\n+#define CONV_FILT\t\t0x0020\n+#define CONV_MASK\t\t0x003f\n+#define CONV_NORM_NEEDED\t0x010000\n+\n #define CE_NAMEMASK  (0x0fff)\n #define CE_STAGEMASK (0x3000)\n #define CE_EXTENDED  (0x4000)\n@@ -1016,6 +1026,7 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);\n \n /* convert.c */\n /* returns 1 if *dst was used */\n+extern unsigned int git_conv_flags(const char *path);\n extern int convert_to_git(const char *path, const char *src, size_t len,\n                           struct strbuf *dst, enum safe_crlf checksafe);\n extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\ndiff --git a/convert.c b/convert.c\nindex 80d80b1..387a7c7 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -681,6 +681,52 @@ enum action determine_action(enum action text_attr, enum eol eol_attr) {\n \treturn text_attr;\n }\n \n+unsigned int git_conv_flags(const char *path)\n+{\n+\tstruct git_attr_check check[5];\n+\tenum action action = CRLF_GUESS;\n+\tenum eol eol_attr = EOL_UNSET;\n+\tint ident = 0;\n+\tunsigned ret = 0;\n+\tstruct convert_driver *drv = NULL;\n+\n+\tsetup_convert_check(check);\n+\tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n+\t\taction = git_path_check_crlf(path, check + 4);\n+\t\tif (action == CRLF_GUESS)\n+\t\t\taction = git_path_check_crlf(path, check + 0);\n+\t\tident = git_path_check_ident(path, check + 1);\n+\t\tdrv = git_path_check_convert(path, check + 2);\n+\t\teol_attr = git_path_check_eol(path, check + 3);\n+\t}\n+\n+\tret = get_output_conversion(action);\n+\n+\taction = determine_action(action, eol_attr);\n+\n+\tswitch(action) {\n+\tcase CRLF_BINARY:\n+\t\tbreak;\n+\tcase CRLF_GUESS:\n+\t\tif (auto_crlf == AUTO_CRLF_FALSE)\n+\t\t\tbreak;\n+\t\tret |= CONV_GIT_AUTO;\n+\t\tbreak;\n+\tcase CRLF_AUTO:\n+\t\tret |= CONV_GIT_AUTO|CONV_GIT_LF;\n+\t\tbreak;\n+\tdefault:\n+\t\tret |= CONV_GIT_LF;\n+\t\tbreak;\n+\t}\n+\n+\tif (ident)\n+\t\tret |= CONV_IDENT;\n+\tif (drv)\n+\t\tret |= CONV_FILT;\n+\treturn ret;\n+}\n+\n int convert_to_git(const char *path, const char *src, size_t len,\n                    struct strbuf *dst, enum safe_crlf checksafe)\n {\ndiff --git a/read-cache.c b/read-cache.c\nindex f1f789b..eeda928 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -88,12 +88,38 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n {\n \tint match = -1;\n-\tint fd = open(ce->name, O_RDONLY);\n-\n+\tint fd;\n+\tunsigned char norm_sha1[20];\n+\tunsigned int conv_flags = git_conv_flags(ce->name);\n+\tconst unsigned char *cmp_sha1 = ce->sha1;\n+\n+\tif ((conv_flags ^ ce->ce_conv_flags) & CONV_MASK) {\n+\t\tif (ce->ce_conv_flags & CONV_NORM_NEEDED) {\n+\t\t\t/* Smudge the entry since it was only correct\n+\t\t\t * for the old conversion mode. */\n+\t\t\tce->ce_size = 0;\n+\t\t}\n+\t\tce->ce_conv_flags = conv_flags;\n+\t} else\n+\t\tconv_flags = ce->ce_conv_flags & CONV_NORM_NEEDED;\n+\n+\tif (conv_flags) {\n+\t\tindex_blob(norm_sha1, ce->sha1, 0, ce->name);\n+\t\tif (!(conv_flags & CONV_NORM_NEEDED) &&\n+\t\t    hashcmp(norm_sha1, ce->sha1)) {\n+\t\t\tce->ce_conv_flags = conv_flags | CONV_NORM_NEEDED;\n+\t\t\t/* Smudge the entry since we don't know\n+\t\t\t * the correct value. */\n+\t\t\tce->ce_size = 0;\n+\t\t}\n+\t\tcmp_sha1 = norm_sha1;\n+\t}\n+\t\n+\tfd = open(ce->name, O_RDONLY);\n \tif (fd >= 0) {\n \t\tunsigned char sha1[20];\n \t\tif (!index_fd(sha1, fd, st, 0, OBJ_BLOB, ce->name))\n-\t\t\tmatch = hashcmp(sha1, ce->sha1);\n+\t\t\tmatch = hashcmp(sha1, cmp_sha1);\n \t\t/* index_fd() closed the file descriptor already */\n \t}\n \treturn match;\n@@ -227,6 +253,11 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \t\tchanged |= INODE_CHANGED;\n #endif\n \n+\t/* ce_size can not be trusted if the conversion mode has changed. */\n+\tif ((ce->ce_mode & S_IFMT) == S_IFREG &&\n+\t    ((ce->ce_conv_flags ^ git_conv_flags(ce->name)) & CONV_MASK))\n+\t\treturn changed;\n+\n \tif (ce->ce_size != (unsigned int) st->st_size)\n \t\tchanged |= DATA_CHANGED;\n \ndiff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh\nindex f5f67a6..1bcf3dd 100755\n--- a/t/t0025-crlf-auto.sh\n+++ b/t/t0025-crlf-auto.sh\n@@ -47,9 +47,10 @@ test_expect_success 'crlf=true causes a CRLF file to be normalized' '\n \tgit read-tree --reset -u HEAD &&\n \n \t# Note, \"normalized\" means that git will normalize it if added\n+\techo >>two &&\n \thas_cr two &&\n-\ttwodiff=`git diff two` &&\n-\ttest -n \"$twodiff\"\n+\ttwodiff=`git diff --numstat two | cut -f1` &&\n+\ttest -n \"$twodiff\" -a \"$twodiff\" -gt 1\n '\n \n test_expect_success 'text=true causes a CRLF file to be normalized' '\n@@ -59,9 +60,10 @@ test_expect_success 'text=true causes a CRLF file to be normalized' '\n \tgit read-tree --reset -u HEAD &&\n \n \t# Note, \"normalized\" means that git will normalize it if added\n+\techo >>two &&\n \thas_cr two &&\n-\ttwodiff=`git diff two` &&\n-\ttest -n \"$twodiff\"\n+\ttwodiff=`git diff --numstat two | cut -f1` &&\n+\ttest -n \"$twodiff\" -a \"$twodiff\" -gt 1\n '\n \n test_expect_success 'eol=crlf gives a normalized file CRLFs with autocrlf=false' '\n@@ -107,11 +109,12 @@ test_expect_success 'autocrlf=true does not normalize CRLF files' '\n \tgit read-tree --reset -u HEAD &&\n \n \thas_cr one &&\n+\techo >>two &&\n \thas_cr two &&\n \tonediff=`git diff one` &&\n-\ttwodiff=`git diff two` &&\n+\ttwodiff=`git diff --numstat two | cut -f1` &&\n \tthreediff=`git diff three` &&\n-\ttest -z \"$onediff\" -a -z \"$twodiff\" -a -z \"$threediff\"\n+\ttest -z \"$onediff\" -a -n \"$twodiff\" -a -z \"$threediff\" -a \"$twodiff\" -eq 1\n '\n \n test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' '\n@@ -122,11 +125,12 @@ test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' '\n \tgit read-tree --reset -u HEAD &&\n \n \thas_cr one &&\n+\techo >>two &&\n \thas_cr two &&\n \tonediff=`git diff one` &&\n-\ttwodiff=`git diff two` &&\n+\ttwodiff=`git diff --numstat two | cut -f1` &&\n \tthreediff=`git diff three` &&\n-\ttest -z \"$onediff\" -a -n \"$twodiff\" -a -z \"$threediff\"\n+\ttest -z \"$onediff\" -a -n \"$twodiff\" -a -z \"$threediff\" -a \"$twodiff\" -gt 1\n '\n \n test_expect_success 'text=auto, autocrlf=true does not normalize binary files' '\n-- \n1.7.0.4.369.g81e89\n"},{"id":"142690","messageId":"6b41ac9d06d0180b3a03b99449f8307530804eb9.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"[PATCH v4 4/5] cache: Add index extension \"CONV\".","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:54Z","receivedAt":"2010-06-01T14:41:54Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"The index can now store and retrieve the ce_conv_flags data.\n\nSigned-off-by: Henrik Grubbström <grubba@grubba.org>\n---\nThe on disk format has changed slightly due to the changes in the\nprevious patch, but the change should have minimal impact, since\nit typically only would force a one-time reindex.\n\n read-cache.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 59 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex eeda928..630e001 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -27,6 +27,7 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall\n #define CACHE_EXT(s) ( (s[0]<<24)|(s[1]<<16)|(s[2]<<8)|(s[3]) )\n #define CACHE_EXT_TREE 0x54524545\t/* \"TREE\" */\n #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* \"REUC\" */\n+#define CACHE_EXT_CONV 0x434f4e56\t/* \"CONV\" */\n \n struct index_state the_index;\n \n@@ -1209,6 +1210,34 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)\n \treturn 0;\n }\n \n+/* The on disk format is the default conversion flags followed\n+ * by alternating cache entry numbers and corresponding flags.\n+ */\n+static int conv_read(struct cache_entry **cache, unsigned int entries,\n+\t\t     const unsigned int *data, unsigned long sz)\n+{\n+\tunsigned int entry_no;\n+\tunsigned int default_conv_flags;\n+\tif (sz < sizeof(*data))\n+\t\treturn 0;\n+\tdefault_conv_flags = ntohl(*data);\n+\tdata++;\n+\tsz -= sizeof(*data);\n+\tif (default_conv_flags) {\n+\t\tfor (entry_no = 0; entry_no < entries; entry_no++)\n+\t\t\tcache[entry_no]->ce_conv_flags = default_conv_flags;\n+\t}\n+\twhile (sz >= 2*sizeof(*data)) {\n+\t\tentry_no = ntohl(*data);\n+\t\tdata++;\n+\t\tif (entry_no >= entries) break;\n+\t\tcache[entry_no]->ce_conv_flags = ntohl(*data);\n+\t\tdata++;\n+\t\tsz -= 2*sizeof(*data);\n+\t}\n+\treturn 0;\n+}\n+\n static int read_index_extension(struct index_state *istate,\n \t\t\t\tconst char *ext, void *data, unsigned long sz)\n {\n@@ -1219,6 +1248,9 @@ static int read_index_extension(struct index_state *istate,\n \tcase CACHE_EXT_RESOLVE_UNDO:\n \t\tistate->resolve_undo = resolve_undo_read(data, sz);\n \t\tbreak;\n+\tcase CACHE_EXT_CONV:\n+\t\treturn conv_read(istate->cache, istate->cache_nr, data, sz);\n+\t\tbreak;\n \tdefault:\n \t\tif (*ext < 'A' || 'Z' < *ext)\n \t\t\treturn error(\"index uses %.4s extension, which we do not understand\",\n@@ -1542,6 +1574,15 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)\n \t}\n }\n \n+static void conv_write(struct strbuf *sb, const struct cache_entry *ce,\n+\t\t       int entry_no)\n+{\n+\tunsigned int entry[2];\n+\tentry[0] = htonl(entry_no);\n+\tentry[1] = htonl(ce->ce_conv_flags);\n+\tstrbuf_add(sb, &entry, sizeof(entry));\n+}\n+\n static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce)\n {\n \tint size = ondisk_ce_size(ce);\n@@ -1577,10 +1618,12 @@ int write_index(struct index_state *istate, int newfd)\n {\n \tgit_SHA_CTX c;\n \tstruct cache_header hdr;\n-\tint i, err, removed, extended;\n+\tint i, j, err, removed, extended;\n \tstruct cache_entry **cache = istate->cache;\n \tint entries = istate->cache_nr;\n \tstruct stat st;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tunsigned int default_conv_flags;\n \n \tfor (i = removed = extended = 0; i < entries; i++) {\n \t\tif (cache[i]->ce_flags & CE_REMOVE)\n@@ -1603,7 +1646,10 @@ int write_index(struct index_state *istate, int newfd)\n \tif (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)\n \t\treturn -1;\n \n-\tfor (i = 0; i < entries; i++) {\n+\tdefault_conv_flags = git_conv_flags(\"\");\n+\tstrbuf_add_uint32(&sb, default_conv_flags);\n+\n+\tfor (i = j = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n \t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\tcontinue;\n@@ -1611,12 +1657,21 @@ int write_index(struct index_state *istate, int newfd)\n \t\t\tce_smudge_racily_clean_entry(ce);\n \t\tif (ce_write_entry(&c, newfd, ce) < 0)\n \t\t\treturn -1;\n+\t\tif (ce->ce_conv_flags != default_conv_flags)\n+\t\t\tconv_write(&sb, ce, j);\n+\t\tj++;\n \t}\n \n \t/* Write extension data here */\n+\tif (default_conv_flags || sb.len > sizeof(default_conv_flags)) {\n+\t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_CONV, sb.len) < 0\n+\t\t\t|| ce_write(&c, newfd, sb.buf, sb.len) < 0;\n+\t\tstrbuf_release(&sb);\n+\t\tif (err)\n+\t\t\treturn -1;\n+\t} else\n+\t\tstrbuf_release(&sb);\n \tif (istate->cache_tree) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\n \t\tcache_tree_write(&sb, istate->cache_tree);\n \t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_TREE, sb.len) < 0\n \t\t\t|| ce_write(&c, newfd, sb.buf, sb.len) < 0;\n@@ -1625,8 +1680,6 @@ int write_index(struct index_state *istate, int newfd)\n \t\t\treturn -1;\n \t}\n \tif (istate->resolve_undo) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\n \t\tresolve_undo_write(&sb, istate->resolve_undo);\n \t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_RESOLVE_UNDO,\n \t\t\t\t\t     sb.len) < 0\n-- \n1.7.0.4.369.g81e89\n"},{"id":"142689","messageId":"99a6d5d56ff1b6a9b5005b782044939b1cfc1c5a.1275309129.git.grubba@grubba.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"[PATCH v4 5/5] t/t0021: Test that conversion changes are detected.","fromName":"Henrik Grubbström (Grubba)","fromEmail":"grubba@grubba.org","sentAt":"2010-06-01T14:41:55Z","receivedAt":"2010-06-01T14:41:55Z","isPatch":true,"sender":{"key":"grubba@grubba.org","avatar":"https://avatars.githubusercontent.com/u/1169458?v=4"},"body":"Signed-off-by Henrik Grubbström <grubba@grubba.org>\n---\n t/t0021-conversion.sh |   54 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 54 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex 6cb8d60..6d41ee7 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -89,4 +89,58 @@ test_expect_success expanded_in_repo '\n \tcmp expanded-keywords expected-output\n '\n \n+# Check that files that have had their canonical representation\n+# changed since being checked in aren't reported as modified\n+# directly after being checked out.\n+test_expect_success keywords_not_modified '\n+\t{\n+\t\techo \"File with foreign keywords\"\n+\t\techo \"\\$Id\\$\"\n+\t\techo \"\\$Id: NoTerminatingSymbol\"\n+\t\techo \"\\$Id: Foreign Commit With Spaces \\$\"\n+\t\techo \"\\$Id: GitCommitId \\$\"\n+\t\techo \"\\$Id: NoTerminatingSymbolAtEOF\"\n+\t} > expanded-keywords2 &&\n+\n+\tgit add expanded-keywords2 &&\n+\tgit commit -m \"File with keywords expanded\" &&\n+\n+\techo \"expanded-keywords2 ident\" >> .gitattributes &&\n+\n+\trm -f expanded-keywords2 &&\n+\tgit checkout -- expanded-keywords2 &&\n+\n+\ttest \"x`git status --porcelain -- expanded-keywords2`\" = x\n+'\n+\n+# Test detection of CRLF conversion changes CRLF ==> LF.\n+test_expect_success crlf_conversion_change_crlf_to_lf '\n+(\n+\t# step 0. a blob with CRLF\n+\tgit init one && cd one &&\n+\techo -e \"a quick brown fox\\015\" >kuzu &&\n+\tgit add kuzu && git commit -m kuzu &&\n+\t# step 1. you want CRLF in work area, LF in repository\n+\tgit config core.autocrlf true &&\n+\t# step 2. user edit and revert.\n+\ttouch kuzu &&\n+\tgit update-index --refresh\n+)\n+'\n+\n+# Test detection of CRLF conversion changes LF ==> CRLF.\n+test_expect_success crlf_conversion_change_lf_to_crlf '\n+(\n+\t# step 0 & 1. a project with LF ending\n+\tgit init two && cd two &&\n+\techo a quick brown fox >kuzu &&\n+\tgit add kuzu && git commit -m kuzu &&\n+\t# step 2. you want CRLF in your work area\n+\techo -e \"a quick brown fox\\015\" >kuzu &&\n+\tgit config core.autocrlf true &&\n+\t# step 3. oops, refresh\n+\tgit update-index --refresh\n+)\n+'\n+\n test_done\n-- \n1.7.0.4.369.g81e89\n"},{"id":"142764","messageId":"7vfx16oxmz.fsf@alter.siamese.dyndns.org","threadId":"23968","inReplyTo":"cover.1275309129.git.grubba@grubba.org","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-02T04:40:20Z","receivedAt":"2010-06-02T04:40:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Henrik Grubbström (Grubba)\" <grubba@grubba.org> writes:\n\n> This is useful for repositorys not containing fully normalized files\n> (eg containing CRLF's or expanded $Id$ strings), where a later attribute\n> change implies a conversion mode change. Without this set of patches\n> the user would need to recommit semantically unchanged files to get\n> a clean index.\n\nA more fundamental (or perhaps \"silly\") question is if that \"user would\nneed to\" is necessarily a bad thing.  If the user wants to cleanse such\nabnormality in the recorded blobs, shouldn't there be a conscious act,\niow, a commit that records that \"I am fixing that mistake, and from now\non, the recorded data are normalized\"?\n\nPerhaps I am missing something very trivial that you have already\nexplained to the list but I forgot amid my moving and other confusion, and\nif that is the case I apologize in advance ;-).\n"},{"id":"142908","messageId":"Pine.GSO.4.63.1006031543340.22466@shipon.roxen.com","threadId":"23968","inReplyTo":"7vfx16oxmz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-03T16:00:38Z","receivedAt":"2010-06-03T16:00:38Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"On Tue, 1 Jun 2010, Junio C Hamano wrote:\n\n> \"Henrik Grubbström (Grubba)\" <grubba@grubba.org> writes:\n>\n>> This is useful for repositorys not containing fully normalized files\n>> (eg containing CRLF's or expanded $Id$ strings), where a later attribute\n>> change implies a conversion mode change. Without this set of patches\n>> the user would need to recommit semantically unchanged files to get\n>> a clean index.\n>\n> A more fundamental (or perhaps \"silly\") question is if that \"user would\n> need to\" is necessarily a bad thing.  If the user wants to cleanse such\n> abnormality in the recorded blobs, shouldn't there be a conscious act,\n> iow, a commit that records that \"I am fixing that mistake, and from now\n> on, the recorded data are normalized\"?\n\nI believe that users typically aren't interested in if data in the \nrepository is on normalized form or not (witness the autocrlf=true \ndiscussion a few weeks ago, where one of the main complaints was\nthat it required a renormalization (which fg/autocrlf attempts to\nsolve for that specific case by not normalizing)), as long as they\nget the expected content on checkout.\n\nThis set of patches allows for an incremental, on-demand normalization.\nEg the user could switch the attributes for a group of files from\n\n   *.bat -crlf\n\n(let's assume *.bat files use crlf linebreaks) to\n\n   *.bat -crlf text eol=crlf\n\nand then have git normalize the individual files when there's actually a \nsemantic reason for a change. With the current eb/core-eol patches this\nchange would cause a dirty index on checkout. The user who committed the \nchange however has a clean index until any of the files affected is \ntouched.\n\nIn my case, I have repositories containing files both requiring crlf and \nlf line endings, and additionally have expanded $Id$-strings that I want \nchanged on first semantic change (but not before). To be able to use a\ngit binary without this patchset I'd have to do a\n\n   git commit -a -m 'Normalized'\n\nas the first thing after a checkout.\n\nI could of course add a config option to control the behaviour (hmm, or \nmaybe an attribute?).\n\n> Perhaps I am missing something very trivial that you have already\n> explained to the list but I forgot amid my moving and other confusion, and\n> if that is the case I apologize in advance ;-).\n\nNo problem.\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"142939","messageId":"20100604005603.GA25806@progeny.tock","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006031543340.22466@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-04T00:56:03Z","receivedAt":"2010-06-04T00:56:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Henrik,\n\nHenrik Grubbström wrote:\n\n> I believe that users typically aren't interested in if data in the\n> repository is on normalized form or not (witness the autocrlf=true\n> discussion a few weeks ago, where one of the main complaints was\n> that it required a renormalization (which fg/autocrlf attempts to\n> solve for that specific case by not normalizing)), as long as they\n> get the expected content on checkout.\n\nI agree.  (In the case of autocrlf, it is also not very easy to\nrenormalize.  The usual recommendation I have seen is \"git rm -r \\\n--cached . && git add .\", which is not exactly simple.)\n\n> This set of patches allows for an incremental, on-demand normalization.\n> Eg the user could switch the attributes for a group of files from\n>\n>   *.bat -crlf\n>\n> (let's assume *.bat files use crlf linebreaks) to\n>\n>   *.bat -crlf text eol=crlf\n>\n> and then have git normalize the individual files when there's\n> actually a semantic reason for a change.\n\n... but if I understand correctly, I don’t agree with this at all.\n\nImagine someone with an old copy of git that does not do\nnormalization.  If you convert everything at once, she sees a single\nenormous, semantically uninteresting cleanup patch (and she can check\nthe result with ‘diff -w’ or sed if suspicious).  If you wait for some\nreal change to piggy-back onto, on the other hand, then the per-file\nnormalization patches will make it hard to find what changed.\n\nOf course, very few people use such old copies of git.  The real\nproblem is that git itself sees what this person would see; you are\nasking to slow down everyone who tries to use diff or blame on your\nrepository by implicitly requiring the -w option.\n\n> In my case, I have repositories containing files both requiring crlf\n> and lf line endings, and additionally have expanded $Id$-strings\n> that I want changed on first semantic change (but not before). To be\n> able to use a\n> git binary without this patchset I'd have to do a\n> \n>   git commit -a -m 'Normalized'\n> \n> as the first thing after a checkout.\n\nThe Right Thing would be to not set the relevant attributes until it\nis time for the file to be normalized.  I can understand that that\nmight be hard and could require tool support.\n\nThis is not an argument against your patches, since I haven’t read\nthem (for all I know, they make everything better :)).\n\nRegards,\nJonathan\n"},{"id":"142963","messageId":"Pine.GSO.4.63.1006041212200.27465@shipon.roxen.com","threadId":"23968","inReplyTo":"20100604005603.GA25806@progeny.tock","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-04T11:59:23Z","receivedAt":"2010-06-04T11:59:23Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"On Thu, 3 Jun 2010, Jonathan Nieder wrote:\n\n> Hi Henrik,\n\nHi.\n\n> Henrik Grubbström wrote:\n>\n>> I believe that users typically aren't interested in if data in the\n>> repository is on normalized form or not (witness the autocrlf=true\n>> discussion a few weeks ago, where one of the main complaints was\n>> that it required a renormalization (which fg/autocrlf attempts to\n>> solve for that specific case by not normalizing)), as long as they\n>> get the expected content on checkout.\n>\n> I agree.  (In the case of autocrlf, it is also not very easy to\n> renormalize.  The usual recommendation I have seen is \"git rm -r \\\n> --cached . && git add .\", which is not exactly simple.)\n>\n>> This set of patches allows for an incremental, on-demand normalization.\n[...]\n> ... but if I understand correctly, I don't agree with this at all.\n>\n> Imagine someone with an old copy of git that does not do\n> normalization.  If you convert everything at once, she sees a single\n> enormous, semantically uninteresting cleanup patch (and she can check\n> the result with 'diff -w' or sed if suspicious).  If you wait for some\n> real change to piggy-back onto, on the other hand, then the per-file\n> normalization patches will make it hard to find what changed.\n\nThis seems more like an argument against repositories where \nrenormalizations have occurred, than against the feature as such.\n\n> Of course, very few people use such old copies of git.  The real\n> problem is that git itself sees what this person would see; you are\n> asking to slow down everyone who tries to use diff or blame on your\n> repository by implicitly requiring the -w option.\n\nWell, diff and blame would be confused by a crlf renormalization \nregardless of whether the renormalization was piggy-backed or not. \nI haven't looked at the implementation of blame, but it was possible \nto reduce the confusion in the diff case by letting it normalize the \nold blob according to the current set of attributes (this was part of \nmy original patch set, but Junio didn't like the feature).\n\n> The Right Thing would be to not set the relevant attributes until it\n> is time for the file to be normalized.  I can understand that that\n> might be hard and could require tool support.\n\nTrue, but then the .gitattributes file would start to resemble\na manifest file for the entire repository, which would be a\npain to maintain.\n\nI did do an experiment with a .gitattributes file like:\n\n   *.c crlf ident\n   [attr]foreign_ident -ident block_commit=Remove-foreign_ident-attribute-before-commit.\n   # A list of files that haven't been changed since import follows.\n   /foo.c foreign_ident\n   /bar.c foreign_ident\n   # etc\n\nand a suitable pre-commit hook that looked at the block_commit\nattribute, but there were two problems in addition to the long\nlist of files in the .gitattributes file:\n\n   * The attributes file parsing was broken (recently fixed in the\n     master branch), and the above actually caused foo.c and bar.c\n     to have the ident attribute.\n\n   * Hooks are not copied by git clone. Support for copying of hooks\n     to non-POSIX-like systems is not something I'd like to attempt.\n\nThe latter problem could in this case be solved by adding support\nfor the block_commit attribute to the core of git, but it doesn't\nseem like something most users would understand how to use, and I\ndoubt that Junio would accept such a patch.\n\n> This is not an argument against your patches, since I haven't read\n> them (for all I know, they make everything better :)).\n\n:-)\n\nI don't believe that they make everything better, but that they're\na step on the way.\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"143001","messageId":"20100604194201.GB21492@progeny.tock","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006041212200.27465@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-04T19:42:01Z","receivedAt":"2010-06-04T19:42:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Henrik Grubbström wrote:\n> On Thu, 3 Jun 2010, Jonathan Nieder wrote:\n\n>> If you wait for some\n>> real change to piggy-back onto, on the other hand, then the per-file\n>> normalization patches will make it hard to find what changed.\n>\n> This seems more like an argument against repositories where\n> renormalizations have occurred, than against the feature as such.\n\nNo, it is an argument against making the process of renormalization\nmore painful than it has to be (and against piggy-backing in general).\nIt is kindest to have a flag day and yank the carriage returns off all\nat once like a bandage.\n\n> Well, diff and blame would be confused by a crlf renormalization\n> regardless of whether the renormalization was piggy-backed or not.\n\nOnly if they cross the revision where renormalization occurred.\n\n> I did do an experiment with a .gitattributes file like:\n> \n>   *.c crlf ident\n>   [attr]foreign_ident -ident block_commit=Remove-foreign_ident-attribute-before-commit.\n>   # A list of files that haven't been changed since import follows.\n>   /foo.c foreign_ident\n>   /bar.c foreign_ident\n>   # etc\n\nThis looks more sane.  Ident strings usually touch only a few lines.\n\n> there were two problems in addition to the long\n> list of files in the .gitattributes file:\n> \n>   * The attributes file parsing was broken (recently fixed in the\n>     master branch), and the above actually caused foo.c and bar.c\n>     to have the ident attribute.\n\nWouldn’t something like\n\n /foo.c -ident has_foreign_ident\n\nwork?  (Thanks for fixing that attributes macro processing bug, btw.)\n\n>   * Hooks are not copied by git clone. Support for copying of hooks\n>     to non-POSIX-like systems is not something I'd like to attempt.\n\nCan’t you include a hooks/pre-commit file and a HACKING file: \"copy\nthis file to .git/hooks if you want your patches to be accepted\"?\n\nThanks for your hard work,\nJonathan\n"},{"id":"143076","messageId":"Pine.GSO.4.63.1006061143000.27465@shipon.roxen.com","threadId":"23968","inReplyTo":"20100604194201.GB21492@progeny.tock","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-06T10:50:08Z","receivedAt":"2010-06-06T10:50:08Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"On Fri, 4 Jun 2010, Jonathan Nieder wrote:\n\n> Henrik Grubbström wrote:\n>> On Thu, 3 Jun 2010, Jonathan Nieder wrote:\n>\n>>> If you wait for some\n>>> real change to piggy-back onto, on the other hand, then the per-file\n>>> normalization patches will make it hard to find what changed.\n>>\n>> This seems more like an argument against repositories where\n>> renormalizations have occurred, than against the feature as such.\n>\n> No, it is an argument against making the process of renormalization\n> more painful than it has to be (and against piggy-backing in general).\n> It is kindest to have a flag day and yank the carriage returns off all\n> at once like a bandage.\n\nIn the crlf case, yes, probably. In the ident case there might be good \nreasons to want to have the ident strings stay unmodified as long as \npossible (since otherwise there'll be two ident strings that identify\nthe same code).\n\nCurrently (as I believe you know), git has no detection of when the \nconversion mode for a file has changed, and it might even take a while \nbefore the users notice that the repository is not normalized. eg:\n\n   0) There's a repository with some files containing crlf line endings.\n\n   1) User A notices that git now has native support for crlf\n      line endings, and adds the attribute eol=crlf for the\n      affected files.\n\n   2) User A does a git status, sees that .gitattributes is\n      modified, and commits it.\n\n   3) User A does a new git status, and has a clean index.\n\n   4) User B who has been developing on the project for a while\n      does a git pull from user A, and gets the new .gitattributes.\n\n   5) User B does a git status, and has a clean index.\n\n   6) User C is new to the project and does an initial git clone,\n      and ends up with a dirty index.\n\nI believe we both agree that the above is undesireable behaviour. The \nabove behaviour also means that it's likely that there are repositorys\nout there which contain unnormalized files.\n\nWhat my patch set achieves is that user C above also gets a clean index.\n\nWhat it seems you want is that user A above should have all files that got \ndenormalized by the attribute change marked dirty at 2 (and 3).\n\nWith a minor change of the read-cache.c:ce_compare_data() patch (returning \n1 if conv_flags has CONV_NORM_NEEDED), you should get the behaviour you \nwant (all files which are unnormalized in the repository will be dirty).\n\nAs I believe both behaviours may be desireable a config option and/or \nattribute is needed. Any suggestions for a name (and default value)?\n\n>> Well, diff and blame would be confused by a crlf renormalization\n>> regardless of whether the renormalization was piggy-backed or not.\n>\n> Only if they cross the revision where renormalization occurred.\n\nWhich is true in both cases.\n\n> Wouldn't something like\n>\n> /foo.c -ident has_foreign_ident\n>\n> work?\n\nYes, but it sort of reduces the usefulness of macros if you need to expand \nthem by hand... :-)\n\nBetter to fix the bug and wait for the fix to enter a released version.\n\n>>   * Hooks are not copied by git clone. Support for copying of hooks\n>>     to non-POSIX-like systems is not something I'd like to attempt.\n>\n> Can't you include a hooks/pre-commit file and a HACKING file: \"copy\n> this file to .git/hooks if you want your patches to be accepted\"?\n\nYes, but you still have to remember to do it everytime you clone the \nrepository.\n\nThanks for taking the time to discuss the problem.\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"143150","messageId":"20100607085947.GA3924@pvv.org","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006061143000.27465@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2010-06-07T08:59:47Z","receivedAt":"2010-06-07T08:59:47Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Sun, Jun 06, 2010 at 12:50:08PM +0200, Henrik Grubbström wrote:\n[...]\n>\n> Currently (as I believe you know), git has no detection of when the  \n> conversion mode for a file has changed, and it might even take a while  \n> before the users notice that the repository is not normalized. eg:\n>\n>   0) There's a repository with some files containing crlf line endings.\n>\n>   1) User A notices that git now has native support for crlf\n>      line endings, and adds the attribute eol=crlf for the\n>      affected files.\n>\n>   2) User A does a git status, sees that .gitattributes is\n>      modified, and commits it.\n\nI think it would be best if git at this time could decide that the\naffected files also become dirty. The ideal commit is one that\nboth alters the .gitattributes _and_ the affected files at the same\ntime, and git should make it easy to create that commit.\n\n> [...]\n>   6) User C is new to the project and does an initial git clone,\n>      and ends up with a dirty index.\n\nAnd the reason for this is mostly that unless you perform some special\nactions, you will commit attributes and contents that are mismatched.\n\nIn your suggested mode, whay would happen if you did this:\n\n$ git clone ......  (which has files that are \"wrong\" wrt line endings and\nattributes for some .c files)\n$ touch *.c\n\nWould it still believe all *.c files were clean? Does it require an\nactual other change at the same time to allow you to normalize the\nfile? That would be detrimental I think.  Changing newlines is best\ndone as a separate commit, intermingling newline changes and real\nchanges in the same commmit is not where you want to go.\n\nHowever, for your ID string you obviously want this behaviour. I'm\nguessing that hook is alreasy set up so that if you just touch the\nfile, it will still be treated as unmodified?\n\n>\n> What my patch set achieves is that user C above also gets a clean index.\n>\n> What it seems you want is that user A above should have all files that \n> got denormalized by the attribute change marked dirty at 2 (and 3).\n\nThat would indeed be a very welcome change.\n\n> As I believe both behaviours may be desireable a config option and/or  \n> attribute is needed. Any suggestions for a name (and default value)?\n\nI think the default behaviour should be to mark files dirty if there\nare ANY attribute changes that could cause content changes done to\nthem at all. I'm not sure that is exactly what your patch series is\nallowing us to track though?\n\nJust to be clear:\n\nIf you add this to your .gitattributes\n\n*.c eol=lf\n\nI think it would be very helpful if git then would treat all .c files\nas \"stat-dirty\" the next time it updates its index.\n\nA for config variables, what about:\n\ncore.rereadOnAttributeChanges = [true]/false    (default = true)\n\nWhich makes some sense for detecting it in 2, but not so much for\nignoring it in 6.\n\n- Finn Arne\n"},{"id":"143174","messageId":"Pine.GSO.4.63.1006071726170.22466@shipon.roxen.com","threadId":"23968","inReplyTo":"20100607085947.GA3924@pvv.org","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-07T16:37:56Z","receivedAt":"2010-06-07T16:37:56Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"\nHi Finn,\n\nOn Mon, 7 Jun 2010, Finn Arne Gangstad wrote:\n\n> On Sun, Jun 06, 2010 at 12:50:08PM +0200, Henrik Grubbström wrote:\n> [...]\n>>\n>> Currently (as I believe you know), git has no detection of when the\n>> conversion mode for a file has changed, and it might even take a while\n>> before the users notice that the repository is not normalized. eg:\n>>\n>>   0) There's a repository with some files containing crlf line endings.\n>>\n>>   1) User A notices that git now has native support for crlf\n>>      line endings, and adds the attribute eol=crlf for the\n>>      affected files.\n>>\n>>   2) User A does a git status, sees that .gitattributes is\n>>      modified, and commits it.\n>\n> I think it would be best if git at this time could decide that the\n> affected files also become dirty. The ideal commit is one that\n> both alters the .gitattributes _and_ the affected files at the same\n> time, and git should make it easy to create that commit.\n\nI agree in the case of newly added attributes. In the case of repositories \nalready containing unnormalized files this however leads to problems.\neg\n\n   Consider the case above, but a while later when the repository has been\n   fixed at HEAD. If an old version from before the normalization is\n   checked out, the index will once again become dirty, which means that\n   git will refuse the user to check out some other version unless the\n   --force flag is given. Excessive use of --force is not a good thing.\n   If the user is aware of the problem, and checking out old versions is\n   a common operation, toggling the suggested option might be a good\n   solution.\n\n>> [...]\n>>   6) User C is new to the project and does an initial git clone,\n>>      and ends up with a dirty index.\n>\n> And the reason for this is mostly that unless you perform some special\n> actions, you will commit attributes and contents that are mismatched.\n>\n> In your suggested mode, whay would happen if you did this:\n>\n> $ git clone ......  (which has files that are \"wrong\" wrt line endings and\n> attributes for some .c files)\n> $ touch *.c\n>\n> Would it still believe all *.c files were clean? Does it require an\n> actual other change at the same time to allow you to normalize the\n> file? That would be detrimental I think.  Changing newlines is best\n> done as a separate commit, intermingling newline changes and real\n> changes in the same commmit is not where you want to go.\n\nWith the patch set as submitted, it would still claim that the files \nare clean. With the additional two-liner I suggested to Jonathan, the \nfiles would be dirty.\n\n> However, for your ID string you obviously want this behaviour. I'm\n> guessing that hook is alreasy set up so that if you just touch the\n> file, it will still be treated as unmodified?\n\nYes.\n\n>> What my patch set achieves is that user C above also gets a clean index.\n>>\n>> What it seems you want is that user A above should have all files that\n>> got denormalized by the attribute change marked dirty at 2 (and 3).\n>\n> That would indeed be a very welcome change.\n>\n>> As I believe both behaviours may be desireable a config option and/or\n>> attribute is needed. Any suggestions for a name (and default value)?\n>\n> I think the default behaviour should be to mark files dirty if there\n> are ANY attribute changes that could cause content changes done to\n> them at all. I'm not sure that is exactly what your patch series is\n> allowing us to track though?\n\nWhat it does, is to track the attributes affecting conversion that \nwere active when the entries in the index were last updated, and \nalso whether the entry is on normalized form in the repository. \nThis makes it possible to invalidate entries when attributes change.\n\n> Just to be clear:\n>\n> If you add this to your .gitattributes\n>\n> *.c eol=lf\n>\n> I think it would be very helpful if git then would treat all .c files\n> as \"stat-dirty\" the next time it updates its index.\n\nOk.\n\n> A for config variables, what about:\n>\n> core.rereadOnAttributeChanges = [true]/false    (default = true)\n>\n> Which makes some sense for detecting it in 2, but not so much for\n> ignoring it in 6.\n\nI was thinking more along the lines of\n\n   core.allowUnnormalizedIndex = true/[false]\t(default = false)\n\nwhich unfortunately contains a negation.\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"143182","messageId":"20100607195013.GA27362@pvv.org","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006071726170.22466@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2010-06-07T19:50:13Z","receivedAt":"2010-06-07T19:50:13Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Mon, Jun 07, 2010 at 06:37:56PM +0200, Henrik Grubbström wrote:\n>\n> On Mon, 7 Jun 2010, Finn Arne Gangstad wrote:\n>\n>> I think it would be best if git at this time could decide that the\n>> affected files also become dirty. The ideal commit is one that\n>> both alters the .gitattributes _and_ the affected files at the same\n>> time, and git should make it easy to create that commit.\n>\n> I agree in the case of newly added attributes. In the case of \n> repositories already containing unnormalized files this however leads to \n> problems.\n> eg\n>\n>   Consider the case above, but a while later when the repository has been\n>   fixed at HEAD. If an old version from before the normalization is\n>   checked out, the index will once again become dirty, which means that\n>   git will refuse the user to check out some other version unless the\n>   --force flag is given. Excessive use of --force is not a good thing.\n>   If the user is aware of the problem, and checking out old versions is\n>   a common operation, toggling the suggested option might be a good\n>   solution.\n\nMaybe I misunderstand something, but if you check out an older\nversion, the .gitattributes file will change to match the old version.\nThe old version should not have the conversion attributes set, and\nshould therefore result in a clean checkout?\n\n- Finn Arne\n"},{"id":"143243","messageId":"Pine.GSO.4.63.1006081731550.22466@shipon.roxen.com","threadId":"23968","inReplyTo":"20100607195013.GA27362@pvv.org","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-08T15:52:37Z","receivedAt":"2010-06-08T15:52:37Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"On Mon, 7 Jun 2010, Finn Arne Gangstad wrote:\n\n> On Mon, Jun 07, 2010 at 06:37:56PM +0200, Henrik Grubbström wrote:\n>>\n>> On Mon, 7 Jun 2010, Finn Arne Gangstad wrote:\n>>\n>>> I think it would be best if git at this time could decide that the\n>>> affected files also become dirty. The ideal commit is one that\n>>> both alters the .gitattributes _and_ the affected files at the same\n>>> time, and git should make it easy to create that commit.\n>>\n>> I agree in the case of newly added attributes. In the case of\n>> repositories already containing unnormalized files this however leads to\n>> problems.\n>> eg\n>>\n>>   Consider the case above, but a while later when the repository has been\n>>   fixed at HEAD. If an old version from before the normalization is\n>>   checked out, the index will once again become dirty, which means that\n>>   git will refuse the user to check out some other version unless the\n>>   --force flag is given. Excessive use of --force is not a good thing.\n>>   If the user is aware of the problem, and checking out old versions is\n>>   a common operation, toggling the suggested option might be a good\n>>   solution.\n>\n> Maybe I misunderstand something, but if you check out an older\n> version, the .gitattributes file will change to match the old version.\n> The old version should not have the conversion attributes set, and\n> should therefore result in a clean checkout?\n\nTrue, there's no problem before the attribute change, but there is \nfor commits between the attribute change and when the repository got \nnormalized (which can be a while with the current git).\n\nRe: configuration option naming:\n\n   I've settled for core.normalizationPolicy, with the values\n   'strict' (default) for the behaviour requested by you and Jonathan,\n   and 'relaxed' for my initial behaviour.\n\nTeaser:\n\n   $ git init foo\n   warning: templates not found /home/grubba/share/git-core/templates\n   Initialized empty Git repository in /tmp/grubba/foo/.git/\n   $ cd foo\n   $ cat >expanded-keywords\n   $Id: some id string $\n   $ git add expanded-keywords\n   $ git commit -m 'Initial commit.'\n   [master (root-commit) 755d1f6] Initial commit.\n    1 files changed, 1 insertions(+), 0 deletions(-)\n    create mode 100644 expanded-keywords\n   $ git status\n   # On branch master\n   nothing to commit (working directory clean)\n   $ cat >.gitattributes\n   * ident\n   $ git status\n   # On branch master\n   # Changed but not updated:\n   #   (use \"git add <file>...\" to update what will be committed)\n   #   (use \"git checkout -- <file>...\" to discard changes in working directory)\n   #\n   #       modified:   expanded-keywords\n   #\n   # Untracked files:\n   #   (use \"git add <file>...\" to include in what will be committed)\n   #\n   #       .gitattributes\n   no changes added to commit (use \"git add\" and/or \"git commit -a\")\n   $ git config core.normalizationPolicy relaxed\n   $ git status\n   # On branch master\n   # Untracked files:\n   #   (use \"git add <file>...\" to include in what will be committed)\n   #\n   #       .gitattributes\n   nothing added to commit but untracked files present (use \"git add\" to track)\n   $ git config core.normalizationPolicy strict\n   $ git status\n   # On branch master\n   # Changed but not updated:\n   #   (use \"git add <file>...\" to update what will be committed)\n   #   (use \"git checkout -- <file>...\" to discard changes in working directory)\n   #\n   #       modified:   expanded-keywords\n   #\n   # Untracked files:\n   #   (use \"git add <file>...\" to include in what will be committed)\n   #\n   #       .gitattributes\n   no changes added to commit (use \"git add\" and/or \"git commit -a\")\n   $ rm .gitattributes\n   $ git status\n   # On branch master\n   nothing to commit (working directory clean)\n\nWhich I believe matches all the behaviours that have been requested.\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"143341","messageId":"20100609140327.GA19828@pvv.org","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006081731550.22466@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2010-06-09T14:03:28Z","receivedAt":"2010-06-09T14:03:28Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Tue, Jun 08, 2010 at 05:52:37PM +0200, Henrik Grubbström wrote:\n\n> [...]\n> True, there's no problem before the attribute change, but there is for \n> commits between the attribute change and when the repository got  \n> normalized (which can be a while with the current git).\n\nAs you say, the current git makes it easy to commit something where\nthe attributes and the contents do not match. I think this needs to be\nfixed, and that your proposed patch in relaxed mode makes the problem\n_worse_, since it will then take even longer before these commits are\nfixed. But see below.\n\n>\n> Re: configuration option naming:\n>\n>   I've settled for core.normalizationPolicy, with the values\n>   'strict' (default) for the behaviour requested by you and Jonathan,\n>   and 'relaxed' for my initial behaviour.\n\nThe name might be a bit vague, maybe there are other things that could\nbe normalized? Maybe adding the word \"index\" is an improvement -\ne.g. core.indexNormalizationPolicy or just core.indexNormalization. \n\n>\n> Teaser:\n> [...]\n>   $ git status\n>   # On branch master\n>   nothing to commit (working directory clean)\n>   $ cat >.gitattributes\n>   * ident\n>   $ git status\n> [...]\n>   #       modified:   expanded-keywords\n> [...]\n>\n>   $ git config core.normalizationPolicy relaxed\n>   $ git status\n>   # On branch master\n> [No longer modified]\n\nTHIS behaviour is what I find scary. In this case, \"ident\" is clearly\na newly added attribute, and git should not hide this from you. If you\nadd a mode where git will hide this permanently, chances are the\nrepositories will never be fixed.\n\nThe ident attribute may be a bit special since in your case it is only\nsupposed to change if some other contents in the file change as well,\nbut please also think how this will work with the text/eol\nattributes. Setting the text attribute and then having to CHANGE a\nfile before getting it normalized is not good.\n\nStill, I think your original problem description of cloning something\nand ending up with a dirty tree is indeed an annoying problem.  So\nwhat about having the relaxed mode behave as follows:\n\nIf both of these are true:\n - the current attributes for a file are the same as it is registered as\n   in the index with your new patch\n - a checkout of the file would result in identical contents to what is \n   currently in the working directory\nThen behave as if the file is not modified.\n\nOr, in other words: If attributes are unchanged, a file is unmodified\nnot only if it would result in the same contents after being added,\nbut also if it would result in the current working directory\ncontents after being checked out again.\n\nThis should work for both text and ident on clone at least.\n\n- Finn Arne\n"},{"id":"143364","messageId":"Pine.GSO.4.63.1006091943100.22466@shipon.roxen.com","threadId":"23968","inReplyTo":"20100609140327.GA19828@pvv.org","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Henrik Grubbström","fromEmail":"grubba@roxen.com","sentAt":"2010-06-09T18:04:34Z","receivedAt":"2010-06-09T18:04:34Z","isPatch":true,"sender":{"key":"grubba@roxen.com","avatar":null},"body":"On Wed, 9 Jun 2010, Finn Arne Gangstad wrote:\n\n> On Tue, Jun 08, 2010 at 05:52:37PM +0200, Henrik Grubbström wrote:\n>\n>> [...]\n>> True, there's no problem before the attribute change, but there is for\n>> commits between the attribute change and when the repository got\n>> normalized (which can be a while with the current git).\n>\n> As you say, the current git makes it easy to commit something where\n> the attributes and the contents do not match. I think this needs to be\n> fixed, and that your proposed patch in relaxed mode makes the problem\n> _worse_, since it will then take even longer before these commits are\n> fixed. But see below.\n>\n>>\n>> Re: configuration option naming:\n>>\n>>   I've settled for core.normalizationPolicy, with the values\n>>   'strict' (default) for the behaviour requested by you and Jonathan,\n>>   and 'relaxed' for my initial behaviour.\n>\n> The name might be a bit vague, maybe there are other things that could\n> be normalized? Maybe adding the word \"index\" is an improvement -\n> e.g. core.indexNormalizationPolicy or just core.indexNormalization.\n\nHmm... Maybe.\n\n>> Teaser:\n[...]\n>\n> THIS behaviour is what I find scary. In this case, \"ident\" is clearly\n> a newly added attribute, and git should not hide this from you. If you\n> add a mode where git will hide this permanently, chances are the\n> repositories will never be fixed.\n\nTrue.\n\n> The ident attribute may be a bit special since in your case it is only\n> supposed to change if some other contents in the file change as well,\n> but please also think how this will work with the text/eol\n> attributes. Setting the text attribute and then having to CHANGE a\n> file before getting it normalized is not good.\n>\n> Still, I think your original problem description of cloning something\n> and ending up with a dirty tree is indeed an annoying problem.  So\n> what about having the relaxed mode behave as follows:\n>\n> If both of these are true:\n> - the current attributes for a file are the same as it is registered as\n>   in the index with your new patch\n> - a checkout of the file would result in identical contents to what is\n>   currently in the working directory\n> Then behave as if the file is not modified.\n>\n> Or, in other words: If attributes are unchanged, a file is unmodified\n> not only if it would result in the same contents after being added,\n> but also if it would result in the current working directory\n> contents after being checked out again.\n\nOk, so the expanded-keywords file in the example should show up as \nmodified in relaxed mode as well, but be cleaned if the modified \nattributes file is added to the index? Or only after being committed?\n\n> This should work for both text and ident on clone at least.\n\nI'll see what I can do...\n\nCurrently, I'm trying to understand the semantic difference between \nce_modified() and ce_match_stat().\n\n--\nHenrik Grubbström\t\t\t\t\tgrubba@grubba.org\nRoxen Internet Software AB\t\t\t\tgrubba@roxen.com"},{"id":"143476","messageId":"20100610195555.GA20759@pvv.org","threadId":"23968","inReplyTo":"Pine.GSO.4.63.1006091943100.22466@shipon.roxen.com","subject":"Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2010-06-10T19:55:55Z","receivedAt":"2010-06-10T19:55:55Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Wed, Jun 09, 2010 at 08:04:34PM +0200, Henrik Grubbström wrote:\n\n> Ok, so the expanded-keywords file in the example should show up as  \n> modified in relaxed mode as well, but be cleaned if the modified  \n> attributes file is added to the index? Or only after being committed?\n\nNot before being committed, since you would otherwise have to add all\nother files before adding .gitattributes, but I am not sure even that\nis sufficient reason to claim the files are unmodified (see case 3 below).\n\nI think we agree on the following:\n\nIf there is a discrepancy between .gitattributes and the contents in\nthe repository, the following should be true:\n\ngit checkout -f (or git reset --hard)\ngit status -> ALWAYS report modified files in strict mode\nsleep 1\ntouch *\ngit status -> NEVER report modified files in relaxed mode\n\n\nThe case I think you are asking about above is the following in\n\"relaxed\" mode:\n\necho \"something that causes a discrepancy\" >> .gitattributes\ngit status -> MODIFIED (1)\ngit add .gitattributes\ngit status -> MODIFIED (2)\ngit commit -m \"bad commit\"\ngit status -> ???????  (3)    <<-- Do you want this to be CLEAN?\ngit reset --hard (or git checkout -f)\nsleep 1\ntouch *\ngit status -> CLEAN    (4)\n\n1 and 4 should be uncontroversial and 2 I think is necessary because\nyou should be able to git add in several steps. Whether 3 should be\nclean or modified I'm not so sure about, I think that it would make it\nmore likely to get the repo normalized properly if it was still seen\nas modified there.\n\n- Finn Arne\n"}]}