diff --git a/src/index.c b/src/index.c index 0903770bb..bce16348c 100644 --- a/src/index.c +++ b/src/index.c @@ -18,6 +18,7 @@ #include "ignore.h" #include "blob.h" #include "idxmap.h" +#include "diff.h" #include "git2/odb.h" #include "git2/oid.h" @@ -752,7 +753,9 @@ static int truncate_racily_clean(git_index *index) int error; git_index_entry *entry; git_diff_options diff_opts = GIT_DIFF_OPTIONS_INIT; - git_diff *diff; + git_diff *diff = NULL; + git_vector paths = GIT_VECTOR_INIT; + git_diff_delta *delta; /* Nothing to do if there's no repo to talk about */ if (!INDEX_OWNER(index)) @@ -764,22 +767,33 @@ static int truncate_racily_clean(git_index *index) diff_opts.flags |= GIT_DIFF_INCLUDE_TYPECHANGE | GIT_DIFF_IGNORE_SUBMODULES | GIT_DIFF_DISABLE_PATHSPEC_MATCH; git_vector_foreach(&index->entries, i, entry) { - if (!is_racy_timestamp(&index->stamp.mtime, entry)) - continue; - - /* TODO: use the (non-fnmatching) filelist iterator */ - diff_opts.pathspec.count = 1; - diff_opts.pathspec.strings = (char **) &entry->path; - - if ((error = git_diff_index_to_workdir(&diff, INDEX_OWNER(index), index, &diff_opts)) < 0) - return error; - - if (git_diff_num_deltas(diff) > 0) - entry->file_size = 0; - - git_diff_free(diff); + if ((entry->flags_extended & GIT_IDXENTRY_UPTODATE) == 0 && + is_racy_timestamp(&index->stamp.mtime, entry)) + git_vector_insert(&paths, (char *)entry->path); } + if (paths.length == 0) + goto done; + + diff_opts.pathspec.count = paths.length; + diff_opts.pathspec.strings = (char **)paths.contents; + + if ((error = git_diff_index_to_workdir(&diff, INDEX_OWNER(index), index, &diff_opts)) < 0) + return error; + + git_vector_foreach(&diff->deltas, i, delta) { + entry = (git_index_entry *)git_index_get_bypath(index, delta->old_file.path, 0); + + /* Ensure that we have a stage 0 for this file (ie, it's not a + * conflict), otherwise smudging it is quite pointless. + */ + if (entry) + entry->file_size = 0; + } + +done: + git_diff_free(diff); + git_vector_free(&paths); return 0; } @@ -989,20 +1003,11 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out, static void index_entry_cpy( git_index_entry *tgt, - git_index *index, - const git_index_entry *src, - bool update_path) + const git_index_entry *src) { const char *tgt_path = tgt->path; memcpy(tgt, src, sizeof(*tgt)); - - /* keep the existing path buffer, but update the path to the one - * given by the caller, if we trust it. - */ tgt->path = tgt_path; - - if (index->ignore_case && update_path) - memcpy((char *)tgt->path, src->path, strlen(tgt->path)); } static int index_entry_dup( @@ -1010,18 +1015,32 @@ static int index_entry_dup( git_index *index, const git_index_entry *src) { - git_index_entry *entry; - - if (!src) { - *out = NULL; - return 0; - } - - if (index_entry_create(&entry, INDEX_OWNER(index), src->path) < 0) + if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0) return -1; - index_entry_cpy(entry, index, src, false); - *out = entry; + index_entry_cpy(*out, src); + return 0; +} + +static void index_entry_cpy_nocache( + git_index_entry *tgt, + const git_index_entry *src) +{ + git_oid_cpy(&tgt->id, &src->id); + tgt->mode = src->mode; + tgt->flags = src->flags; + tgt->flags_extended = (src->flags_extended & GIT_IDXENTRY_EXTENDED_FLAGS); +} + +static int index_entry_dup_nocache( + git_index_entry **out, + git_index *index, + const git_index_entry *src) +{ + if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0) + return -1; + + index_entry_cpy_nocache(*out, src); return 0; } @@ -1282,6 +1301,9 @@ static int index_insert( else entry->flags |= GIT_IDXENTRY_NAMEMASK; + /* this entry is now up-to-date and should not be checked for raciness */ + entry->flags_extended |= GIT_IDXENTRY_UPTODATE; + if (git_mutex_lock(&index->lock) < 0) { giterr_set(GITERR_OS, "Unable to acquire index lock"); return -1; @@ -1314,8 +1336,13 @@ static int index_insert( * and return it in place of the passed in one. */ else if (existing) { - if (replace) - index_entry_cpy(existing, index, entry, trust_path); + if (replace) { + index_entry_cpy(existing, entry); + + if (trust_path) + memcpy((char *)existing->path, entry->path, strlen(entry->path)); + } + index_entry_free(entry); *entry_ptr = entry = existing; } @@ -1692,9 +1719,12 @@ int git_index_conflict_add(git_index *index, assert (index); - if ((ret = index_entry_dup(&entries[0], index, ancestor_entry)) < 0 || - (ret = index_entry_dup(&entries[1], index, our_entry)) < 0 || - (ret = index_entry_dup(&entries[2], index, their_entry)) < 0) + if ((ancestor_entry && + (ret = index_entry_dup(&entries[0], index, ancestor_entry)) < 0) || + (our_entry && + (ret = index_entry_dup(&entries[1], index, our_entry)) < 0) || + (their_entry && + (ret = index_entry_dup(&entries[2], index, their_entry)) < 0)) goto on_error; /* Validate entries */ @@ -2545,7 +2575,8 @@ static int write_disk_entry(git_filebuf *file, git_index_entry *entry) if (entry->flags & GIT_IDXENTRY_EXTENDED) { struct entry_long *ondisk_ext; ondisk_ext = (struct entry_long *)ondisk; - ondisk_ext->flags_extended = htons(entry->flags_extended); + ondisk_ext->flags_extended = htons(entry->flags_extended & + GIT_IDXENTRY_EXTENDED_FLAGS); path = ondisk_ext->path; } else @@ -2728,6 +2759,15 @@ static int write_tree_extension(git_index *index, git_filebuf *file) return error; } +static void clear_uptodate(git_index *index) +{ + git_index_entry *entry; + size_t i; + + git_vector_foreach(&index->entries, i, entry) + entry->flags_extended &= ~GIT_IDXENTRY_UPTODATE; +} + static int write_index(git_oid *checksum, git_index *index, git_filebuf *file) { git_oid hash_final; @@ -2767,7 +2807,13 @@ static int write_index(git_oid *checksum, git_index *index, git_filebuf *file) git_oid_cpy(checksum, &hash_final); /* write it at the end of the file */ - return git_filebuf_write(file, hash_final.id, GIT_OID_RAWSZ); + if (git_filebuf_write(file, hash_final.id, GIT_OID_RAWSZ) < 0) + return -1; + + /* file entries are no longer up to date */ + clear_uptodate(index); + + return 0; } int git_index_entry_stage(const git_index_entry *entry) @@ -2816,7 +2862,7 @@ static int read_tree_cb( entry->mode == old_entry->mode && git_oid_equal(&entry->id, &old_entry->id)) { - index_entry_cpy(entry, data->index, old_entry, false); + index_entry_cpy(entry, old_entry); entry->flags_extended = 0; } @@ -2941,7 +2987,10 @@ int git_index_read_index( goto done; while (true) { - git_index_entry *add_entry = NULL, *remove_entry = NULL; + git_index_entry + *dup_entry = NULL, + *add_entry = NULL, + *remove_entry = NULL; int diff; if (old_entry && new_entry) @@ -2956,8 +3005,7 @@ int git_index_read_index( if (diff < 0) { remove_entry = (git_index_entry *)old_entry; } else if (diff > 0) { - if ((error = index_entry_dup(&add_entry, index, new_entry)) < 0) - goto done; + dup_entry = (git_index_entry *)new_entry; } else { /* Path and stage are equal, if the OID is equal, keep it to * keep the stat cache data. @@ -2965,13 +3013,16 @@ int git_index_read_index( if (git_oid_equal(&old_entry->id, &new_entry->id)) { add_entry = (git_index_entry *)old_entry; } else { - if ((error = index_entry_dup(&add_entry, index, new_entry)) < 0) - goto done; - + dup_entry = (git_index_entry *)new_entry; remove_entry = (git_index_entry *)old_entry; } } + if (dup_entry) { + if ((error = index_entry_dup_nocache(&add_entry, index, dup_entry)) < 0) + goto done; + } + if (add_entry) { if ((error = git_vector_insert(&new_entries, add_entry)) == 0) INSERT_IN_MAP_EX(index, new_entries_map, add_entry, error); diff --git a/tests/checkout/crlf.c b/tests/checkout/crlf.c index 8e77d0845..347c22198 100644 --- a/tests/checkout/crlf.c +++ b/tests/checkout/crlf.c @@ -278,6 +278,7 @@ void test_checkout_crlf__autocrlf_true_index_size_is_filtered_size(void) void test_checkout_crlf__with_ident(void) { git_index *index; + git_index_entry *entry; git_blob *blob; git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; opts.checkout_strategy = GIT_CHECKOUT_FORCE; @@ -310,14 +311,14 @@ void test_checkout_crlf__with_ident(void) /* check that blobs have $Id$ */ - cl_git_pass(git_blob_lookup(&blob, g_repo, - & git_index_get_bypath(index, "lf.ident", 0)->id)); + cl_assert((entry = git_index_get_bypath(index, "lf.ident", 0))); + cl_git_pass(git_blob_lookup(&blob, g_repo, &entry->id)); cl_assert_equal_s( ALL_LF_TEXT_RAW "\n$Id$\n", git_blob_rawcontent(blob)); git_blob_free(blob); - cl_git_pass(git_blob_lookup(&blob, g_repo, - & git_index_get_bypath(index, "more2.identcrlf", 0)->id)); + cl_assert((entry = git_index_get_bypath(index, "more2.identcrlf", 0))); + cl_git_pass(git_blob_lookup(&blob, g_repo, &entry->id)); cl_assert_equal_s( "\n$Id$\n" MORE_CRLF_TEXT_AS_LF, git_blob_rawcontent(blob)); git_blob_free(blob); diff --git a/tests/index/racy.c b/tests/index/racy.c index df25c851c..f7440c3af 100644 --- a/tests/index/racy.c +++ b/tests/index/racy.c @@ -100,13 +100,13 @@ void test_index_racy__write_index_just_after_file(void) git_index_free(index); } -void test_index_racy__empty_file_after_smudge(void) + +static void setup_race(void) { - git_index *index; - git_diff *diff; git_buf path = GIT_BUF_INIT; - int i, found_race = 0; + git_index *index; const git_index_entry *entry; + int i, found_race = 0; /* Make sure we do have a timestamp */ cl_git_pass(git_repository_index__weakptr(&index, g_repo)); @@ -131,17 +131,198 @@ void test_index_racy__empty_file_after_smudge(void) found_race = 1; break; } - } if (!found_race) cl_fail("failed to find race after 10 attempts"); + git_buf_free(&path); +} + +void test_index_racy__smudges_index_entry_on_save(void) +{ + git_index *index; + const git_index_entry *entry; + + setup_race(); + + /* write the index, which will smudge anything that had the same timestamp + * as the index when the index was loaded. that way future loads of the + * index (with the new timestamp) will know that these files were not + * clean. + */ + + cl_git_pass(git_repository_index__weakptr(&index, g_repo)); + cl_git_pass(git_index_write(index)); + + cl_assert(entry = git_index_get_bypath(index, "A", 0)); cl_assert_equal_i(0, entry->file_size); +} + +void test_index_racy__detects_diff_of_change_in_identical_timestamp(void) +{ + git_index *index; + git_diff *diff; + + cl_git_pass(git_repository_index__weakptr(&index, g_repo)); + + setup_race(); cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, NULL)); cl_assert_equal_i(1, git_diff_num_deltas(diff)); - git_buf_free(&path); git_diff_free(diff); } + +static void setup_uptodate_files(void) +{ + git_buf path = GIT_BUF_INIT; + git_index *index; + git_index_entry new_entry = {0}; + + cl_git_pass(git_repository_index(&index, g_repo)); + + cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "A")); + cl_git_mkfile(path.ptr, "A"); + + /* Put 'A' into the index */ + cl_git_pass(git_index_add_bypath(index, "A")); + + /* Put 'B' into the index */ + new_entry.path = "B"; + new_entry.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_index_add(index, &new_entry)); + + /* Put 'C' into the index */ + new_entry.path = "C"; + new_entry.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_index_add_frombuffer(index, &new_entry, "hello!\n", 7)); + + git_index_free(index); + git_buf_free(&path); +} + +void test_index_racy__adding_to_index_is_uptodate(void) +{ + git_index *index; + const git_index_entry *entry; + + setup_uptodate_files(); + + cl_git_pass(git_repository_index(&index, g_repo)); + + /* ensure that they're all uptodate */ + cl_assert((entry = git_index_get_bypath(index, "A", 0))); + cl_assert_equal_i(GIT_IDXENTRY_UPTODATE, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "B", 0))); + cl_assert_equal_i(GIT_IDXENTRY_UPTODATE, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "C", 0))); + cl_assert_equal_i(GIT_IDXENTRY_UPTODATE, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_git_pass(git_index_write(index)); + + git_index_free(index); +} + +void test_index_racy__reading_clears_uptodate_bit(void) +{ + git_index *index; + const git_index_entry *entry; + + setup_uptodate_files(); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_write(index)); + + cl_git_pass(git_index_read(index, true)); + + /* ensure that no files are uptodate */ + cl_assert((entry = git_index_get_bypath(index, "A", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "B", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "C", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + git_index_free(index); +} + +void test_index_racy__read_tree_clears_uptodate_bit(void) +{ + git_index *index; + git_tree *tree; + const git_index_entry *entry; + git_oid id; + + setup_uptodate_files(); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_write_tree_to(&id, index, g_repo)); + cl_git_pass(git_tree_lookup(&tree, g_repo, &id)); + cl_git_pass(git_index_read_tree(index, tree)); + + /* ensure that no files are uptodate */ + cl_assert((entry = git_index_get_bypath(index, "A", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "B", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(index, "C", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + git_tree_free(tree); + git_index_free(index); +} + +void test_index_racy__read_index_smudges(void) +{ + git_index *index, *newindex; + const git_index_entry *entry; + + /* if we are reading an index into our new index, ensure that any + * racy entries in the index that we're reading are smudged so that + * we don't propagate their timestamps without further investigation. + */ + setup_race(); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_new(&newindex)); + cl_git_pass(git_index_read_index(newindex, index)); + + cl_assert(entry = git_index_get_bypath(newindex, "A", 0)); + cl_assert_equal_i(0, entry->file_size); + + git_index_free(index); + git_index_free(newindex); +} + +void test_index_racy__read_index_clears_uptodate_bit(void) +{ + git_index *index, *newindex; + const git_index_entry *entry; + git_oid id; + + setup_uptodate_files(); + + cl_git_pass(git_repository_index(&index, g_repo)); + cl_git_pass(git_index_new(&newindex)); + cl_git_pass(git_index_read_index(newindex, index)); + + /* ensure that files brought in from the other index are not uptodate */ + cl_assert((entry = git_index_get_bypath(newindex, "A", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(newindex, "B", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + cl_assert((entry = git_index_get_bypath(newindex, "C", 0))); + cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE)); + + git_index_free(index); + git_index_free(newindex); +}