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>
			
			
				maint
			
			
		
							parent
							
								
									611d8139e4
								
							
						
					
					
						commit
						09d5dc32fb
					
				|  | @ -103,7 +103,6 @@ static void update_callback(struct diff_queue_struct *q, | ||||||
| 			break; | 			break; | ||||||
| 		case DIFF_STATUS_DELETED: | 		case DIFF_STATUS_DELETED: | ||||||
| 			remove_file_from_cache(path); | 			remove_file_from_cache(path); | ||||||
| 			cache_tree_invalidate_path(active_cache_tree, path); |  | ||||||
| 			if (verbose) | 			if (verbose) | ||||||
| 				printf("remove '%s'\n", path); | 				printf("remove '%s'\n", path); | ||||||
| 			break; | 			break; | ||||||
|  |  | ||||||
|  | @ -2394,7 +2394,6 @@ static void remove_file(struct patch *patch, int rmdir_empty) | ||||||
| 	if (update_index) { | 	if (update_index) { | ||||||
| 		if (remove_file_from_cache(patch->old_name) < 0) | 		if (remove_file_from_cache(patch->old_name) < 0) | ||||||
| 			die("unable to remove %s from index", patch->old_name); | 			die("unable to remove %s from index", patch->old_name); | ||||||
| 		cache_tree_invalidate_path(active_cache_tree, patch->old_name); |  | ||||||
| 	} | 	} | ||||||
| 	if (!cached) { | 	if (!cached) { | ||||||
| 		if (S_ISGITLINK(patch->old_mode)) { | 		if (S_ISGITLINK(patch->old_mode)) { | ||||||
|  | @ -2549,7 +2548,6 @@ static void create_file(struct patch *patch) | ||||||
| 		mode = S_IFREG | 0644; | 		mode = S_IFREG | 0644; | ||||||
| 	create_one_file(path, mode, buf, size); | 	create_one_file(path, mode, buf, size); | ||||||
| 	add_index_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 */ | ||||||
|  |  | ||||||
|  | @ -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++) { | 		for (i = 0; i < deleted.nr; i++) | ||||||
| 			const char *path = deleted.items[i].path; | 			remove_file_from_cache(deleted.items[i].path); | ||||||
| 			remove_file_from_cache(path); |  | ||||||
| 			cache_tree_invalidate_path(active_cache_tree, path); |  | ||||||
| 		} |  | ||||||
|  |  | ||||||
| 		if (active_cache_changed) { | 		if (active_cache_changed) { | ||||||
| 			if (write_cache(newfd, active_cache, active_nr) || | 			if (write_cache(newfd, active_cache, active_nr) || | ||||||
|  |  | ||||||
|  | @ -227,7 +227,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix) | ||||||
|  |  | ||||||
| 		if (remove_file_from_cache(path)) | 		if (remove_file_from_cache(path)) | ||||||
| 			die("git-rm: unable to remove %s", path); | 			die("git-rm: unable to remove %s", path); | ||||||
| 		cache_tree_invalidate_path(active_cache_tree, path); |  | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	if (show_only) | 	if (show_only) | ||||||
|  |  | ||||||
|  | @ -195,11 +195,6 @@ static int process_path(const char *path) | ||||||
| 	int len; | 	int len; | ||||||
| 	struct stat st; | 	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 | 	 * First things first: get the stat information, to decide | ||||||
| 	 * what to do about the pathname! | 	 * 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?", | 		return error("%s: cannot add to the index - missing --add option?", | ||||||
| 			     path); | 			     path); | ||||||
| 	report("add '%s'", 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); | 			die("Unable to mark file %s", path); | ||||||
| 		goto free_return; | 		goto free_return; | ||||||
| 	} | 	} | ||||||
| 	cache_tree_invalidate_path(active_cache_tree, path); |  | ||||||
|  |  | ||||||
| 	if (force_remove) { | 	if (force_remove) { | ||||||
| 		if (remove_file_from_cache(p)) | 		if (remove_file_from_cache(p)) | ||||||
|  | @ -367,7 +360,6 @@ static void read_index_info(int line_termination) | ||||||
| 				free(path_name); | 				free(path_name); | ||||||
| 			continue; | 			continue; | ||||||
| 		} | 		} | ||||||
| 		cache_tree_invalidate_path(active_cache_tree, path_name); |  | ||||||
|  |  | ||||||
| 		if (!mode) { | 		if (!mode) { | ||||||
| 			/* mode == 0 means there is no such path -- remove */ | 			/* 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); | 	remove_file_from_cache(path); | ||||||
| 	if (add_cache_entry(ce_2, ADD_CACHE_OK_TO_ADD)) { | 	if (add_cache_entry(ce_2, ADD_CACHE_OK_TO_ADD)) { | ||||||
| 		error("%s: cannot add our version to the index.", path); | 		error("%s: cannot add our version to the index.", path); | ||||||
|  |  | ||||||
|  | @ -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)); | 	int pos = index_name_pos(istate, path, strlen(path)); | ||||||
| 	if (pos < 0) | 	if (pos < 0) | ||||||
| 		pos = -pos-1; | 		pos = -pos-1; | ||||||
|  | 	cache_tree_invalidate_path(istate->cache_tree, path); | ||||||
| 	while (pos < istate->cache_nr && !strcmp(istate->cache[pos]->name, path)) | 	while (pos < istate->cache_nr && !strcmp(istate->cache[pos]->name, path)) | ||||||
| 		remove_index_entry_at(istate, pos); | 		remove_index_entry_at(istate, pos); | ||||||
| 	return 0; | 	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); | 		die("unable to add %s to index",path); | ||||||
| 	if (verbose) | 	if (verbose) | ||||||
| 		printf("add '%s'\n", path); | 		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 ok_to_replace = option & ADD_CACHE_OK_TO_REPLACE; | ||||||
| 	int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK; | 	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)); | 	pos = index_name_pos(istate, ce->name, ntohs(ce->ce_flags)); | ||||||
|  |  | ||||||
| 	/* existing match? Just replace it. */ | 	/* existing match? Just replace it. */ | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Junio C Hamano
						Junio C Hamano