Re: Broken adding of cache entries
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:41:56
This patch makes git-update-cache --add refuse to create an
index where one entry is a file and another entry needs to have
that entry as its leading directory.
I am not quite sure the semantics is quite right, so I am
holding it off from putting it in git-jc repository for now, but
please review it, give it a try and tell me what you think.
Here is what I did.
$ ls -a
./ ../
$ git-init-db
defaulting to local storage area
$ date >path
$ git-update-cache --add path
$ rm -f path; mkdir path; date >path/file
$ git-update-cache --add path/file
fatal: Unable to add path/file to database
$ git-update-cache --remove path ;# *****
fatal: Unable to add path to database
$ git-update-cache --force-remove path
$ git-update-cache --add path/file
$ git-ls-files --stage
100644 3e3f5e43a8e7baa7f0b1f2d7b99ed25fcbe582d2 0 path/file
$ rm -fr path
$ git-update-cache --remove path/file
$ mkdir path; date >path/file
$ git-update-cache --add path/file
$ git-ls-files --stage
100644 1b4e067c82b90419c2d2ce80e72079f926a49cc3 0 path/file
$ rm -fr path; date >path
$ git-update-cache --add path
fatal: Unable to add path to database
$ git-update-cache --remove path/file
$ git-update-cache --add path
$ git-ls-files --stage
100644 b4509b7a4fed200befc45746061eb1d32ad6cbc9 0 path
The one marked with ***** is what I am debating myself about,
but that one would probably be an independent fix.
Not-signed-off-yet-by: Junio C Hamano [off-list ref]
---
jit-diff 0:7 read-cache.c
# - HEAD: Notice tree objects with duplicate entries.
# + 7: Refuse to create file/dir conflict cache entry.--- a/read-cache.c
+++ b/read-cache.c@@ -123,6 +123,64 @@ int same_name(struct cache_entry *a, str return ce_namelen(b) == len && !memcmp(a->name, b->name, len); } +/* We may be in a situation where we already have + * path/file and path is being added, or we already + * have path and path/file is being added. Either one + * would result in a nonsense tree that has path twice + * when git-write-tree tries to write it out. Prevent it. + */ +static int check_file_directory_conflict(const char *path) +{ + int pos; + int namelen = strlen(path); + char *pathbuf = xmalloc(namelen + 1); + char *cp; + + memcpy(pathbuf, path, namelen + 1); + + cp = pathbuf; + while (1) { + char *ep = strchr(cp, '/'); + if (ep == 0) + break; + *ep = 0; + pos = cache_name_pos(pathbuf, + htons(create_ce_flags(ep-cp, 0))); + if (0 <= pos) { + /* Our leading path component is registered as a file, + * and we are trying to make it a directory. + */ + free(pathbuf); + return -1; + } + *ep = '/'; + cp = ep + 1; + } + free(pathbuf); + + /* Do we have an entry in the cache that makes our path a prefix + * of it? That is, are we creating a file where they expect a + * directory there? + */ + pos = cache_name_pos(path, + htons(create_ce_flags(namelen, 0))); + + /* (0 <= pos) cannot happen because add_cache_entry() + * should have taken care of that case. + */ + pos = -pos-1; + + /* pos would point at an existing entry that would come immediately + * after our path. Does it have our path as a leading path prefix? + */ + if ((pos < active_nr) && + !strncmp(active_cache[pos]->name, path, namelen) && + active_cache[pos]->name[namelen] == '/') + return -1; + + return 0; +} + int add_cache_entry(struct cache_entry *ce, int ok_to_add) { int pos;
@@ -152,6 +210,9 @@ int add_cache_entry(struct cache_entry * if (!ok_to_add) return -1; + if (check_file_directory_conflict(ce->name)) + return -1; + /* Make sure the array is big enough .. */ if (active_nr == active_alloc) { active_alloc = alloc_nr(active_alloc);