Commit d3282680 by John Fultz

Fix index-adding functions to know when to trust filemodes.

The idea...sometimes, a filemode is user-specified via an
explicit git_index_entry.  In this case, believe the user, always.

Sometimes, it is instead built up by statting the file system.  In
those cases, go with the existing logic we have to determine
whether the file system supports all filemodes and symlinks, and
make the best guess.

On file systems which have full filemode and symlink support, this
commit should make no difference.  On others (most notably Windows),
this will fix problems things like:
* git_index_add and git_index_add_frombuffer() should be believed.
* As a consequence, git_checkout_tree should make the filemodes in
the index match the ones in the tree.
* And diffs with GIT_DIFF_UPDATE_INDEX don't write the wrong filemodes.
* And merges, and probably other downstream stuff now fixed, too.

This makes my previous changes to checkout.c unnecessary,
so they are now reverted.

Also, added a test for index_entry permissions from git_index_add
and git_index_add_frombuffer, both of which failed before these changes.
parent 6598aa7e
...@@ -1794,9 +1794,6 @@ static int checkout_create_the_new( ...@@ -1794,9 +1794,6 @@ static int checkout_create_the_new(
int error = 0; int error = 0;
git_diff_delta *delta; git_diff_delta *delta;
size_t i; size_t i;
int caps = git_index_caps(data->index);
git_index_set_caps(data->index, caps & ~GIT_INDEXCAP_NO_FILEMODE);
git_vector_foreach(&data->diff->deltas, i, delta) { git_vector_foreach(&data->diff->deltas, i, delta) {
if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) { if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) {
...@@ -1818,8 +1815,6 @@ static int checkout_create_the_new( ...@@ -1818,8 +1815,6 @@ static int checkout_create_the_new(
} }
} }
git_index_set_caps(data->index, caps);
return 0; return 0;
} }
...@@ -2548,12 +2543,7 @@ int git_checkout_iterator( ...@@ -2548,12 +2543,7 @@ int git_checkout_iterator(
cleanup: cleanup:
if (!error && data.index != NULL && if (!error && data.index != NULL &&
(data.strategy & CHECKOUT_INDEX_DONT_WRITE_MASK) == 0) (data.strategy & CHECKOUT_INDEX_DONT_WRITE_MASK) == 0)
{
int caps = git_index_caps(data.index);
git_index_set_caps(data.index, caps & ~GIT_INDEXCAP_NO_FILEMODE);
error = git_index_write(data.index); error = git_index_write(data.index);
git_index_set_caps(data.index, caps);
}
git_diff_free(data.diff); git_diff_free(data.diff);
git_iterator_free(workdir); git_iterator_free(workdir);
......
...@@ -987,9 +987,10 @@ static int index_no_dups(void **old, void *new) ...@@ -987,9 +987,10 @@ static int index_no_dups(void **old, void *new)
* it, then it will return an error **and also free the entry**. When * it, then it will return an error **and also free the entry**. When
* it replaces an existing entry, it will update the entry_ptr with the * it replaces an existing entry, it will update the entry_ptr with the
* actual entry in the index (and free the passed in one). * actual entry in the index (and free the passed in one).
* trust_mode is whether we trust the mode in entry_ptr.
*/ */
static int index_insert( static int index_insert(
git_index *index, git_index_entry **entry_ptr, int replace) git_index *index, git_index_entry **entry_ptr, int replace, bool trust_mode)
{ {
int error = 0; int error = 0;
size_t path_length, position; size_t path_length, position;
...@@ -1021,6 +1022,9 @@ static int index_insert( ...@@ -1021,6 +1022,9 @@ static int index_insert(
&position, index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false)) { &position, index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false)) {
existing = index->entries.contents[position]; existing = index->entries.contents[position];
/* update filemode to existing values if stat is not trusted */ /* update filemode to existing values if stat is not trusted */
if (trust_mode)
entry->mode = git_index__create_mode(entry->mode);
else
entry->mode = index_merge_mode(index, existing, entry->mode); entry->mode = index_merge_mode(index, existing, entry->mode);
} }
...@@ -1122,7 +1126,7 @@ int git_index_add_frombuffer( ...@@ -1122,7 +1126,7 @@ int git_index_add_frombuffer(
git_oid_cpy(&entry->id, &id); git_oid_cpy(&entry->id, &id);
entry->file_size = len; entry->file_size = len;
if ((error = index_insert(index, &entry, 1)) < 0) if ((error = index_insert(index, &entry, 1, true)) < 0)
return error; return error;
/* Adding implies conflict was resolved, move conflict entries to REUC */ /* Adding implies conflict was resolved, move conflict entries to REUC */
...@@ -1142,7 +1146,7 @@ int git_index_add_bypath(git_index *index, const char *path) ...@@ -1142,7 +1146,7 @@ int git_index_add_bypath(git_index *index, const char *path)
assert(index && path); assert(index && path);
if ((ret = index_entry_init(&entry, index, path)) < 0 || if ((ret = index_entry_init(&entry, index, path)) < 0 ||
(ret = index_insert(index, &entry, 1)) < 0) (ret = index_insert(index, &entry, 1, false)) < 0)
return ret; return ret;
/* Adding implies conflict was resolved, move conflict entries to REUC */ /* Adding implies conflict was resolved, move conflict entries to REUC */
...@@ -1182,7 +1186,7 @@ int git_index_add(git_index *index, const git_index_entry *source_entry) ...@@ -1182,7 +1186,7 @@ int git_index_add(git_index *index, const git_index_entry *source_entry)
} }
if ((ret = index_entry_dup(&entry, INDEX_OWNER(index), source_entry)) < 0 || if ((ret = index_entry_dup(&entry, INDEX_OWNER(index), source_entry)) < 0 ||
(ret = index_insert(index, &entry, 1)) < 0) (ret = index_insert(index, &entry, 1, true)) < 0)
return ret; return ret;
git_tree_cache_invalidate_path(index->tree, entry->path); git_tree_cache_invalidate_path(index->tree, entry->path);
...@@ -1313,7 +1317,7 @@ int git_index_conflict_add(git_index *index, ...@@ -1313,7 +1317,7 @@ int git_index_conflict_add(git_index *index,
/* Make sure stage is correct */ /* Make sure stage is correct */
GIT_IDXENTRY_STAGE_SET(entries[i], i + 1); GIT_IDXENTRY_STAGE_SET(entries[i], i + 1);
if ((ret = index_insert(index, &entries[i], 1)) < 0) if ((ret = index_insert(index, &entries[i], 1, true)) < 0)
goto on_error; goto on_error;
entries[i] = NULL; /* don't free if later entry fails */ entries[i] = NULL; /* don't free if later entry fails */
...@@ -2537,7 +2541,7 @@ int git_index_add_all( ...@@ -2537,7 +2541,7 @@ int git_index_add_all(
entry->id = blobid; entry->id = blobid;
/* add working directory item to index */ /* add working directory item to index */
if ((error = index_insert(index, &entry, 1)) < 0) if ((error = index_insert(index, &entry, 1, false)) < 0)
break; break;
git_tree_cache_invalidate_path(index->tree, wd->path); git_tree_cache_invalidate_path(index->tree, wd->path);
......
...@@ -153,6 +153,85 @@ void test_index_filemodes__trusted(void) ...@@ -153,6 +153,85 @@ void test_index_filemodes__trusted(void)
git_index_free(index); git_index_free(index);
} }
#define add_entry_and_check_mode(I,FF,X) add_entry_and_check_mode_(I,FF,X,__FILE__,__LINE__)
static void add_entry_and_check_mode_(
git_index *index, bool from_file, git_filemode_t mode,
const char *file, int line)
{
size_t pos;
const git_index_entry* entry;
git_index_entry new_entry;
/* If old_filename exists, we copy that to the new file, and test
* git_index_add(), otherwise create a new entry testing git_index_add_frombuffer
*/
if (from_file)
{
clar__assert(!git_index_find(&pos, index, "exec_off"),
file, line, "Cannot find original index entry", NULL, 1);
entry = git_index_get_byindex(index, pos);
memcpy(&new_entry, entry, sizeof(new_entry));
}
else
memset(&new_entry, 0x0, sizeof(git_index_entry));
new_entry.path = "filemodes/explicit_test";
new_entry.mode = mode;
if (from_file)
{
clar__assert(!git_index_add(index, &new_entry),
file, line, "Cannot add index entry", NULL, 1);
}
else
{
const char *content = "hey there\n";
clar__assert(!git_index_add_frombuffer(index, &new_entry, content, strlen(content)),
file, line, "Cannot add index entry from buffer", NULL, 1);
}
clar__assert(!git_index_find(&pos, index, "filemodes/explicit_test"),
file, line, "Cannot find new index entry", NULL, 1);
entry = git_index_get_byindex(index, pos);
clar__assert_equal(file, line, "Expected mode does not match index",
1, "%07o", (unsigned int)entry->mode, (unsigned int)mode);
}
void test_index_filemodes__explicit(void)
{
git_index *index;
/* These tests should run and work everywhere, as the filemode is
* given explicitly to git_index_add or git_index_add_frombuffer
*/
cl_repo_set_bool(g_repo, "core.filemode", false);
cl_git_pass(git_repository_index(&index, g_repo));
/* Each of these tests keeps overwriting the same file in the index. */
/* 1 - add new 0644 entry */
add_entry_and_check_mode(index, true, GIT_FILEMODE_BLOB);
/* 2 - add 0755 entry over existing 0644 */
add_entry_and_check_mode(index, true, GIT_FILEMODE_BLOB_EXECUTABLE);
/* 3 - add 0644 entry over existing 0755 */
add_entry_and_check_mode(index, true, GIT_FILEMODE_BLOB);
/* 4 - add 0755 buffer entry over existing 0644 */
add_entry_and_check_mode(index, false, GIT_FILEMODE_BLOB_EXECUTABLE);
/* 5 - add 0644 buffer entry over existing 0755 */
add_entry_and_check_mode(index, false, GIT_FILEMODE_BLOB);
git_index_free(index);
}
void test_index_filemodes__invalid(void) void test_index_filemodes__invalid(void)
{ {
git_index *index; git_index *index;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment