Simplify cache API

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@pobox.com>

diff --git a/builtin-add.c b/builtin-add.c
index 9847b7e019ece82f19e3e5decf8647eb921c4291..866d19dd77e693a1ba77baa802c0e3f3ea85a305 100644 (file)
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -103,7 +103,6 @@ static void update_callback(struct diff_queue_struct *q,
                        break;
                case DIFF_STATUS_DELETED:
                        remove_file_from_cache(path);
                        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;
                        if (verbose)
                                printf("remove '%s'\n", path);
                        break;

diff --git a/builtin-apply.c b/builtin-apply.c
index 976ec770417cba4113c4c55f5ca7a5f7b8520f1e..79a4852bc24077b13bfa5e71fe04ee599717f310 100644 (file)
--- 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);
        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)) {
        }
        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);
                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 */
 }
 
 /* phase zero is to remove, phase one is to create */

diff --git a/builtin-mv.c b/builtin-mv.c
index 3563216acaebba668f465895fe0563e5d7113fef..b95b7d286ab5358a89a0309927b9f5e5e23fc4d0 100644 (file)
--- 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);
                }
 
                        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) ||
 
                if (active_cache_changed) {
                        if (write_cache(newfd, active_cache, active_nr) ||

diff --git a/builtin-rm.c b/builtin-rm.c
index 9a808c1bf96ec52d836b0a69816f6118f0291a45..3b0677e44b290f0a44fd81bd2e31ddc96bc1946d 100644 (file)
--- 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);
 
                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)
        }
 
        if (show_only)

diff --git a/builtin-update-index.c b/builtin-update-index.c
index a7a4574f2bff2a7db4a1c25aa4a514ad99760381..55fb679d68f141253f5d0ba5c73033d267e2a271 100644 (file)
--- 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;
 
        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!
        /*
         * 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);
                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;
 }
 
        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;
        }
                        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))
 
        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;
                }
                                free(path_name);
                        continue;
                }

-               cache_tree_invalidate_path(active_cache_tree, path_name);

 
                if (!mode) {
                        /* mode == 0 means there is no such path -- remove */
 
                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;
        }
 
                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);
        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 8b1c94e0e3f539cd0b507fd130b60aa443b9680b..d82a99a9158f46cbf81d5dda8de1a28228ac16cb 100644 (file)
--- 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;
        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;
        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);
                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;
 }
 
        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;
 
        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. */
        pos = index_name_pos(istate, ce->name, ntohs(ce->ce_flags));
 
        /* existing match? Just replace it. */