Earlier, add_file_to_index() invalidated the path in the cache-tree
but remove_file_from_cache() did not, and the user of the latter
needed to invalidate the entry himself. This led to a few bugs due to
missed invalidate calls already. This patch makes the management of
cache-tree less error prone by making more invalidate calls from lower
level cache API functions.
The rules are:
- If you are going to write the index, you should either maintain
cache_tree correctly.
- If you cannot, alternatively you can remove the entire cache_tree
by calling cache_tree_free() before you call write_cache().
- When you modify the index, cache_tree_invalidate_path() should be
called with the path you are modifying, to discard the entry from
the cache-tree structure.
- The following cache API functions exported from read-cache.c (and
the macro whose names have "cache" instead of "index")
automatically call cache_tree_invalidate_path() for you:
- remove_file_from_index();
- add_file_to_index();
- add_index_entry();
You can modify the index bypassing the above API functions
(e.g. find an existing cache entry from the index and modify it in
place). You need to call cache_tree_invalidate_path() yourself in
such a case.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
* This was long overdue API fix. I noticed that Carlos made
the same mistake I made a few times in the "git reset
<commit> paths..." code he posted recently (already merged
in 'next'). This was a common mistake and we had to fix
breakages that resulted from the API mistake a few times
already. Namely:
"git add -u" that only removed paths (Aug 15, 2007)
"git mv" removal of paths (Oct 1, 2006)
"git update-index --unresolve" (Apr 26, 2006)
Hopefully we will see fewer bugs in new code in the future
with this change in place.
builtin-add.c | 1 -
builtin-apply.c | 2 --
builtin-mv.c | 7 ++-----
builtin-rm.c | 1 -
builtin-update-index.c | 9 ---------
read-cache.c | 3 ++-
6 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 105a9f0..3a038c0 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -102,7 +102,6 @@ static void update_callback(struct diff_queue_struct *q,
break;
case DIFF_STATUS_DELETED:
remove_file_from_cache(path);
- cache_tree_invalidate_path(active_cache_tree, path);
if (verbose)
printf("remove '%s'\n", path);
break;
diff --git a/builtin-apply.c b/builtin-apply.c
index 976ec77..79a4852 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2394,7 +2394,6 @@ static void remove_file(struct patch *patch, int rmdir_empty)
if (update_index) {
if (remove_file_from_cache(patch->old_name) < 0)
die("unable to remove %s from index", patch->old_name);
- cache_tree_invalidate_path(active_cache_tree, patch->old_name);
}
if (!cached) {
if (S_ISGITLINK(patch->old_mode)) {
@@ -2549,7 +2548,6 @@ static void create_file(struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(path, mode, buf, size);
add_index_file(path, mode, buf, size);
- cache_tree_invalidate_path(active_cache_tree, path);
}
/* phase zero is to remove, phase one is to create */
diff --git a/builtin-mv.c b/builtin-mv.c
index 3563216..b95b7d2 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -276,11 +276,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
add_file_to_cache(path, verbose);
}
- for (i = 0; i < deleted.nr; i++) {
- const char *path = deleted.items[i].path;
- remove_file_from_cache(path);
- cache_tree_invalidate_path(active_cache_tree, path);
- }
+ for (i = 0; i < deleted.nr; i++)
+ remove_file_from_cache(deleted.items[i].path);
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
diff --git a/builtin-rm.c b/builtin-rm.c
index 9a808c1..3b0677e 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -227,7 +227,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (remove_file_from_cache(path))
die("git-rm: unable to remove %s", path);
- cache_tree_invalidate_path(active_cache_tree, path);
}
if (show_only)
diff --git a/builtin-update-index.c b/builtin-update-index.c
index a7a4574..55fb679 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -195,11 +195,6 @@ static int process_path(const char *path)
int len;
struct stat st;
- /* We probably want to do this in remove_file_from_cache() and
- * add_cache_entry() instead...
- */
- cache_tree_invalidate_path(active_cache_tree, path);
-
/*
* First things first: get the stat information, to decide
* what to do about the pathname!
@@ -239,7 +234,6 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
return error("%s: cannot add to the index - missing --add option?",
path);
report("add '%s'", path);
- cache_tree_invalidate_path(active_cache_tree, path);
return 0;
}
@@ -284,7 +278,6 @@ static void update_one(const char *path, const char *prefix, int prefix_length)
die("Unable to mark file %s", path);
goto free_return;
}
- cache_tree_invalidate_path(active_cache_tree, path);
if (force_remove) {
if (remove_file_from_cache(p))
@@ -367,7 +360,6 @@ static void read_index_info(int line_termination)
free(path_name);
continue;
}
- cache_tree_invalidate_path(active_cache_tree, path_name);
if (!mode) {
/* mode == 0 means there is no such path -- remove */
@@ -474,7 +466,6 @@ static int unresolve_one(const char *path)
goto free_return;
}
- cache_tree_invalidate_path(active_cache_tree, path);
remove_file_from_cache(path);
if (add_cache_entry(ce_2, ADD_CACHE_OK_TO_ADD)) {
error("%s: cannot add our version to the index.", path);
diff --git a/read-cache.c b/read-cache.c
index 8b1c94e..d82a99a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -346,6 +346,7 @@ int remove_file_from_index(struct index_state *istate, const char *path)
int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0)
pos = -pos-1;
+ cache_tree_invalidate_path(istate->cache_tree, path);
while (pos < istate->cache_nr && !strcmp(istate->cache[pos]->name, path))
remove_index_entry_at(istate, pos);
return 0;
@@ -430,7 +431,6 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)
die("unable to add %s to index",path);
if (verbose)
printf("add '%s'\n", path);
- cache_tree_invalidate_path(istate->cache_tree, path);
return 0;
}
@@ -673,6 +673,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
int ok_to_replace = option & ADD_CACHE_OK_TO_REPLACE;
int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;
+ cache_tree_invalidate_path(istate->cache_tree, ce->name);
pos = index_name_pos(istate, ce->name, ntohs(ce->ce_flags));
/* existing match? Just replace it. */
--
1.5.3.1.932.g0b72
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
