From dbc3904ebc604bf5b818d9840b79228aacdd1343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 16 Dec 2012 11:15:25 +0700 Subject: [PATCH 1/4] cache-tree: remove dead i-t-a code in verify_cache() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code is added in 331fcb5 (git add --intent-to-add: do not let an empty blob be committed by accident - 2008-11-28) to forbid committing when i-t-a entries are present. When we allow that, we forgot to remove this. Noticed-by: Junio C Hamano Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- cache-tree.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 28ed6574a2..e2beab584b 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -166,12 +166,8 @@ static int verify_cache(struct cache_entry **cache, fprintf(stderr, "...\n"); break; } - if (ce_stage(ce)) - fprintf(stderr, "%s: unmerged (%s)\n", - ce->name, sha1_to_hex(ce->sha1)); - else - fprintf(stderr, "%s: not added yet\n", - ce->name); + fprintf(stderr, "%s: unmerged (%s)\n", + ce->name, sha1_to_hex(ce->sha1)); } } if (funny) From 386cc8b031611497186a8c89be0f980a54786525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 16 Dec 2012 11:15:26 +0700 Subject: [PATCH 2/4] cache-tree: replace "for" loops in update_one with "while" loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loops in update_one can be increased in two different ways: step by one for files and by for directories. "for" loop is not suitable for this as it always steps by one and special handling is required for directories. Replace them with "while" loops for clarity. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- cache-tree.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index e2beab584b..44eed28aba 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -259,7 +259,8 @@ static int update_one(struct cache_tree *it, /* * Find the subtrees and update them. */ - for (i = 0; i < entries; i++) { + i = 0; + while (i < entries) { struct cache_entry *ce = cache[i]; struct cache_tree_sub *sub; const char *path, *slash; @@ -271,8 +272,10 @@ static int update_one(struct cache_tree *it, break; /* at the end of this level */ slash = strchr(path + baselen, '/'); - if (!slash) + if (!slash) { + i++; continue; + } /* * a/bbb/c (base = a/, slash = /c) * ==> @@ -289,7 +292,7 @@ static int update_one(struct cache_tree *it, flags); if (subcnt < 0) return subcnt; - i += subcnt - 1; + i += subcnt; sub->used = 1; } @@ -300,7 +303,8 @@ static int update_one(struct cache_tree *it, */ strbuf_init(&buffer, 8192); - for (i = 0; i < entries; i++) { + i = 0; + while (i < entries) { struct cache_entry *ce = cache[i]; struct cache_tree_sub *sub; const char *path, *slash; @@ -320,7 +324,7 @@ static int update_one(struct cache_tree *it, if (!sub) die("cache-tree.c: '%.*s' in '%s' not found", entlen, path + baselen, path); - i += sub->cache_tree->entry_count - 1; + i += sub->cache_tree->entry_count; sha1 = sub->cache_tree->sha1; mode = S_IFDIR; } @@ -328,6 +332,7 @@ static int update_one(struct cache_tree *it, sha1 = ce->sha1; mode = ce->ce_mode; entlen = pathlen - baselen; + i++; } if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) { strbuf_release(&buffer); From 3cf773e4264ecc6e9f603a24aeb72cc68b372f96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 16 Dec 2012 11:15:27 +0700 Subject: [PATCH 3/4] cache-tree: fix writing cache-tree when CE_REMOVE is present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit entry_count is used in update_one() for two purposes: 1. to skip through the number of processed entries in in-memory index 2. to record the number of entries this cache-tree covers on disk Unfortunately when CE_REMOVE is present these numbers are not the same because CE_REMOVE entries are automatically removed before writing to disk but entry_count is not adjusted and still counts CE_REMOVE entries. Separate the two use cases into two different variables. #1 is taken care by the new field count in struct cache_tree_sub and entry_count is prepared for #2. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- cache-tree.c | 30 +++++++++++++++++++++++------- cache-tree.h | 1 + 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 44eed28aba..2c10b2e2dd 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -238,6 +238,7 @@ static int update_one(struct cache_tree *it, int entries, const char *base, int baselen, + int *skip_count, int flags) { struct strbuf buffer; @@ -245,6 +246,8 @@ static int update_one(struct cache_tree *it, int dryrun = flags & WRITE_TREE_DRY_RUN; int i; + *skip_count = 0; + if (0 <= it->entry_count && has_sha1_file(it->sha1)) return it->entry_count; @@ -264,7 +267,7 @@ static int update_one(struct cache_tree *it, struct cache_entry *ce = cache[i]; struct cache_tree_sub *sub; const char *path, *slash; - int pathlen, sublen, subcnt; + int pathlen, sublen, subcnt, subskip; path = ce->name; pathlen = ce_namelen(ce); @@ -289,10 +292,13 @@ static int update_one(struct cache_tree *it, cache + i, entries - i, path, baselen + sublen + 1, + &subskip, flags); if (subcnt < 0) return subcnt; i += subcnt; + sub->count = subcnt; /* to be used in the next loop */ + *skip_count += subskip; sub->used = 1; } @@ -324,7 +330,7 @@ static int update_one(struct cache_tree *it, if (!sub) die("cache-tree.c: '%.*s' in '%s' not found", entlen, path + baselen, path); - i += sub->cache_tree->entry_count; + i += sub->count; sha1 = sub->cache_tree->sha1; mode = S_IFDIR; } @@ -340,8 +346,18 @@ static int update_one(struct cache_tree *it, mode, sha1_to_hex(sha1), entlen+baselen, path); } - if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD)) - continue; /* entry being removed or placeholder */ + /* + * CE_REMOVE entries are removed before the index is + * written to disk. Skip them to remain consistent + * with the future on-disk index. + */ + if (ce->ce_flags & CE_REMOVE) { + *skip_count = *skip_count + 1; + continue; + } + + if (ce->ce_flags & CE_INTENT_TO_ADD) + continue; strbuf_grow(&buffer, entlen + 100); strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); @@ -361,7 +377,7 @@ static int update_one(struct cache_tree *it, } strbuf_release(&buffer); - it->entry_count = i; + it->entry_count = i - *skip_count; #if DEBUG fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n", it->entry_count, it->subtree_nr, @@ -375,11 +391,11 @@ int cache_tree_update(struct cache_tree *it, int entries, int flags) { - int i; + int i, skip; i = verify_cache(cache, entries, flags); if (i) return i; - i = update_one(it, cache, entries, "", 0, flags); + i = update_one(it, cache, entries, "", 0, &skip, flags); if (i < 0) return i; return 0; diff --git a/cache-tree.h b/cache-tree.h index d8cb2e9e39..55d0f59f2b 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -7,6 +7,7 @@ struct cache_tree; struct cache_tree_sub { struct cache_tree *cache_tree; + int count; /* internally used by update_one() */ int namelen; int used; char name[FLEX_ARRAY]; From eec3e7e406867487036cf447fe8fa18740824b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 16 Dec 2012 11:15:28 +0700 Subject: [PATCH 4/4] cache-tree: invalidate i-t-a paths after generating trees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Intent-to-add entries used to forbid writing trees so it was not a problem. After commit 3f6d56d (commit: ignore intent-to-add entries instead of refusing - 2012-02-07), we can generate trees from an index with i-t-a entries. However, the commit forgets to invalidate all paths leading to i-t-a entries. With fully valid cache-tree (e.g. after commit or write-tree), diff operations may prefer cache-tree to index and not see i-t-a entries in the index, because cache-tree does not have them. Reported-by: Jonathon Mah Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- cache-tree.c | 14 ++++++++++++-- t/t2203-add-intent.sh | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 2c10b2e2dd..37e4d008b5 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -244,6 +244,7 @@ static int update_one(struct cache_tree *it, struct strbuf buffer; int missing_ok = flags & WRITE_TREE_MISSING_OK; int dryrun = flags & WRITE_TREE_DRY_RUN; + int to_invalidate = 0; int i; *skip_count = 0; @@ -333,6 +334,8 @@ static int update_one(struct cache_tree *it, i += sub->count; sha1 = sub->cache_tree->sha1; mode = S_IFDIR; + if (sub->cache_tree->entry_count < 0) + to_invalidate = 1; } else { sha1 = ce->sha1; @@ -356,8 +359,15 @@ static int update_one(struct cache_tree *it, continue; } - if (ce->ce_flags & CE_INTENT_TO_ADD) + /* + * CE_INTENT_TO_ADD entries exist on on-disk index but + * they are not part of generated trees. Invalidate up + * to root to force cache-tree users to read elsewhere. + */ + if (ce->ce_flags & CE_INTENT_TO_ADD) { + to_invalidate = 1; continue; + } strbuf_grow(&buffer, entlen + 100); strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); @@ -377,7 +387,7 @@ static int update_one(struct cache_tree *it, } strbuf_release(&buffer); - it->entry_count = i - *skip_count; + it->entry_count = to_invalidate ? -1 : i - *skip_count; #if DEBUG fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n", it->entry_count, it->subtree_nr, diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index ec35409f9c..2a4a749b4f 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -62,5 +62,25 @@ test_expect_success 'can "commit -a" with an i-t-a entry' ' git commit -a -m all ' +test_expect_success 'cache-tree invalidates i-t-a paths' ' + git reset --hard && + mkdir dir && + : >dir/foo && + git add dir/foo && + git commit -m foo && + + : >dir/bar && + git add -N dir/bar && + git diff --cached --name-only >actual && + echo dir/bar >expect && + test_cmp expect actual && + + git write-tree >/dev/null && + + git diff --cached --name-only >actual && + echo dir/bar >expect && + test_cmp expect actual +' + test_done