From 10df661b8cd2a7e4751f8344633825dfc88be169 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 29 Sep 2015 14:16:51 -0400 Subject: [PATCH 01/23] index: test that add_bypath preserves mode --- tests/index/bypath.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/index/bypath.c b/tests/index/bypath.c index b152b091797..d26273f76fe 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -240,3 +240,26 @@ void test_index_bypath__add_honors_existing_case_4(void) cl_assert_equal_s("just_a_dir/a/b/Z/y/X/foo.txt", entry->path); } +void test_index_bypath__add_honors_mode(void) +{ + git_index_entry *entry, new_entry; + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + + memcpy(&new_entry, entry, sizeof(git_index_entry)); + new_entry.path = "README.txt"; + new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE; + + cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE)); + + cl_git_pass(git_index_add(g_idx, &new_entry)); + cl_git_pass(git_index_write(g_idx)); + + cl_git_rewritefile("submod2/README.txt", "Modified but still executable"); + + cl_git_pass(git_index_add_bypath(g_idx, "README.txt")); + cl_git_pass(git_index_write(g_idx)); + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode); +} From 21515f228b739a3ecd2237bafbba50e8d219d8dd Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 29 Sep 2015 15:49:16 -0400 Subject: [PATCH 02/23] index: also try conflict mode when inserting When we do not trust the on-disk mode, we use the mode of an existing index entry. This allows us to preserve executable bits on platforms that do not honor them on the filesystem. If there is no stage 0 index entry, also look at conflicts to attempt to answer this question: prefer the data from the 'ours' side, then the 'theirs' side before falling back to the common ancestor. --- include/git2/index.h | 28 ++++++++++++---- src/index.c | 77 +++++++++++++++++++++++++++++++++++--------- tests/index/bypath.c | 67 +++++++++++++++++++++++++++++++++++++- 3 files changed, 149 insertions(+), 23 deletions(-) diff --git a/include/git2/index.h b/include/git2/index.h index 176ba301ec1..466765be3b1 100644 --- a/include/git2/index.h +++ b/include/git2/index.h @@ -154,13 +154,27 @@ typedef enum { GIT_INDEX_ADD_CHECK_PATHSPEC = (1u << 2), } git_index_add_option_t; -/** - * Match any index stage. - * - * Some index APIs take a stage to match; pass this value to match - * any entry matching the path regardless of stage. - */ -#define GIT_INDEX_STAGE_ANY -1 +typedef enum { + /** + * Match any index stage. + * + * Some index APIs take a stage to match; pass this value to match + * any entry matching the path regardless of stage. + */ + GIT_INDEX_STAGE_ANY = -1, + + /** A normal staged file in the index. */ + GIT_INDEX_STAGE_NORMAL = 0, + + /** The ancestor side of a conflict. */ + GIT_INDEX_STAGE_ANCESTOR = 1, + + /** The "ours" side of a conflict. */ + GIT_INDEX_STAGE_OURS = 2, + + /** The "theirs" side of a conflict. */ + GIT_INDEX_STAGE_THEIRS = 3, +} git_index_stage_t; /** @name Index File Functions * diff --git a/src/index.c b/src/index.c index 2e8934780ae..c0be5b90d12 100644 --- a/src/index.c +++ b/src/index.c @@ -1114,7 +1114,9 @@ static int check_file_directory_collision(git_index *index, } static int canonicalize_directory_path( - git_index *index, git_index_entry *entry) + git_index *index, + git_index_entry *entry, + git_index_entry *existing) { const git_index_entry *match, *best = NULL; char *search, *sep; @@ -1124,8 +1126,8 @@ static int canonicalize_directory_path( return 0; /* item already exists in the index, simply re-use the existing case */ - if ((match = git_index_get_bypath(index, entry->path, 0)) != NULL) { - memcpy((char *)entry->path, match->path, strlen(entry->path)); + if (existing) { + memcpy((char *)entry->path, existing->path, strlen(existing->path)); return 0; } @@ -1190,6 +1192,52 @@ static int index_no_dups(void **old, void *new) return GIT_EEXISTS; } +static void index_existing_and_best( + const git_index_entry **existing, + size_t *existing_position, + const git_index_entry **best, + git_index *index, + const git_index_entry *entry) +{ + const git_index_entry *e; + size_t pos; + int error; + + error = index_find(&pos, + index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false); + + if (error == 0) { + *existing = index->entries.contents[pos]; + *existing_position = pos; + *best = index->entries.contents[pos]; + return; + } + + *existing = NULL; + *existing_position = 0; + *best = NULL; + + if (GIT_IDXENTRY_STAGE(entry) == 0) { + for (; pos < index->entries.length; pos++) { + int (*strcomp)(const char *a, const char *b) = + index->ignore_case ? git__strcasecmp : git__strcmp; + + e = index->entries.contents[pos]; + + if (strcomp(entry->path, e->path) != 0) + break; + + if (GIT_IDXENTRY_STAGE(e) == GIT_INDEX_STAGE_ANCESTOR) { + *best = e; + continue; + } else { + *best = e; + break; + } + } + } +} + /* index_insert takes ownership of the new entry - if it can't insert * 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 @@ -1208,7 +1256,7 @@ static int index_insert( { int error = 0; size_t path_length, position; - git_index_entry *existing = NULL, *entry; + git_index_entry *existing, *best, *entry; assert(index && entry_ptr); @@ -1231,20 +1279,19 @@ static int index_insert( git_vector_sort(&index->entries); - /* look if an entry with this path already exists */ - if (!index_find( - &position, index, entry->path, 0, GIT_IDXENTRY_STAGE(entry), false)) { - existing = index->entries.contents[position]; - /* 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); - } + /* look if an entry with this path already exists, either staged, or (if + * this entry is a regular staged item) as the "ours" side of a conflict. + */ + index_existing_and_best(&existing, &position, &best, index, entry); + + /* update the file mode */ + entry->mode = trust_mode ? + git_index__create_mode(entry->mode) : + index_merge_mode(index, best, entry->mode); /* canonicalize the directory name */ if (!trust_path) - error = canonicalize_directory_path(index, entry); + error = canonicalize_directory_path(index, entry, best); /* look for tree / blob name collisions, removing conflicts if requested */ if (!error) diff --git a/tests/index/bypath.c b/tests/index/bypath.c index d26273f76fe..0c10cfe4c64 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -242,7 +242,8 @@ void test_index_bypath__add_honors_existing_case_4(void) void test_index_bypath__add_honors_mode(void) { - git_index_entry *entry, new_entry; + const git_index_entry *entry; + git_index_entry new_entry; cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); @@ -263,3 +264,67 @@ void test_index_bypath__add_honors_mode(void) cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode); } + +void test_index_bypath__add_honors_conflict_mode(void) +{ + const git_index_entry *entry; + git_index_entry new_entry; + int stage = 0; + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + + memcpy(&new_entry, entry, sizeof(git_index_entry)); + new_entry.path = "README.txt"; + new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE; + + cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE)); + + cl_git_pass(git_index_remove_bypath(g_idx, "README.txt")); + + for (stage = 1; stage <= 3; stage++) { + new_entry.flags = stage << GIT_IDXENTRY_STAGESHIFT; + cl_git_pass(git_index_add(g_idx, &new_entry)); + } + + cl_git_pass(git_index_write(g_idx)); + + cl_git_rewritefile("submod2/README.txt", "Modified but still executable"); + + cl_git_pass(git_index_add_bypath(g_idx, "README.txt")); + cl_git_pass(git_index_write(g_idx)); + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode); +} + +void test_index_bypath__add_honors_conflict_case(void) +{ + const git_index_entry *entry; + git_index_entry new_entry; + int stage = 0; + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + + memcpy(&new_entry, entry, sizeof(git_index_entry)); + new_entry.path = "README.txt"; + new_entry.mode = GIT_FILEMODE_BLOB_EXECUTABLE; + + cl_must_pass(p_chmod("submod2/README.txt", GIT_FILEMODE_BLOB_EXECUTABLE)); + + cl_git_pass(git_index_remove_bypath(g_idx, "README.txt")); + + for (stage = 1; stage <= 3; stage++) { + new_entry.flags = stage << GIT_IDXENTRY_STAGESHIFT; + cl_git_pass(git_index_add(g_idx, &new_entry)); + } + + cl_git_pass(git_index_write(g_idx)); + + cl_git_rewritefile("submod2/README.txt", "Modified but still executable"); + + cl_git_pass(git_index_add_bypath(g_idx, "README.txt")); + cl_git_pass(git_index_write(g_idx)); + + cl_assert((entry = git_index_get_bypath(g_idx, "README.txt", 0)) != NULL); + cl_assert_equal_i(GIT_FILEMODE_BLOB_EXECUTABLE, entry->mode); +} From 4bc9b74c14cd2d37de12295cc6ebfd90274e222b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 28 Sep 2015 16:24:50 -0400 Subject: [PATCH 03/23] GITERR_CHECK_ALLOC_ADDn: multi-arg adders --- src/common.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/common.h b/src/common.h index 6dca36fbd4d..7170df91a54 100644 --- a/src/common.h +++ b/src/common.h @@ -208,6 +208,15 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v #define GITERR_CHECK_ALLOC_ADD(out, one, two) \ if (GIT_ADD_SIZET_OVERFLOW(out, one, two)) { return -1; } +#define GITERR_CHECK_ALLOC_ADD3(out, one, two, three) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), three)) { return -1; } + +#define GITERR_CHECK_ALLOC_ADD4(out, one, two, three, four) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), three) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), four)) { return -1; } + /** Check for multiplicative overflow, failing if it would occur. */ #define GITERR_CHECK_ALLOC_MULTIPLY(out, nelem, elsize) \ if (GIT_MULTIPLY_SIZET_OVERFLOW(out, nelem, elsize)) { return -1; } From 46c0e6e3c169f8d76ed739444bc51c4b3f74969b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 28 Sep 2015 16:34:29 -0400 Subject: [PATCH 04/23] xdiff: convert size variables to size_t --- src/xdiff/xdiffi.c | 12 ++-- src/xdiff/xhistogram.c | 6 +- src/xdiff/xmerge.c | 142 ++++++++++++++++++++++++++++------------- 3 files changed, 110 insertions(+), 50 deletions(-) diff --git a/src/xdiff/xdiffi.c b/src/xdiff/xdiffi.c index 0620e5fff4c..f4d01b48c56 100644 --- a/src/xdiff/xdiffi.c +++ b/src/xdiff/xdiffi.c @@ -21,7 +21,8 @@ */ #include "xinclude.h" - +#include "common.h" +#include "integer.h" #define XDL_MAX_COST_MIN 256 @@ -323,7 +324,7 @@ int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1, int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdfenv_t *xe) { - long ndiags; + size_t ndiags, allocsize; long *kvd, *kvdf, *kvdb; xdalgoenv_t xenv; diffdata_t dd1, dd2; @@ -343,9 +344,12 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, * Allocate and setup K vectors to be used by the differential algorithm. * One is to store the forward path and one to store the backward path. */ - ndiags = xe->xdf1.nreff + xe->xdf2.nreff + 3; - if (!(kvd = (long *) xdl_malloc((2 * ndiags + 2) * sizeof(long)))) { + GITERR_CHECK_ALLOC_ADD3(&ndiags, xe->xdf1.nreff, xe->xdf2.nreff, 3); + GITERR_CHECK_ALLOC_MULTIPLY(&allocsize, ndiags, 2); + GITERR_CHECK_ALLOC_ADD(&allocsize, allocsize, 2); + GITERR_CHECK_ALLOC_MULTIPLY(&allocsize, allocsize, sizeof(long)); + if (!(kvd = (long *) xdl_malloc(allocsize))) { xdl_free_env(xe); return -1; } diff --git a/src/xdiff/xhistogram.c b/src/xdiff/xhistogram.c index 500d8112a9d..0c2edb89c59 100644 --- a/src/xdiff/xhistogram.c +++ b/src/xdiff/xhistogram.c @@ -44,6 +44,7 @@ #include "xinclude.h" #include "xtypes.h" #include "xdiff.h" +#include "common.h" #define MAX_PTR UINT_MAX #define MAX_CNT UINT_MAX @@ -271,7 +272,7 @@ static int histogram_diff( { struct histindex index; struct region lcs; - unsigned int sz; + size_t sz; int result = -1; if (count1 <= 0 && count2 <= 0) @@ -302,7 +303,8 @@ static int histogram_diff( index.table_bits = xdl_hashbits(count1); sz = index.records_size = 1 << index.table_bits; - sz *= sizeof(struct record *); + GITERR_CHECK_ALLOC_MULTIPLY(&sz, sz, sizeof(struct record *)); + if (!(index.records = (struct record **) xdl_malloc(sz))) goto cleanup; memset(index.records, 0, sz); diff --git a/src/xdiff/xmerge.c b/src/xdiff/xmerge.c index b11e5981792..7b7e0e2d36e 100644 --- a/src/xdiff/xmerge.c +++ b/src/xdiff/xmerge.c @@ -21,6 +21,7 @@ */ #include "xinclude.h" +#include "common.h" typedef struct s_xdmerge { struct s_xdmerge *next; @@ -109,59 +110,74 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2, return 0; } -static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest) +static int xdl_recs_copy_0(size_t *out, int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest) { xrecord_t **recs; - int size = 0; + size_t size = 0; + + *out = 0; recs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i; if (count < 1) return 0; - for (i = 0; i < count; size += recs[i++]->size) + for (i = 0; i < count; ) { if (dest) memcpy(dest + size, recs[i]->ptr, recs[i]->size); + + GITERR_CHECK_ALLOC_ADD(&size, size, recs[i++]->size); + } + if (add_nl) { i = recs[count - 1]->size; if (i == 0 || recs[count - 1]->ptr[i - 1] != '\n') { if (dest) dest[size] = '\n'; - size++; + + GITERR_CHECK_ALLOC_ADD(&size, size, 1); } } - return size; + + *out = size; + return 0; } -static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest) +static int xdl_recs_copy(size_t *out, xdfenv_t *xe, int i, int count, int add_nl, char *dest) { - return xdl_recs_copy_0(0, xe, i, count, add_nl, dest); + return xdl_recs_copy_0(out, 0, xe, i, count, add_nl, dest); } -static int xdl_orig_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest) +static int xdl_orig_copy(size_t *out, xdfenv_t *xe, int i, int count, int add_nl, char *dest) { - return xdl_recs_copy_0(1, xe, i, count, add_nl, dest); + return xdl_recs_copy_0(out, 1, xe, i, count, add_nl, dest); } -static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1, +static int fill_conflict_hunk(size_t *out, xdfenv_t *xe1, const char *name1, xdfenv_t *xe2, const char *name2, const char *name3, - int size, int i, int style, + size_t size, int i, int style, xdmerge_t *m, char *dest, int marker_size) { int marker1_size = (name1 ? (int)strlen(name1) + 1 : 0); int marker2_size = (name2 ? (int)strlen(name2) + 1 : 0); int marker3_size = (name3 ? (int)strlen(name3) + 1 : 0); + size_t copied; + + *out = 0; if (marker_size <= 0) marker_size = DEFAULT_CONFLICT_MARKER_SIZE; /* Before conflicting part */ - size += xdl_recs_copy(xe1, i, m->i1 - i, 0, - dest ? dest + size : NULL); + if (xdl_recs_copy(&copied, xe1, i, m->i1 - i, 0, + dest ? dest + size : NULL) < 0) + return -1; + + GITERR_CHECK_ALLOC_ADD(&size, size, copied); if (!dest) { - size += marker_size + 1 + marker1_size; + GITERR_CHECK_ALLOC_ADD4(&size, size, marker_size, 1, marker1_size); } else { memset(dest + size, '<', marker_size); size += marker_size; @@ -174,13 +190,16 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1, } /* Postimage from side #1 */ - size += xdl_recs_copy(xe1, m->i1, m->chg1, 1, - dest ? dest + size : NULL); + if (xdl_recs_copy(&copied, xe1, m->i1, m->chg1, 1, + dest ? dest + size : NULL) < 0) + return -1; + + GITERR_CHECK_ALLOC_ADD(&size, size, copied); if (style == XDL_MERGE_DIFF3) { /* Shared preimage */ if (!dest) { - size += marker_size + 1 + marker3_size; + GITERR_CHECK_ALLOC_ADD4(&size, size, marker_size, 1, marker3_size); } else { memset(dest + size, '|', marker_size); size += marker_size; @@ -191,12 +210,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1, } dest[size++] = '\n'; } - size += xdl_orig_copy(xe1, m->i0, m->chg0, 1, - dest ? dest + size : NULL); + + if (xdl_orig_copy(&copied, xe1, m->i0, m->chg0, 1, + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); } if (!dest) { - size += marker_size + 1; + GITERR_CHECK_ALLOC_ADD3(&size, size, marker_size, 1); } else { memset(dest + size, '=', marker_size); size += marker_size; @@ -204,10 +226,14 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1, } /* Postimage from side #2 */ - size += xdl_recs_copy(xe2, m->i2, m->chg2, 1, - dest ? dest + size : NULL); + + if (xdl_recs_copy(&copied, xe2, m->i2, m->chg2, 1, + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); + if (!dest) { - size += marker_size + 1 + marker2_size; + GITERR_CHECK_ALLOC_ADD4(&size, size, marker_size, 1, marker2_size); } else { memset(dest + size, '>', marker_size); size += marker_size; @@ -218,46 +244,69 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1, } dest[size++] = '\n'; } - return size; + + *out = size; + return 0; } -static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1, +static int xdl_fill_merge_buffer(size_t *out, + xdfenv_t *xe1, const char *name1, xdfenv_t *xe2, const char *name2, const char *ancestor_name, int favor, xdmerge_t *m, char *dest, int style, int marker_size) { - int size, i; + size_t size, copied; + int i; + + *out = 0; for (size = i = 0; m; m = m->next) { if (favor && !m->mode) m->mode = favor; - if (m->mode == 0) - size = fill_conflict_hunk(xe1, name1, xe2, name2, + if (m->mode == 0) { + if (fill_conflict_hunk(&size, xe1, name1, xe2, name2, ancestor_name, size, i, style, m, dest, - marker_size); + marker_size) < 0) + return -1; + } else if (m->mode & 3) { /* Before conflicting part */ - size += xdl_recs_copy(xe1, i, m->i1 - i, 0, - dest ? dest + size : NULL); + if (xdl_recs_copy(&copied, xe1, i, m->i1 - i, 0, + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); + /* Postimage from side #1 */ - if (m->mode & 1) - size += xdl_recs_copy(xe1, m->i1, m->chg1, (m->mode & 2), - dest ? dest + size : NULL); + if (m->mode & 1) { + if (xdl_recs_copy(&copied, xe1, m->i1, m->chg1, (m->mode & 2), + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); + } + /* Postimage from side #2 */ - if (m->mode & 2) - size += xdl_recs_copy(xe2, m->i2, m->chg2, 0, - dest ? dest + size : NULL); + if (m->mode & 2) { + if (xdl_recs_copy(&copied, xe2, m->i2, m->chg2, 0, + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); + } } else continue; i = m->i1 + m->chg1; } - size += xdl_recs_copy(xe1, i, xe1->xdf2.nrec - i, 0, - dest ? dest + size : NULL); - return size; + + if (xdl_recs_copy(&copied, xe1, i, xe1->xdf2.nrec - i, 0, + dest ? dest + size : NULL) < 0) + return -1; + GITERR_CHECK_ALLOC_ADD(&size, size, copied); + + *out = size; + return 0; } /* @@ -551,19 +600,24 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, /* output */ if (result) { int marker_size = xmp->marker_size; - int size = xdl_fill_merge_buffer(xe1, name1, xe2, name2, + size_t size; + + if (xdl_fill_merge_buffer(&size, xe1, name1, xe2, name2, ancestor_name, favor, changes, NULL, style, - marker_size); + marker_size) < 0) + return -1; + result->ptr = xdl_malloc(size); if (!result->ptr) { xdl_cleanup_merge(changes); return -1; } result->size = size; - xdl_fill_merge_buffer(xe1, name1, xe2, name2, + if (xdl_fill_merge_buffer(&size, xe1, name1, xe2, name2, ancestor_name, favor, changes, - result->ptr, style, marker_size); + result->ptr, style, marker_size) < 0) + return -1; } return xdl_cleanup_merge(changes); } From e43520660c6dba6470af28ce3be822be401f5788 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 28 Sep 2015 18:25:24 -0400 Subject: [PATCH 05/23] merge_file: treat large files as binary xdiff craps the bed on large files. Treat very large files as binary, so that it doesn't even have to try. Refactor our merge binary handling to better match git.git, which looks for a NUL in the first 8000 bytes. --- src/merge.c | 55 -------------------------- src/merge_file.c | 96 +++++++++++++++++++++++++++++++++++---------- src/merge_file.h | 5 +-- src/xdiff/xdiff.h | 8 ++-- tests/merge/files.c | 88 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 82 deletions(-) diff --git a/src/merge.c b/src/merge.c index fc5088c82a5..89b8e85054d 100644 --- a/src/merge.c +++ b/src/merge.c @@ -61,12 +61,6 @@ struct merge_diff_df_data { git_merge_diff *prev_conflict; }; -GIT_INLINE(int) merge_diff_detect_binary( - bool *binary_out, - git_repository *repo, - const git_merge_diff *conflict); - - /* Merge base computation */ int merge_bases_many(git_commit_list **out, git_revwalk **walk_out, git_repository *repo, size_t length, const git_oid input_array[]) @@ -668,7 +662,6 @@ static int merge_conflict_resolve_automerge( git_odb *odb = NULL; git_oid automerge_oid; int error = 0; - bool binary = false; assert(resolved && diff_list && conflict); @@ -703,12 +696,6 @@ static int merge_conflict_resolve_automerge( strcmp(conflict->ancestor_entry.path, conflict->their_entry.path) != 0) return 0; - /* Reject binary conflicts */ - if ((error = merge_diff_detect_binary(&binary, diff_list->repo, conflict)) < 0) - return error; - if (binary) - return 0; - ancestor = GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->ancestor_entry) ? &conflict->ancestor_entry : NULL; ours = GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->our_entry) ? @@ -1314,48 +1301,6 @@ GIT_INLINE(int) merge_diff_detect_type( return 0; } -GIT_INLINE(int) merge_diff_detect_binary( - bool *binary_out, - git_repository *repo, - const git_merge_diff *conflict) -{ - git_blob *ancestor_blob = NULL, *our_blob = NULL, *their_blob = NULL; - int error = 0; - bool binary = false; - - if (GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->ancestor_entry)) { - if ((error = git_blob_lookup(&ancestor_blob, repo, &conflict->ancestor_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(ancestor_blob); - } - - if (!binary && - GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->our_entry)) { - if ((error = git_blob_lookup(&our_blob, repo, &conflict->our_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(our_blob); - } - - if (!binary && - GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->their_entry)) { - if ((error = git_blob_lookup(&their_blob, repo, &conflict->their_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(their_blob); - } - - *binary_out = binary; - -done: - git_blob_free(ancestor_blob); - git_blob_free(our_blob); - git_blob_free(their_blob); - - return error; -} - GIT_INLINE(int) index_entry_dup_pool( git_index_entry *out, git_pool *pool, diff --git a/src/merge_file.c b/src/merge_file.c index 6d89b089dfc..b174231cc2b 100644 --- a/src/merge_file.c +++ b/src/merge_file.c @@ -15,9 +15,15 @@ #include "git2/repository.h" #include "git2/object.h" #include "git2/index.h" +#include "git2/merge.h" #include "xdiff/xdiff.h" +/* only examine the first 8000 bytes for binaryness. + * https://github.com/git/git/blob/77bd3ea9f54f1584147b594abc04c26ca516d987/xdiff-interface.c#L197 + */ +#define GIT_MERGE_FILE_BINARY_SIZE 8000 + #define GIT_MERGE_FILE_SIDE_EXISTS(X) ((X)->mode != 0) GIT_INLINE(const char *) merge_file_best_path( @@ -100,7 +106,7 @@ static void merge_file_normalize_opts( } } -static int git_merge_file__from_inputs( +static int merge_file__xdiff( git_merge_file_result *out, const git_merge_file_input *ancestor, const git_merge_file_input *ours, @@ -189,6 +195,63 @@ static int git_merge_file__from_inputs( return error; } +static bool merge_file__is_binary(const git_merge_file_input *file) +{ + size_t len = file ? file->size : 0; + + if (len > GIT_MERGE_FILE_XDIFF_MAX) + return true; + if (len > GIT_MERGE_FILE_BINARY_SIZE) + len = GIT_MERGE_FILE_BINARY_SIZE; + + return len ? (memchr(file->ptr, 0, len) != NULL) : false; +} + +static int merge_file__binary( + git_merge_file_result *out, + const git_merge_file_input *ours, + const git_merge_file_input *theirs, + const git_merge_file_options *given_opts) +{ + const git_merge_file_input *favored = NULL; + + memset(out, 0x0, sizeof(git_merge_file_result)); + + if (given_opts && given_opts->favor == GIT_MERGE_FILE_FAVOR_OURS) + favored = ours; + else if (given_opts && given_opts->favor == GIT_MERGE_FILE_FAVOR_THEIRS) + favored = theirs; + else + goto done; + + if ((out->path = git__strdup(favored->path)) == NULL || + (out->ptr = git__malloc(favored->size)) == NULL) + goto done; + + memcpy((char *)out->ptr, favored->ptr, favored->size); + out->len = favored->size; + out->mode = favored->mode; + out->automergeable = 1; + +done: + return 0; +} + +static int merge_file__from_inputs( + git_merge_file_result *out, + const git_merge_file_input *ancestor, + const git_merge_file_input *ours, + const git_merge_file_input *theirs, + const git_merge_file_options *given_opts) +{ + if (merge_file__is_binary(ancestor) || + merge_file__is_binary(ours) || + merge_file__is_binary(theirs)) + return merge_file__binary(out, ours, theirs, given_opts); + + return merge_file__xdiff(out, ancestor, ours, theirs, given_opts); +} + static git_merge_file_input *git_merge_file__normalize_inputs( git_merge_file_input *out, const git_merge_file_input *given) @@ -223,7 +286,7 @@ int git_merge_file( ours = git_merge_file__normalize_inputs(&inputs[1], ours); theirs = git_merge_file__normalize_inputs(&inputs[2], theirs); - return git_merge_file__from_inputs(out, ancestor, ours, theirs, options); + return merge_file__from_inputs(out, ancestor, ours, theirs, options); } int git_merge_file_from_index( @@ -234,8 +297,8 @@ int git_merge_file_from_index( const git_index_entry *theirs, const git_merge_file_options *options) { - git_merge_file_input inputs[3] = { {0} }, - *ancestor_input = NULL, *our_input = NULL, *their_input = NULL; + git_merge_file_input *ancestor_ptr = NULL, + ancestor_input = {0}, our_input = {0}, their_input = {0}; git_odb *odb = NULL; git_odb_object *odb_object[3] = { 0 }; int error = 0; @@ -249,27 +312,20 @@ int git_merge_file_from_index( if (ancestor) { if ((error = git_merge_file__input_from_index( - &inputs[0], &odb_object[0], odb, ancestor)) < 0) + &ancestor_input, &odb_object[0], odb, ancestor)) < 0) goto done; - ancestor_input = &inputs[0]; + ancestor_ptr = &ancestor_input; } if ((error = git_merge_file__input_from_index( - &inputs[1], &odb_object[1], odb, ours)) < 0) - goto done; - - our_input = &inputs[1]; - - if ((error = git_merge_file__input_from_index( - &inputs[2], &odb_object[2], odb, theirs)) < 0) + &our_input, &odb_object[1], odb, ours)) < 0 || + (error = git_merge_file__input_from_index( + &their_input, &odb_object[2], odb, theirs)) < 0) goto done; - their_input = &inputs[2]; - - if ((error = git_merge_file__from_inputs(out, - ancestor_input, our_input, their_input, options)) < 0) - goto done; + error = merge_file__from_inputs(out, + ancestor_ptr, &our_input, &their_input, options); done: git_odb_object_free(odb_object[0]); @@ -286,7 +342,5 @@ void git_merge_file_result_free(git_merge_file_result *result) return; git__free((char *)result->path); - - /* xdiff uses malloc() not git_malloc, so we use free(), not git_free() */ - free((char *)result->ptr); + git__free((char *)result->ptr); } diff --git a/src/merge_file.h b/src/merge_file.h index 263391ee379..08ecfc09542 100644 --- a/src/merge_file.h +++ b/src/merge_file.h @@ -7,8 +7,7 @@ #ifndef INCLUDE_filediff_h__ #define INCLUDE_filediff_h__ -#include "xdiff/xdiff.h" - -#include "git2/merge.h" +/* xdiff cannot cope with large files, just treat them as binary */ +#define GIT_MERGE_FILE_XDIFF_MAX (1024UL * 1024 * 1023) #endif diff --git a/src/xdiff/xdiff.h b/src/xdiff/xdiff.h index e2f1e892b19..db5d59884a4 100644 --- a/src/xdiff/xdiff.h +++ b/src/xdiff/xdiff.h @@ -20,6 +20,8 @@ * */ +#include "util.h" + #if !defined(XDIFF_H) #define XDIFF_H @@ -106,9 +108,9 @@ typedef struct s_bdiffparam { } bdiffparam_t; -#define xdl_malloc(x) malloc(x) -#define xdl_free(ptr) free(ptr) -#define xdl_realloc(ptr,x) realloc(ptr,x) +#define xdl_malloc(x) git__malloc(x) +#define xdl_free(ptr) git__free(ptr) +#define xdl_realloc(ptr,x) git__realloc(ptr,x) void *xdl_mmfile_first(mmfile_t *mmf, long *size); long xdl_mmfile_size(mmfile_t *mmf); diff --git a/tests/merge/files.c b/tests/merge/files.c index 2fd90d0666f..b365d5a426b 100644 --- a/tests/merge/files.c +++ b/tests/merge/files.c @@ -3,6 +3,7 @@ #include "git2/merge.h" #include "buffer.h" #include "merge.h" +#include "merge_file.h" #include "merge_helpers.h" #include "refs.h" #include "fileops.h" @@ -288,3 +289,90 @@ void test_merge_files__doesnt_add_newline(void) git_merge_file_result_free(&result); } +void test_merge_files__skips_large_files(void) +{ + git_merge_file_input ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; + git_merge_file_result result = {0}; + + ours.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + ours.path = "testfile.txt"; + ours.mode = 0100755; + + theirs.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + theirs.path = "testfile.txt"; + theirs.mode = 0100755; + + cl_git_pass(git_merge_file(&result, NULL, &ours, &theirs, &opts)); + + cl_assert_equal_i(0, result.automergeable); + + git_merge_file_result_free(&result); +} + +void test_merge_files__skips_binaries(void) +{ + git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT, + ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_result result = {0}; + + ancestor.ptr = "ance\0stor\0"; + ancestor.size = 10; + ancestor.path = "ancestor.txt"; + ancestor.mode = 0100755; + + ours.ptr = "foo\0bar\0"; + ours.size = 8; + ours.path = "ours.txt"; + ours.mode = 0100755; + + theirs.ptr = "bar\0foo\0"; + theirs.size = 8; + theirs.path = "theirs.txt"; + theirs.mode = 0100644; + + cl_git_pass(git_merge_file(&result, &ancestor, &ours, &theirs, NULL)); + + cl_assert_equal_i(0, result.automergeable); + + git_merge_file_result_free(&result); +} + +void test_merge_files__handles_binaries_when_favored(void) +{ + git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT, + ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; + git_merge_file_result result = {0}; + + ancestor.ptr = "ance\0stor\0"; + ancestor.size = 10; + ancestor.path = "ancestor.txt"; + ancestor.mode = 0100755; + + ours.ptr = "foo\0bar\0"; + ours.size = 8; + ours.path = "ours.txt"; + ours.mode = 0100755; + + theirs.ptr = "bar\0foo\0"; + theirs.size = 8; + theirs.path = "theirs.txt"; + theirs.mode = 0100644; + + opts.favor = GIT_MERGE_FILE_FAVOR_OURS; + cl_git_pass(git_merge_file(&result, &ancestor, &ours, &theirs, &opts)); + + cl_assert_equal_i(1, result.automergeable); + + cl_assert_equal_s("ours.txt", result.path); + cl_assert_equal_i(0100755, result.mode); + + cl_assert_equal_i(ours.size, result.len); + cl_assert(memcmp(result.ptr, ours.ptr, ours.size) == 0); + + git_merge_file_result_free(&result); +} From 6c014bcc54fb9923490f9af917dc43e3661e5782 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 29 Sep 2015 12:18:17 -0400 Subject: [PATCH 06/23] diff: don't feed large files to xdiff --- src/checkout.c | 3 ++- src/diff_patch.c | 4 ++++ src/diff_xdiff.c | 7 +++++++ src/diff_xdiff.h | 5 +++++ src/merge.c | 1 - src/merge_file.c | 4 ++-- src/merge_file.h | 13 ------------- tests/merge/files.c | 6 +++--- 8 files changed, 23 insertions(+), 20 deletions(-) delete mode 100644 src/merge_file.h diff --git a/src/checkout.c b/src/checkout.c index 2a8bfd55848..632556622ef 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -18,6 +18,7 @@ #include "git2/submodule.h" #include "git2/sys/index.h" #include "git2/sys/filter.h" +#include "git2/merge.h" #include "refs.h" #include "repository.h" @@ -27,7 +28,7 @@ #include "diff.h" #include "pathspec.h" #include "buf_text.h" -#include "merge_file.h" +#include "diff_xdiff.h" #include "path.h" #include "attr.h" #include "pool.h" diff --git a/src/diff_patch.c b/src/diff_patch.c index 0628da6f21f..50faa3b3f7a 100644 --- a/src/diff_patch.c +++ b/src/diff_patch.c @@ -30,6 +30,10 @@ static void diff_patch_update_binary(git_patch *patch) (patch->nfile.file->flags & GIT_DIFF_FLAG_BINARY) != 0) patch->delta->flags |= GIT_DIFF_FLAG_BINARY; + else if (patch->ofile.file->size > GIT_XDIFF_MAX_SIZE || + patch->nfile.file->size > GIT_XDIFF_MAX_SIZE) + patch->delta->flags |= GIT_DIFF_FLAG_BINARY; + else if ((patch->ofile.file->flags & DIFF_FLAGS_NOT_BINARY) != 0 && (patch->nfile.file->flags & DIFF_FLAGS_NOT_BINARY) != 0) patch->delta->flags |= GIT_DIFF_FLAG_NOT_BINARY; diff --git a/src/diff_xdiff.c b/src/diff_xdiff.c index e5984f1c95e..1057df3aaca 100644 --- a/src/diff_xdiff.c +++ b/src/diff_xdiff.c @@ -4,6 +4,7 @@ * This file is part of libgit2, distributed under the GNU GPL v2 with * a Linking Exception. For full terms see the included COPYING file. */ +#include "git2/errors.h" #include "common.h" #include "diff.h" #include "diff_driver.h" @@ -208,6 +209,12 @@ static int git_xdiff(git_diff_output *output, git_patch *patch) git_patch__old_data(&info.xd_old_data.ptr, &info.xd_old_data.size, patch); git_patch__new_data(&info.xd_new_data.ptr, &info.xd_new_data.size, patch); + if (info.xd_old_data.size > GIT_XDIFF_MAX_SIZE || + info.xd_new_data.size > GIT_XDIFF_MAX_SIZE) { + giterr_set(GITERR_INVALID, "files too large for diff"); + return -1; + } + xdl_diff(&info.xd_old_data, &info.xd_new_data, &xo->params, &xo->config, &xo->callback); diff --git a/src/diff_xdiff.h b/src/diff_xdiff.h index c547b00cf05..98e11b2cbf6 100644 --- a/src/diff_xdiff.h +++ b/src/diff_xdiff.h @@ -11,6 +11,11 @@ #include "diff_patch.h" #include "xdiff/xdiff.h" +/* xdiff cannot cope with large files. these files should not be passed to + * xdiff. callers should treat these large files as binary. + */ +#define GIT_XDIFF_MAX_SIZE (1024LL * 1024 * 1023) + /* A git_xdiff_output is a git_diff_output with extra fields necessary * to use libxdiff. Calling git_xdiff_init() will set the diff_cb field * of the output to use xdiff to generate the diffs. diff --git a/src/merge.c b/src/merge.c index 89b8e85054d..930457bdb27 100644 --- a/src/merge.c +++ b/src/merge.c @@ -20,7 +20,6 @@ #include "diff.h" #include "checkout.h" #include "tree.h" -#include "merge_file.h" #include "blob.h" #include "oid.h" #include "index.h" diff --git a/src/merge_file.c b/src/merge_file.c index b174231cc2b..6d4738065f1 100644 --- a/src/merge_file.c +++ b/src/merge_file.c @@ -7,10 +7,10 @@ #include "common.h" #include "repository.h" -#include "merge_file.h" #include "posix.h" #include "fileops.h" #include "index.h" +#include "diff_xdiff.h" #include "git2/repository.h" #include "git2/object.h" @@ -199,7 +199,7 @@ static bool merge_file__is_binary(const git_merge_file_input *file) { size_t len = file ? file->size : 0; - if (len > GIT_MERGE_FILE_XDIFF_MAX) + if (len > GIT_XDIFF_MAX_SIZE) return true; if (len > GIT_MERGE_FILE_BINARY_SIZE) len = GIT_MERGE_FILE_BINARY_SIZE; diff --git a/src/merge_file.h b/src/merge_file.h deleted file mode 100644 index 08ecfc09542..00000000000 --- a/src/merge_file.h +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Copyright (C) the libgit2 contributors. All rights reserved. - * - * This file is part of libgit2, distributed under the GNU GPL v2 with - * a Linking Exception. For full terms see the included COPYING file. - */ -#ifndef INCLUDE_filediff_h__ -#define INCLUDE_filediff_h__ - -/* xdiff cannot cope with large files, just treat them as binary */ -#define GIT_MERGE_FILE_XDIFF_MAX (1024UL * 1024 * 1023) - -#endif diff --git a/tests/merge/files.c b/tests/merge/files.c index b365d5a426b..2d55df2b280 100644 --- a/tests/merge/files.c +++ b/tests/merge/files.c @@ -3,10 +3,10 @@ #include "git2/merge.h" #include "buffer.h" #include "merge.h" -#include "merge_file.h" #include "merge_helpers.h" #include "refs.h" #include "fileops.h" +#include "diff_xdiff.h" #define TEST_REPO_PATH "merge-resolve" #define TEST_INDEX_PATH TEST_REPO_PATH "/.git/index" @@ -296,11 +296,11 @@ void test_merge_files__skips_large_files(void) git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; git_merge_file_result result = {0}; - ours.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + ours.size = GIT_XDIFF_MAX_SIZE + 1; ours.path = "testfile.txt"; ours.mode = 0100755; - theirs.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + theirs.size = GIT_XDIFF_MAX_SIZE + 1; theirs.path = "testfile.txt"; theirs.mode = 0100755; From ae195a71ae898b7d5d61943f24f31d97065a87c6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 29 Sep 2015 12:46:41 -0400 Subject: [PATCH 07/23] blame: guard xdiff calls for large files --- src/blame.c | 2 +- src/blame_git.c | 34 +++++++++++++++++++++++++++------- src/blame_git.h | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/blame.c b/src/blame.c index a52f9402b68..08a90dcfd64 100644 --- a/src/blame.c +++ b/src/blame.c @@ -331,7 +331,7 @@ static int blame_internal(git_blame *blame) blame->ent = ent; - git_blame__like_git(blame, blame->options.flags); + error = git_blame__like_git(blame, blame->options.flags); cleanup: for (ent = blame->ent; ent; ) { diff --git a/src/blame_git.c b/src/blame_git.c index f481426aa85..67bae238445 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -9,6 +9,7 @@ #include "commit.h" #include "blob.h" #include "xdiff/xinclude.h" +#include "diff_xdiff.h" /* * Origin is refcounted and usually we keep the blob contents to be @@ -351,6 +352,13 @@ static int diff_hunks(mmfile_t file_a, mmfile_t file_b, void *cb_data) ecb.priv = cb_data; trim_common_tail(&file_a, &file_b, 0); + + if (file_a.size > GIT_XDIFF_MAX_SIZE || + file_b.size > GIT_XDIFF_MAX_SIZE) { + giterr_set(GITERR_INVALID, "file too large to blame"); + return -1; + } + return xdl_diff(&file_a, &file_b, &xpp, &xecfg, &ecb); } @@ -379,7 +387,9 @@ static int pass_blame_to_parent( fill_origin_blob(parent, &file_p); fill_origin_blob(target, &file_o); - diff_hunks(file_p, file_o, &d); + if (diff_hunks(file_p, file_o, &d) < 0) + return -1; + /* The reset (i.e. anything after tlno) are the same as the parent */ blame_chunk(blame, d.tlno, d.plno, last_in_target, target, parent); @@ -477,12 +487,13 @@ static void pass_whole_blame(git_blame *blame, } } -static void pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt) +static int pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt) { git_commit *commit = origin->commit; int i, num_parents; git_blame__origin *sg_buf[16]; git_blame__origin *porigin, **sg_origin = sg_buf; + int ret, error = 0; num_parents = git_commit_parentcount(commit); if (!git_oid_cmp(git_commit_id(commit), &blame->options.oldest_commit)) @@ -540,8 +551,13 @@ static void pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt origin_incref(porigin); origin->previous = porigin; } - if (pass_blame_to_parent(blame, origin, porigin)) + + if ((ret = pass_blame_to_parent(blame, origin, porigin)) != 0) { + if (ret < 0) + error = -1; + goto finish; + } } /* TODO: optionally find moves in parents' files */ @@ -554,7 +570,7 @@ static void pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt origin_decref(sg_origin[i]); if (sg_origin != sg_buf) git__free(sg_origin); - return; + return error; } /* @@ -583,7 +599,7 @@ static void coalesce(git_blame *blame) } } -void git_blame__like_git(git_blame *blame, uint32_t opt) +int git_blame__like_git(git_blame *blame, uint32_t opt) { while (true) { git_blame__entry *ent; @@ -594,11 +610,13 @@ void git_blame__like_git(git_blame *blame, uint32_t opt) if (!ent->guilty) suspect = ent->suspect; if (!suspect) - return; /* all done */ + return 0; /* all done */ /* We'll use this suspect later in the loop, so hold on to it for now. */ origin_incref(suspect); - pass_blame(blame, suspect, opt); + + if (pass_blame(blame, suspect, opt) < 0) + return -1; /* Take responsibility for the remaining entries */ for (ent = blame->ent; ent; ent = ent->next) { @@ -613,6 +631,8 @@ void git_blame__like_git(git_blame *blame, uint32_t opt) } coalesce(blame); + + return 0; } void git_blame__free_entry(git_blame__entry *ent) diff --git a/src/blame_git.h b/src/blame_git.h index 3ec2710b80c..1891b0e1f27 100644 --- a/src/blame_git.h +++ b/src/blame_git.h @@ -15,6 +15,6 @@ int git_blame__get_origin( git_commit *commit, const char *path); void git_blame__free_entry(git_blame__entry *ent); -void git_blame__like_git(git_blame *sb, uint32_t flags); +int git_blame__like_git(git_blame *sb, uint32_t flags); #endif From e3f94c71c3792716b89b3b56b7dbb984e1620e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 6 Oct 2015 13:35:45 +0200 Subject: [PATCH 08/23] CMake: be more explicit with python errors There's been a few reports of users not understanding what the python error means, so spell out the options they have. --- CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 713640d6a7d..abcc0e49df3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -575,7 +575,12 @@ INSTALL(FILES include/git2.h DESTINATION ${INCLUDE_INSTALL_DIR} ) # Tests IF (BUILD_CLAR) - FIND_PACKAGE(PythonInterp REQUIRED) + FIND_PACKAGE(PythonInterp) + + IF(NOT PYTHONINTERP_FOUND) + MESSAGE(FATAL_ERROR "Could not find a python interpeter, which is needed to build the tests. " + "Make sure python is available, or pass -DBUILD_CLAR=OFF to skip building the tests") + ENDIF() SET(CLAR_FIXTURES "${CMAKE_CURRENT_SOURCE_DIR}/tests/resources/") SET(CLAR_PATH "${CMAKE_CURRENT_SOURCE_DIR}/tests") From 8b8f1f91881a024d0d8efbf1f69e49ed65fdbc6f Mon Sep 17 00:00:00 2001 From: Eun Date: Wed, 7 Oct 2015 14:01:05 +0200 Subject: [PATCH 09/23] fix return --- examples/network/fetch.c | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/network/fetch.c b/examples/network/fetch.c index bc7a5a05ec0..177359b8802 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -63,6 +63,7 @@ static int transfer_progress_cb(const git_transfer_progress *stats, void *payloa stats->received_objects, stats->total_objects, stats->indexed_objects, stats->received_bytes); } + return 0; } /** Entry point for this command */ From 5ffdea6f65bb105c17a1c4730d51732e3391bf63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 14 Oct 2015 16:49:01 +0200 Subject: [PATCH 10/23] revwalk: make commit list use 64 bits for time We moved the "main" parsing to use 64 bits for the timestamp, but the quick parsing for the revwalk did not. This means that for large timestamps we fail to parse the time and thus the walk. Move this parser to use 64 bits as well. --- src/commit_list.c | 6 +++--- src/commit_list.h | 2 +- tests/revwalk/basic.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/commit_list.c b/src/commit_list.c index 3054c18dd5e..53612d51494 100644 --- a/src/commit_list.c +++ b/src/commit_list.c @@ -110,7 +110,7 @@ static int commit_quick_parse( const uint8_t *buffer_end = buffer + buffer_len; const uint8_t *parents_start, *committer_start; int i, parents = 0; - int commit_time; + int64_t commit_time; buffer += strlen("tree ") + GIT_OID_HEXSZ + 1; @@ -166,10 +166,10 @@ static int commit_quick_parse( buffer--; } - if ((buffer == committer_start) || (git__strtol32(&commit_time, (char *)(buffer + 1), NULL, 10) < 0)) + if ((buffer == committer_start) || (git__strtol64(&commit_time, (char *)(buffer + 1), NULL, 10) < 0)) return commit_error(commit, "cannot parse commit time"); - commit->time = (time_t)commit_time; + commit->time = commit_time; commit->parsed = 1; return 0; } diff --git a/src/commit_list.h b/src/commit_list.h index 6b3f473d3ce..b1d88e01651 100644 --- a/src/commit_list.h +++ b/src/commit_list.h @@ -22,7 +22,7 @@ typedef struct git_commit_list_node { git_oid oid; - uint32_t time; + int64_t time; unsigned int seen:1, uninteresting:1, topo_delay:1, diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index 7e50452c9a9..d8236ce72f1 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -437,3 +437,38 @@ void test_revwalk_basic__mimic_git_rev_list(void) cl_git_fail_with(git_revwalk_next(&oid, _walk), GIT_ITEROVER); } + +void test_revwalk_basic__big_timestamp(void) +{ + git_reference *head; + git_commit *tip; + git_signature *sig; + git_tree *tree; + git_oid id; + int error; + + revwalk_basic_setup_walk("testrepo.git"); + + cl_git_pass(git_repository_head(&head, _repo)); + cl_git_pass(git_reference_peel((git_object **) &tip, head, GIT_OBJ_COMMIT)); + + /* Commit with a far-ahead timestamp, we should be able to parse it in the revwalk */ + cl_git_pass(git_signature_new(&sig, "Joe", "joe@example.com", 2399662595, 0)); + cl_git_pass(git_commit_tree(&tree, tip)); + + cl_git_pass(git_commit_create(&id, _repo, "HEAD", sig, sig, NULL, "some message", tree, 1, &tip)); + + cl_git_pass(git_revwalk_push_head(_walk)); + + while ((error = git_revwalk_next(&id, _walk)) == 0) { + /* nothing */ + } + + cl_assert_equal_i(GIT_ITEROVER, error); + + git_tree_free(tree); + git_commit_free(tip); + git_reference_free(head); + git_signature_free(sig); + +} From 43820f204ea32503b4083e3b6b83f30a0a0031c9 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 14 Oct 2015 19:24:07 +0200 Subject: [PATCH 11/23] odb: Be smarter when refreshing backends In the current implementation of ODB backends, each backend is tasked with refreshing itself after a failed lookup. This is standard Git behavior: we want to e.g. reload the packfiles on disk in case they have changed and that's the reason we can't find the object we're looking for. This behavior, however, becomes pathological in repositories where multiple alternates have been loaded. Given that each alternate counts as a separate backend, a miss in the main repository (which can potentially be very frequent in cases where object storage comes from the alternate) will result in refreshing all its packfiles before we move on to the alternate backend where the object will most likely be found. To fix this, the code in `odb.c` has been refactored as to perform the refresh of all the backends externally, once we've verified that the object is nowhere to be found. If the refresh is successful, we then perform the lookup sequentially through all the backends, skipping the ones that we know for sure weren't refreshed (because they have no refresh API). The on-disk pack backend has been adjusted accordingly: it no longer performs refreshes internally. --- src/odb.c | 231 +++++++++++++++++++++++++++++++++---------------- src/odb_pack.c | 82 +----------------- 2 files changed, 159 insertions(+), 154 deletions(-) diff --git a/src/odb.c b/src/odb.c index 2b2c35fe88e..af1ff7fa254 100644 --- a/src/odb.c +++ b/src/odb.c @@ -620,23 +620,18 @@ void git_odb_free(git_odb *db) GIT_REFCOUNT_DEC(db, odb_free); } -int git_odb_exists(git_odb *db, const git_oid *id) +static int odb_exists_1(git_odb *db, const git_oid *id, bool only_refreshed) { - git_odb_object *object; size_t i; bool found = false; - assert(db && id); - - if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) { - git_odb_object_free(object); - return (int)true; - } - for (i = 0; i < db->backends.length && !found; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; + if (only_refreshed && !b->refresh) + continue; + if (b->exists != NULL) found = (bool)b->exists(b, id); } @@ -644,43 +639,45 @@ int git_odb_exists(git_odb *db, const git_oid *id) return (int)found; } -int git_odb_exists_prefix( - git_oid *out, git_odb *db, const git_oid *short_id, size_t len) +int git_odb_exists(git_odb *db, const git_oid *id) { - int error = GIT_ENOTFOUND, num_found = 0; - size_t i; - git_oid key = {{0}}, last_found = {{0}}, found; - - assert(db && short_id); + git_odb_object *object; - if (len < GIT_OID_MINPREFIXLEN) - return git_odb__error_ambiguous("prefix length too short"); - if (len > GIT_OID_HEXSZ) - len = GIT_OID_HEXSZ; + assert(db && id); - if (len == GIT_OID_HEXSZ) { - if (git_odb_exists(db, short_id)) { - if (out) - git_oid_cpy(out, short_id); - return 0; - } else { - return git_odb__error_notfound("no match for id prefix", short_id); - } + if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) { + git_odb_object_free(object); + return (int)true; } - /* just copy valid part of short_id */ - memcpy(&key.id, short_id->id, (len + 1) / 2); - if (len & 1) - key.id[len / 2] &= 0xF0; + if (odb_exists_1(db, id, false)) + return 1; + + if (!git_odb_refresh(db)) + return odb_exists_1(db, id, true); + + /* Failed to refresh, hence not found */ + return 0; +} + +static int odb_exists_prefix_1(git_oid *out, git_odb *db, + const git_oid *key, size_t len, bool only_refreshed) +{ + size_t i; + int error = GIT_ENOTFOUND, num_found = 0; + git_oid last_found = {{0}}, found; for (i = 0; i < db->backends.length; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; + if (only_refreshed && !b->refresh) + continue; + if (!b->exists_prefix) continue; - error = b->exists_prefix(&found, b, &key, len); + error = b->exists_prefix(&found, b, key, len); if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH) continue; if (error) @@ -697,13 +694,53 @@ int git_odb_exists_prefix( } if (!num_found) - return git_odb__error_notfound("no match for id prefix", &key); + return GIT_ENOTFOUND; + if (out) git_oid_cpy(out, &last_found); return 0; } +int git_odb_exists_prefix( + git_oid *out, git_odb *db, const git_oid *short_id, size_t len) +{ + int error; + git_oid key = {{0}}; + + assert(db && short_id); + + if (len < GIT_OID_MINPREFIXLEN) + return git_odb__error_ambiguous("prefix length too short"); + if (len > GIT_OID_HEXSZ) + len = GIT_OID_HEXSZ; + + if (len == GIT_OID_HEXSZ) { + if (git_odb_exists(db, short_id)) { + if (out) + git_oid_cpy(out, short_id); + return 0; + } else { + return git_odb__error_notfound("no match for id prefix", short_id); + } + } + + /* just copy valid part of short_id */ + memcpy(&key.id, short_id->id, (len + 1) / 2); + if (len & 1) + key.id[len / 2] &= 0xF0; + + error = odb_exists_prefix_1(out, db, &key, len, false); + + if (error == GIT_ENOTFOUND && !git_odb_refresh(db)) + error = odb_exists_prefix_1(out, db, &key, len, true); + + if (error == GIT_ENOTFOUND) + return git_odb__error_notfound("no match for id prefix", &key); + + return error; +} + int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id) { int error; @@ -783,36 +820,38 @@ static int hardcoded_objects(git_rawobj *raw, const git_oid *id) } } -int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) +static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id, + bool only_refreshed) { - size_t i, reads = 0; - int error; + size_t i; git_rawobj raw; git_odb_object *object; + bool found = false; - assert(out && db && id); - - *out = git_cache_get_raw(odb_cache(db), id); - if (*out != NULL) - return 0; - - error = hardcoded_objects(&raw, id); + if (!hardcoded_objects(&raw, id)) + found = true; - for (i = 0; i < db->backends.length && error < 0; ++i) { + for (i = 0; i < db->backends.length && !found; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; + if (only_refreshed && !b->refresh) + continue; + if (b->read != NULL) { - ++reads; - error = b->read(&raw.data, &raw.len, &raw.type, b, id); + int error = b->read(&raw.data, &raw.len, &raw.type, b, id); + if (error == GIT_PASSTHROUGH || error == GIT_ENOTFOUND) + continue; + + if (error < 0) + return error; + + found = true; } } - if (error && error != GIT_PASSTHROUGH) { - if (!reads) - return git_odb__error_notfound("no match for id", id); - return error; - } + if (!found) + return GIT_ENOTFOUND; giterr_clear(); if ((object = odb_object__alloc(id, &raw)) == NULL) @@ -822,42 +861,48 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) return 0; } -int git_odb_read_prefix( - git_odb_object **out, git_odb *db, const git_oid *short_id, size_t len) +int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) +{ + int error; + + assert(out && db && id); + + *out = git_cache_get_raw(odb_cache(db), id); + if (*out != NULL) + return 0; + + error = odb_read_1(out, db, id, false); + + if (error == GIT_ENOTFOUND && !git_odb_refresh(db)) + error = odb_read_1(out, db, id, true); + + if (error == GIT_ENOTFOUND) + return git_odb__error_notfound("no match for id", id); + + return error; +} + +static int read_prefix_1(git_odb_object **out, git_odb *db, + const git_oid *key, size_t len, bool only_refreshed) { size_t i; int error = GIT_ENOTFOUND; - git_oid key = {{0}}, found_full_oid = {{0}}; + git_oid found_full_oid = {{0}}; git_rawobj raw; void *data = NULL; bool found = false; git_odb_object *object; - assert(out && db); - - if (len < GIT_OID_MINPREFIXLEN) - return git_odb__error_ambiguous("prefix length too short"); - if (len > GIT_OID_HEXSZ) - len = GIT_OID_HEXSZ; - - if (len == GIT_OID_HEXSZ) { - *out = git_cache_get_raw(odb_cache(db), short_id); - if (*out != NULL) - return 0; - } - - /* just copy valid part of short_id */ - memcpy(&key.id, short_id->id, (len + 1) / 2); - if (len & 1) - key.id[len / 2] &= 0xF0; - for (i = 0; i < db->backends.length; ++i) { backend_internal *internal = git_vector_get(&db->backends, i); git_odb_backend *b = internal->backend; + if (only_refreshed && !b->refresh) + continue; + if (b->read_prefix != NULL) { git_oid full_oid; - error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, &key, len); + error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, key, len); if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH) continue; @@ -878,7 +923,7 @@ int git_odb_read_prefix( } if (!found) - return git_odb__error_notfound("no match for prefix", &key); + return GIT_ENOTFOUND; if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL) return -1; @@ -887,6 +932,42 @@ int git_odb_read_prefix( return 0; } +int git_odb_read_prefix( + git_odb_object **out, git_odb *db, const git_oid *short_id, size_t len) +{ + git_oid key = {{0}}; + int error; + + assert(out && db); + + if (len < GIT_OID_MINPREFIXLEN) + return git_odb__error_ambiguous("prefix length too short"); + + if (len > GIT_OID_HEXSZ) + len = GIT_OID_HEXSZ; + + if (len == GIT_OID_HEXSZ) { + *out = git_cache_get_raw(odb_cache(db), short_id); + if (*out != NULL) + return 0; + } + + /* just copy valid part of short_id */ + memcpy(&key.id, short_id->id, (len + 1) / 2); + if (len & 1) + key.id[len / 2] &= 0xF0; + + error = read_prefix_1(out, db, &key, len, false); + + if (error == GIT_ENOTFOUND && !git_odb_refresh(db)) + error = read_prefix_1(out, db, &key, len, true); + + if (error == GIT_ENOTFOUND) + return git_odb__error_notfound("no match for prefix", &key); + + return error; +} + int git_odb_foreach(git_odb *db, git_odb_foreach_cb cb, void *payload) { unsigned int i; diff --git a/src/odb_pack.c b/src/odb_pack.c index 735158d9604..77d2c75b969 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -346,7 +346,7 @@ static int pack_backend__refresh(git_odb_backend *backend_) return error; } -static int pack_backend__read_header_internal( +static int pack_backend__read_header( size_t *len_p, git_otype *type_p, struct git_odb_backend *backend, const git_oid *oid) { @@ -361,24 +361,7 @@ static int pack_backend__read_header_internal( return git_packfile_resolve_header(len_p, type_p, e.p, e.offset); } -static int pack_backend__read_header( - size_t *len_p, git_otype *type_p, - struct git_odb_backend *backend, const git_oid *oid) -{ - int error; - - error = pack_backend__read_header_internal(len_p, type_p, backend, oid); - - if (error != GIT_ENOTFOUND) - return error; - - if ((error = pack_backend__refresh(backend)) < 0) - return error; - - return pack_backend__read_header_internal(len_p, type_p, backend, oid); -} - -static int pack_backend__read_internal( +static int pack_backend__read( void **buffer_p, size_t *len_p, git_otype *type_p, git_odb_backend *backend, const git_oid *oid) { @@ -397,24 +380,7 @@ static int pack_backend__read_internal( return 0; } -static int pack_backend__read( - void **buffer_p, size_t *len_p, git_otype *type_p, - git_odb_backend *backend, const git_oid *oid) -{ - int error; - - error = pack_backend__read_internal(buffer_p, len_p, type_p, backend, oid); - - if (error != GIT_ENOTFOUND) - return error; - - if ((error = pack_backend__refresh(backend)) < 0) - return error; - - return pack_backend__read_internal(buffer_p, len_p, type_p, backend, oid); -} - -static int pack_backend__read_prefix_internal( +static int pack_backend__read_prefix( git_oid *out_oid, void **buffer_p, size_t *len_p, @@ -451,45 +417,9 @@ static int pack_backend__read_prefix_internal( return error; } -static int pack_backend__read_prefix( - git_oid *out_oid, - void **buffer_p, - size_t *len_p, - git_otype *type_p, - git_odb_backend *backend, - const git_oid *short_oid, - size_t len) -{ - int error; - - error = pack_backend__read_prefix_internal( - out_oid, buffer_p, len_p, type_p, backend, short_oid, len); - - if (error != GIT_ENOTFOUND) - return error; - - if ((error = pack_backend__refresh(backend)) < 0) - return error; - - return pack_backend__read_prefix_internal( - out_oid, buffer_p, len_p, type_p, backend, short_oid, len); -} - static int pack_backend__exists(git_odb_backend *backend, const git_oid *oid) { struct git_pack_entry e; - int error; - - error = pack_entry_find(&e, (struct pack_backend *)backend, oid); - - if (error != GIT_ENOTFOUND) - return error == 0; - - if ((error = pack_backend__refresh(backend)) < 0) { - giterr_clear(); - return (int)false; - } - return pack_entry_find(&e, (struct pack_backend *)backend, oid) == 0; } @@ -501,12 +431,7 @@ static int pack_backend__exists_prefix( struct git_pack_entry e = {0}; error = pack_entry_find_prefix(&e, pb, short_id, len); - - if (error == GIT_ENOTFOUND && !(error = pack_backend__refresh(backend))) - error = pack_entry_find_prefix(&e, pb, short_id, len); - git_oid_cpy(out, &e.sha1); - return error; } @@ -674,7 +599,6 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) git_path_isdir(git_buf_cstr(&path))) { backend->pack_folder = git_buf_detach(&path); - error = pack_backend__refresh((git_odb_backend *)backend); } From a0a1b19ab043f3579aabfb7602b4c4ac4dd69e72 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 14 Oct 2015 19:31:54 +0200 Subject: [PATCH 12/23] odb: Prioritize alternate backends For most real use cases, repositories with alternates use them as main object storage. Checking the alternate for objects before the main repository should result in measurable speedups. Because of this, we're changing the sorting algorithm to prioritize alternates *in cases where two backends have the same priority*. This means that the pack backend for the alternate will be checked before the pack backend for the main repository *but* both of them will be checked before any loose backends. --- src/odb.c | 12 ++++++++---- tests/odb/sorting.c | 16 ++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/odb.c b/src/odb.c index af1ff7fa254..1c877c9fc30 100644 --- a/src/odb.c +++ b/src/odb.c @@ -374,10 +374,14 @@ static int backend_sort_cmp(const void *a, const void *b) const backend_internal *backend_a = (const backend_internal *)(a); const backend_internal *backend_b = (const backend_internal *)(b); - if (backend_a->is_alternate == backend_b->is_alternate) - return (backend_b->priority - backend_a->priority); - - return backend_a->is_alternate ? 1 : -1; + if (backend_b->priority == backend_a->priority) { + if (backend_a->is_alternate) + return -1; + if (backend_b->is_alternate) + return 1; + return 0; + } + return (backend_b->priority - backend_a->priority); } int git_odb_new(git_odb **out) diff --git a/tests/odb/sorting.c b/tests/odb/sorting.c index d24c49c6937..6af8b0d1b86 100644 --- a/tests/odb/sorting.c +++ b/tests/odb/sorting.c @@ -57,14 +57,14 @@ void test_odb_sorting__basic_backends_sorting(void) void test_odb_sorting__alternate_backends_sorting(void) { - cl_git_pass(git_odb_add_backend(_odb, new_backend(0), 5)); - cl_git_pass(git_odb_add_backend(_odb, new_backend(2), 3)); - cl_git_pass(git_odb_add_backend(_odb, new_backend(1), 4)); - cl_git_pass(git_odb_add_backend(_odb, new_backend(3), 1)); - cl_git_pass(git_odb_add_alternate(_odb, new_backend(4), 5)); - cl_git_pass(git_odb_add_alternate(_odb, new_backend(6), 3)); - cl_git_pass(git_odb_add_alternate(_odb, new_backend(5), 4)); - cl_git_pass(git_odb_add_alternate(_odb, new_backend(7), 1)); + cl_git_pass(git_odb_add_backend(_odb, new_backend(1), 5)); + cl_git_pass(git_odb_add_backend(_odb, new_backend(5), 3)); + cl_git_pass(git_odb_add_backend(_odb, new_backend(3), 4)); + cl_git_pass(git_odb_add_backend(_odb, new_backend(7), 1)); + cl_git_pass(git_odb_add_alternate(_odb, new_backend(0), 5)); + cl_git_pass(git_odb_add_alternate(_odb, new_backend(4), 3)); + cl_git_pass(git_odb_add_alternate(_odb, new_backend(2), 4)); + cl_git_pass(git_odb_add_alternate(_odb, new_backend(6), 1)); check_backend_sorting(_odb); } From 307c4a2b6d1e3b403225b3d5b7f9894b940b24bd Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 21 Oct 2015 11:58:44 +0200 Subject: [PATCH 13/23] signature: Strip crud just like Git does --- src/signature.c | 18 ++++++++++++++++-- tests/commit/signature.c | 7 +++++++ tests/revwalk/signatureparsing.c | 2 +- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/signature.c b/src/signature.c index 818cd300eaf..109476efe1a 100644 --- a/src/signature.c +++ b/src/signature.c @@ -34,13 +34,27 @@ static bool contains_angle_brackets(const char *input) return strchr(input, '<') != NULL || strchr(input, '>') != NULL; } +static bool is_crud(unsigned char c) +{ + return c <= 32 || + c == '.' || + c == ',' || + c == ':' || + c == ';' || + c == '<' || + c == '>' || + c == '"' || + c == '\\' || + c == '\''; +} + static char *extract_trimmed(const char *ptr, size_t len) { - while (len && git__isspace(ptr[0])) { + while (len && is_crud((unsigned char)ptr[0])) { ptr++; len--; } - while (len && git__isspace(ptr[len - 1])) { + while (len && is_crud((unsigned char)ptr[len - 1])) { len--; } diff --git a/tests/commit/signature.c b/tests/commit/signature.c index 41a74b999d4..0070320ae37 100644 --- a/tests/commit/signature.c +++ b/tests/commit/signature.c @@ -35,6 +35,13 @@ void test_commit_signature__leading_and_trailing_spaces_are_trimmed(void) assert_name_and_email("nulltoken", "emeric.fermas@gmail.com", " \t nulltoken \n", " \n emeric.fermas@gmail.com \n"); } +void test_commit_signature__leading_and_trailing_crud_is_trimmed(void) +{ + assert_name_and_email("nulltoken", "emeric.fermas@gmail.com", "\"nulltoken\"", "\"emeric.fermas@gmail.com\""); + assert_name_and_email("nulltoken w", "emeric.fermas@gmail.com", "nulltoken w.", "emeric.fermas@gmail.com"); + assert_name_and_email("nulltoken \xe2\x98\xba", "emeric.fermas@gmail.com", "nulltoken \xe2\x98\xba", "emeric.fermas@gmail.com"); +} + void test_commit_signature__angle_brackets_in_names_are_not_supported(void) { cl_git_fail(try_build_signature("email); - cl_assert_equal_s("", signature->name); + cl_assert_equal_s("Yu V. Bin Haacked", signature->name); cl_assert_equal_i(1323847743, (int)signature->when.time); cl_assert_equal_i(60, signature->when.offset); From 128e94bbbb1f1af539be7fb2844e261bfdb28fed Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 21 Oct 2015 12:04:53 +0200 Subject: [PATCH 14/23] index: Remove unneeded consts --- src/index.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.c b/src/index.c index c0be5b90d12..334a1313503 100644 --- a/src/index.c +++ b/src/index.c @@ -1193,13 +1193,13 @@ static int index_no_dups(void **old, void *new) } static void index_existing_and_best( - const git_index_entry **existing, + git_index_entry **existing, size_t *existing_position, - const git_index_entry **best, + git_index_entry **best, git_index *index, const git_index_entry *entry) { - const git_index_entry *e; + git_index_entry *e; size_t pos; int error; From bbe1957b8c75760c81ce04c7edf6d203513b39f8 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Wed, 21 Oct 2015 12:09:29 +0200 Subject: [PATCH 15/23] tests: Fix warnings --- tests/diff/workdir.c | 2 +- tests/revwalk/basic.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/diff/workdir.c b/tests/diff/workdir.c index 89f9483a675..dac32453b8a 100644 --- a/tests/diff/workdir.c +++ b/tests/diff/workdir.c @@ -2080,7 +2080,7 @@ void test_diff_workdir__to_index_pathlist(void) cl_git_pass(git_repository_index(&index, g_repo)); opts.flags = GIT_DIFF_INCLUDE_IGNORED; - opts.pathspec.strings = pathlist.contents; + opts.pathspec.strings = (char **)pathlist.contents; opts.pathspec.count = pathlist.length; cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, index, &opts)); diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index d8236ce72f1..5ed7da4eba9 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -456,7 +456,8 @@ void test_revwalk_basic__big_timestamp(void) cl_git_pass(git_signature_new(&sig, "Joe", "joe@example.com", 2399662595, 0)); cl_git_pass(git_commit_tree(&tree, tip)); - cl_git_pass(git_commit_create(&id, _repo, "HEAD", sig, sig, NULL, "some message", tree, 1, &tip)); + cl_git_pass(git_commit_create(&id, _repo, "HEAD", sig, sig, NULL, "some message", tree, 1, + (const git_commit **)&tip)); cl_git_pass(git_revwalk_push_head(_walk)); From 8c7c5fa585c6a63dc8186febd6e032880655e85e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 20 Oct 2015 17:42:42 +0200 Subject: [PATCH 16/23] config: add a ProgramData level This is where portable git stores the global configuration which we can use to adhere to it even though git isn't quite installed on the system. --- include/git2/config.h | 24 +++++++++++++++++++----- src/config.c | 10 ++++++++++ src/config.h | 1 + src/repository.c | 14 ++++++++++++-- src/sysdir.c | 19 ++++++++++++++++++- src/sysdir.h | 14 ++++++++++++-- src/win32/findfile.c | 10 ++++++++++ src/win32/findfile.h | 1 + tests/config/global.c | 40 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 123 insertions(+), 10 deletions(-) diff --git a/include/git2/config.h b/include/git2/config.h index 56b5431ac3b..d0f1ba1b30f 100644 --- a/include/git2/config.h +++ b/include/git2/config.h @@ -29,25 +29,28 @@ GIT_BEGIN_DECL * priority levels as well. */ typedef enum { + /** System-wide on Windows, for compatibility with portable git */ + GIT_CONFIG_LEVEL_PROGRAMDATA = 1, + /** System-wide configuration file; /etc/gitconfig on Linux systems */ - GIT_CONFIG_LEVEL_SYSTEM = 1, + GIT_CONFIG_LEVEL_SYSTEM = 2, /** XDG compatible configuration file; typically ~/.config/git/config */ - GIT_CONFIG_LEVEL_XDG = 2, + GIT_CONFIG_LEVEL_XDG = 3, /** User-specific configuration file (also called Global configuration * file); typically ~/.gitconfig */ - GIT_CONFIG_LEVEL_GLOBAL = 3, + GIT_CONFIG_LEVEL_GLOBAL = 4, /** Repository specific configuration file; $WORK_DIR/.git/config on * non-bare repos */ - GIT_CONFIG_LEVEL_LOCAL = 4, + GIT_CONFIG_LEVEL_LOCAL = 5, /** Application specific configuration file; freely defined by applications */ - GIT_CONFIG_LEVEL_APP = 5, + GIT_CONFIG_LEVEL_APP = 6, /** Represents the highest level available config file (i.e. the most * specific config file available that actually is loaded) @@ -141,6 +144,17 @@ GIT_EXTERN(int) git_config_find_xdg(git_buf *out); */ GIT_EXTERN(int) git_config_find_system(git_buf *out); +/** + * Locate the path to the configuration file in ProgramData + * + * Look for the file in %PROGRAMDATA%\Git\config used by portable git. + * + * @param out Pointer to a user-allocated git_buf in which to store the path + * @return 0 if a ProgramData configuration file has been + * found. Its path will be stored in `out`. + */ +GIT_EXTERN(int) git_config_find_programdata(git_buf *out); + /** * Open the global, XDG and system configuration files * diff --git a/src/config.c b/src/config.c index f0b2c3a61b0..f4d4cb2b9b1 100644 --- a/src/config.c +++ b/src/config.c @@ -1086,6 +1086,12 @@ int git_config_find_system(git_buf *path) return git_sysdir_find_system_file(path, GIT_CONFIG_FILENAME_SYSTEM); } +int git_config_find_programdata(git_buf *path) +{ + git_buf_sanitize(path); + return git_sysdir_find_programdata_file(path, GIT_CONFIG_FILENAME_PROGRAMDATA); +} + int git_config__global_location(git_buf *buf) { const git_buf *paths; @@ -1133,6 +1139,10 @@ int git_config_open_default(git_config **out) error = git_config_add_file_ondisk(cfg, buf.ptr, GIT_CONFIG_LEVEL_SYSTEM, 0); + if (!error && !git_config_find_programdata(&buf)) + error = git_config_add_file_ondisk(cfg, buf.ptr, + GIT_CONFIG_LEVEL_PROGRAMDATA, 0); + git_buf_free(&buf); if (error) { diff --git a/src/config.h b/src/config.h index ba745331a02..00c12b50d6a 100644 --- a/src/config.h +++ b/src/config.h @@ -12,6 +12,7 @@ #include "vector.h" #include "repository.h" +#define GIT_CONFIG_FILENAME_PROGRAMDATA "config" #define GIT_CONFIG_FILENAME_SYSTEM "gitconfig" #define GIT_CONFIG_FILENAME_GLOBAL ".gitconfig" #define GIT_CONFIG_FILENAME_XDG "config" diff --git a/src/repository.c b/src/repository.c index 77145cfc8e5..38d18693a18 100644 --- a/src/repository.c +++ b/src/repository.c @@ -585,7 +585,8 @@ static int load_config( git_repository *repo, const char *global_config_path, const char *xdg_config_path, - const char *system_config_path) + const char *system_config_path, + const char *programdata_path) { int error; git_buf config_path = GIT_BUF_INIT; @@ -626,6 +627,12 @@ static int load_config( error != GIT_ENOTFOUND) goto on_error; + if (programdata_path != NULL && + (error = git_config_add_file_ondisk( + cfg, programdata_path, GIT_CONFIG_LEVEL_PROGRAMDATA, 0)) < 0 && + error != GIT_ENOTFOUND) + goto on_error; + giterr_clear(); /* clear any lingering ENOTFOUND errors */ *out = cfg; @@ -651,11 +658,13 @@ int git_repository_config__weakptr(git_config **out, git_repository *repo) git_buf global_buf = GIT_BUF_INIT; git_buf xdg_buf = GIT_BUF_INIT; git_buf system_buf = GIT_BUF_INIT; + git_buf programdata_buf = GIT_BUF_INIT; git_config *config; git_config_find_global(&global_buf); git_config_find_xdg(&xdg_buf); git_config_find_system(&system_buf); + git_config_find_programdata(&programdata_buf); /* If there is no global file, open a backend for it anyway */ if (git_buf_len(&global_buf) == 0) @@ -665,7 +674,8 @@ int git_repository_config__weakptr(git_config **out, git_repository *repo) &config, repo, path_unless_empty(&global_buf), path_unless_empty(&xdg_buf), - path_unless_empty(&system_buf)); + path_unless_empty(&system_buf), + path_unless_empty(&programdata_buf)); if (!error) { GIT_REFCOUNT_OWN(config, repo); diff --git a/src/sysdir.c b/src/sysdir.c index 2795de4918f..bf53d830f92 100644 --- a/src/sysdir.c +++ b/src/sysdir.c @@ -15,6 +15,16 @@ #include "win32/findfile.h" #endif +static int git_sysdir_guess_programdata_dirs(git_buf *out) +{ +#ifdef GIT_WIN32 + return git_win32__find_programdata_dirs(out); +#else + git_buf_clear(out); + return 0; +#endif +} + static int git_sysdir_guess_system_dirs(git_buf *out) { #ifdef GIT_WIN32 @@ -76,12 +86,13 @@ static int git_sysdir_guess_template_dirs(git_buf *out) typedef int (*git_sysdir_guess_cb)(git_buf *out); static git_buf git_sysdir__dirs[GIT_SYSDIR__MAX] = - { GIT_BUF_INIT, GIT_BUF_INIT, GIT_BUF_INIT, GIT_BUF_INIT }; + { GIT_BUF_INIT, GIT_BUF_INIT, GIT_BUF_INIT, GIT_BUF_INIT, GIT_BUF_INIT }; static git_sysdir_guess_cb git_sysdir__dir_guess[GIT_SYSDIR__MAX] = { git_sysdir_guess_system_dirs, git_sysdir_guess_global_dirs, git_sysdir_guess_xdg_dirs, + git_sysdir_guess_programdata_dirs, git_sysdir_guess_template_dirs, }; @@ -258,6 +269,12 @@ int git_sysdir_find_xdg_file(git_buf *path, const char *filename) path, filename, GIT_SYSDIR_XDG, "global/xdg"); } +int git_sysdir_find_programdata_file(git_buf *path, const char *filename) +{ + return git_sysdir_find_in_dirlist( + path, filename, GIT_SYSDIR_PROGRAMDATA, "ProgramData"); +} + int git_sysdir_find_template_dir(git_buf *path) { return git_sysdir_find_in_dirlist( diff --git a/src/sysdir.h b/src/sysdir.h index f1bbf0baea0..12874fc8538 100644 --- a/src/sysdir.h +++ b/src/sysdir.h @@ -38,6 +38,15 @@ extern int git_sysdir_find_xdg_file(git_buf *path, const char *filename); */ extern int git_sysdir_find_system_file(git_buf *path, const char *filename); +/** + * Find a "ProgramData" file (i.e. one in %PROGRAMDATA%) + * + * @param path buffer to write the full path into + * @param filename name of file to find in the ProgramData directory + * @return 0 if found, GIT_ENOTFOUND if not found, or -1 on other OS error + */ +extern int git_sysdir_find_programdata_file(git_buf *path, const char *filename); + /** * Find template directory. * @@ -50,8 +59,9 @@ typedef enum { GIT_SYSDIR_SYSTEM = 0, GIT_SYSDIR_GLOBAL = 1, GIT_SYSDIR_XDG = 2, - GIT_SYSDIR_TEMPLATE = 3, - GIT_SYSDIR__MAX = 4, + GIT_SYSDIR_PROGRAMDATA = 3, + GIT_SYSDIR_TEMPLATE = 4, + GIT_SYSDIR__MAX = 5, } git_sysdir_t; /** diff --git a/src/win32/findfile.c b/src/win32/findfile.c index de27dd06062..58c22279e57 100644 --- a/src/win32/findfile.c +++ b/src/win32/findfile.c @@ -215,3 +215,13 @@ int git_win32__find_xdg_dirs(git_buf *out) return win32_find_existing_dirs(out, global_tmpls); } + +int git_win32__find_programdata_dirs(git_buf *out) +{ + static const wchar_t *programdata_tmpls[2] = { + L"%PROGRAMDATA%\\Git", + NULL, + }; + + return win32_find_existing_dirs(out, programdata_tmpls); +} diff --git a/src/win32/findfile.h b/src/win32/findfile.h index a50319b9a3a..3d5fff439c2 100644 --- a/src/win32/findfile.h +++ b/src/win32/findfile.h @@ -11,6 +11,7 @@ extern int git_win32__find_system_dirs(git_buf *out, const wchar_t *subpath); extern int git_win32__find_global_dirs(git_buf *out); extern int git_win32__find_xdg_dirs(git_buf *out); +extern int git_win32__find_programdata_dirs(git_buf *out); #endif diff --git a/tests/config/global.c b/tests/config/global.c index b5e83fec04f..1336ef6e757 100644 --- a/tests/config/global.c +++ b/tests/config/global.c @@ -65,3 +65,43 @@ void test_config_global__open_xdg(void) git_config_free(xdg); git_config_free(cfg); } + +void test_config_global__open_programdata(void) +{ + char *programdata; + git_config *cfg; + git_repository *repo; + git_buf config_path = GIT_BUF_INIT; + git_buf var_contents = GIT_BUF_INIT; + + if (!cl_getenv("GITTEST_INVASIVE_FS_STRUCTURE")) + cl_skip(); + + programdata = cl_getenv("PROGRAMDATA"); + cl_git_pass(git_buf_printf(&config_path, "%s/Git", programdata)); + cl_git_pass(p_mkdir(config_path.ptr, 0777)); + cl_git_pass(git_buf_puts(&config_path, "/config")); + + cl_git_pass(git_config_open_ondisk(&cfg, config_path.ptr)); + cl_git_pass(git_config_set_string(cfg, "programdata.var", "even higher level")); + + git_buf_free(&config_path); + git_config_free(cfg); + + git_config_open_default(&cfg); + cl_git_pass(git_config_get_string_buf(&var_contents, cfg, "programdata.var")); + cl_assert_equal_s("even higher level", var_contents.ptr); + + git_config_free(cfg); + git_buf_free(&var_contents); + + cl_git_pass(git_repository_init(&repo, "./foo.git", true)); + cl_git_pass(git_repository_config(&cfg, repo)); + cl_git_pass(git_config_get_string_buf(&var_contents, cfg, "programdata.var")); + cl_assert_equal_s("even higher level", var_contents.ptr); + + git_config_free(cfg); + git_buf_free(&var_contents); + git_repository_free(repo); + cl_fixture_cleanup("./foo.git"); +} From 0f9b6742ad93111e95c6a0f14f2c2c5834c685eb Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 21 Oct 2015 09:24:10 -0400 Subject: [PATCH 17/23] win32: add c linkage guard around inttypes.h inclusion --- include/git2/common.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/git2/common.h b/include/git2/common.h index d84a765123d..748226385d6 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -10,12 +10,6 @@ #include #include -#ifdef _MSC_VER -# include "inttypes.h" -#else -# include -#endif - #ifdef __cplusplus # define GIT_BEGIN_DECL extern "C" { # define GIT_END_DECL } @@ -26,6 +20,14 @@ # define GIT_END_DECL /* empty */ #endif +#ifdef _MSC_VER + GIT_BEGIN_DECL +# include "inttypes.h" + GIT_END_DECL +#else +# include +#endif + /** Declare a public function exported for application use. */ #if __GNUC__ >= 4 # define GIT_EXTERN(type) extern \ From 240a85cf10b0ff22a4690d7b01a652b121026d41 Mon Sep 17 00:00:00 2001 From: Linquize Date: Thu, 22 Oct 2015 07:56:34 +0800 Subject: [PATCH 18/23] inttypes.h is built-in header file since MSVC 2013 The reason is that the types defined in libgit2's inttypes.h collide with system inttypes.h 3rd party library header files may directly reference MSVC's built-in inttypes.h Fixes #3476 --- include/git2/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/git2/common.h b/include/git2/common.h index 748226385d6..57790611560 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -20,7 +20,7 @@ # define GIT_END_DECL /* empty */ #endif -#ifdef _MSC_VER +#if defined(_MSC_VER) && _MSC_VER < 1800 GIT_BEGIN_DECL # include "inttypes.h" GIT_END_DECL From 99a09f7f18f321c2a37e483bcd8aeb6dff781abc Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 22 Oct 2015 09:29:40 -0400 Subject: [PATCH 19/23] index: test that we round-trip nsecs Test that nanoseconds are round-tripped correctly when we read an index file that contains them. We should, however, ignore them because we don't understand them, and any new entries in the index should contain a `0` nsecs field, while existing preserving entries. --- tests/index/nsec.c | 78 ++++++++++++++++++ tests/resources/nsecs/.gitted/HEAD | 1 + tests/resources/nsecs/.gitted/config | 8 ++ tests/resources/nsecs/.gitted/index | Bin 0 -> 281 bytes .../03/1986a8372d1442cfe9e3b54906a9aadc524a7e | 2 + .../03/9afd91c98f82c14e425bb6796d8ca98e9c8cac | Bin 0 -> 102 bytes .../6d/8b18077cc99abd8dda05a6062c646406abb2d4 | Bin 0 -> 22 bytes .../c5/12b6c64656b87ea8caf37a32bc5a562d797745 | Bin 0 -> 22 bytes .../df/78d3d51c369e1d2f1eadb73464aadd931d56b4 | Bin 0 -> 22 bytes .../resources/nsecs/.gitted/refs/heads/master | 1 + tests/resources/nsecs/a.txt | 1 + tests/resources/nsecs/b.txt | 1 + tests/resources/nsecs/c.txt | 1 + 13 files changed, 93 insertions(+) create mode 100644 tests/index/nsec.c create mode 100644 tests/resources/nsecs/.gitted/HEAD create mode 100644 tests/resources/nsecs/.gitted/config create mode 100644 tests/resources/nsecs/.gitted/index create mode 100644 tests/resources/nsecs/.gitted/objects/03/1986a8372d1442cfe9e3b54906a9aadc524a7e create mode 100644 tests/resources/nsecs/.gitted/objects/03/9afd91c98f82c14e425bb6796d8ca98e9c8cac create mode 100644 tests/resources/nsecs/.gitted/objects/6d/8b18077cc99abd8dda05a6062c646406abb2d4 create mode 100644 tests/resources/nsecs/.gitted/objects/c5/12b6c64656b87ea8caf37a32bc5a562d797745 create mode 100644 tests/resources/nsecs/.gitted/objects/df/78d3d51c369e1d2f1eadb73464aadd931d56b4 create mode 100644 tests/resources/nsecs/.gitted/refs/heads/master create mode 100644 tests/resources/nsecs/a.txt create mode 100644 tests/resources/nsecs/b.txt create mode 100644 tests/resources/nsecs/c.txt diff --git a/tests/index/nsec.c b/tests/index/nsec.c new file mode 100644 index 00000000000..5004339f0ea --- /dev/null +++ b/tests/index/nsec.c @@ -0,0 +1,78 @@ +#include "clar_libgit2.h" +#include "index.h" +#include "git2/sys/index.h" +#include "git2/repository.h" +#include "../reset/reset_helpers.h" + +static git_repository *repo; +static git_index *repo_index; + +#define TEST_REPO_PATH "nsecs" + +// Fixture setup and teardown +void test_index_nsec__initialize(void) +{ + repo = cl_git_sandbox_init("nsecs"); + git_repository_index(&repo_index, repo); +} + +void test_index_nsec__cleanup(void) +{ + git_index_free(repo_index); + repo_index = NULL; + + cl_git_sandbox_cleanup(); +} + +static bool has_nsecs(void) +{ + const git_index_entry *entry; + size_t i; + bool has_nsecs = false; + + for (i = 0; i < git_index_entrycount(repo_index); i++) { + entry = git_index_get_byindex(repo_index, i); + + if (entry->ctime.nanoseconds || entry->mtime.nanoseconds) { + has_nsecs = true; + break; + } + } + + return has_nsecs; +} + +void test_index_nsec__has_nanos(void) +{ + cl_assert_equal_b(true, has_nsecs()); +} + +void test_index_nsec__staging_maintains_other_nanos(void) +{ + const git_index_entry *entry; + + cl_git_rewritefile("nsecs/a.txt", "This is file A"); + cl_git_pass(git_index_add_bypath(repo_index, "a.txt")); + cl_git_pass(git_index_write(repo_index)); + + cl_git_pass(git_index_write(repo_index)); + + git_index_read(repo_index, 1); + cl_assert_equal_b(true, has_nsecs()); + + cl_assert((entry = git_index_get_bypath(repo_index, "a.txt", 0))); + cl_assert_equal_i(0, entry->ctime.nanoseconds); + cl_assert_equal_i(0, entry->mtime.nanoseconds); +} + +void test_index_nsec__status_doesnt_clear_nsecs(void) +{ + git_status_list *statuslist; + + cl_git_pass(git_status_list_new(&statuslist, repo, NULL)); + + git_index_read(repo_index, 1); + cl_assert_equal_b(true, has_nsecs()); + + git_status_list_free(statuslist); +} diff --git a/tests/resources/nsecs/.gitted/HEAD b/tests/resources/nsecs/.gitted/HEAD new file mode 100644 index 00000000000..cb089cd89a7 --- /dev/null +++ b/tests/resources/nsecs/.gitted/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests/resources/nsecs/.gitted/config b/tests/resources/nsecs/.gitted/config new file mode 100644 index 00000000000..78387c50b47 --- /dev/null +++ b/tests/resources/nsecs/.gitted/config @@ -0,0 +1,8 @@ +[core] + repositoryformatversion = 0 + filemode = false + bare = false + logallrefupdates = true + symlinks = false + ignorecase = true + hideDotFiles = dotGitOnly diff --git a/tests/resources/nsecs/.gitted/index b/tests/resources/nsecs/.gitted/index new file mode 100644 index 0000000000000000000000000000000000000000..9233f1b11e8179f0f9655631bd9064d4e4229fe0 GIT binary patch literal 281 zcmZ?q402{*U|<4b<}eMjgR(2EG=TJB_Mr0bI!T~d62^l|LnLd9wO)Yqy)Ethzf{Hf#$6YckxN zkRVrApk_%1V+8{)=2?Fyp6qWr=;su@tunV~W#62hH5rSxp7WY#$)m^phWFf~Ww-2J HvG4-`s})W= literal 0 HcmV?d00001 diff --git a/tests/resources/nsecs/.gitted/objects/03/1986a8372d1442cfe9e3b54906a9aadc524a7e b/tests/resources/nsecs/.gitted/objects/03/1986a8372d1442cfe9e3b54906a9aadc524a7e new file mode 100644 index 00000000000..a813b74244d --- /dev/null +++ b/tests/resources/nsecs/.gitted/objects/03/1986a8372d1442cfe9e3b54906a9aadc524a7e @@ -0,0 +1,2 @@ +xA +0D]J4DMu-/"FHFf P aʠZ rnX*4kUixK-#%y Z20;Џ; ŰJZ7FRBy?g?<^@]f˔GvܵNUOKv \ No newline at end of file diff --git a/tests/resources/nsecs/.gitted/objects/03/9afd91c98f82c14e425bb6796d8ca98e9c8cac b/tests/resources/nsecs/.gitted/objects/03/9afd91c98f82c14e425bb6796d8ca98e9c8cac new file mode 100644 index 0000000000000000000000000000000000000000..74bb7d3feae8d45514385a02947566a60e2e7556 GIT binary patch literal 102 zcmV-s0Ga=I0V^p=O;xb8WH2-^Ff%bxNYpE-C}GI$mSC?rIcsn4E!JghIw>h^t2bSN zDo6q=I4ZR5m|NJ6x)rBBR~hYz3e&ADcZDiQ1}nH Date: Thu, 22 Oct 2015 09:30:41 -0400 Subject: [PATCH 20/23] diff: ignore nsecs when diffing Although our index contains the literal time present in the index, we do not read nanoseconds from disk, and thus we should not use them in any comparisons, lest we always think our working directory is dirty. Guard this behind a `GIT_USE_NSECS` for future improvement. --- src/diff.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/diff.c b/src/diff.c index d97dcd9d221..d98a2896643 100644 --- a/src/diff.c +++ b/src/diff.c @@ -493,8 +493,10 @@ static int diff_list_apply_options( /* Don't set GIT_DIFFCAPS_USE_DEV - compile time option in core git */ - /* Set GIT_DIFFCAPS_TRUST_NANOSECS on a platform basis */ + /* Don't trust nanoseconds; we do not load nanos from disk */ +#ifdef GIT_USE_NSEC diff->diffcaps = diff->diffcaps | GIT_DIFFCAPS_TRUST_NANOSECS; +#endif /* If not given explicit `opts`, check `diff.xyz` configs */ if (!opts) { From c7b336b0845eaa8f629bf24dbb0c666612a927ab Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 22 Oct 2015 10:29:51 -0400 Subject: [PATCH 21/23] xdiff: reference util.h in parent directory Although CMake will correctly configure include directories for us, some people may use their own build system, and we should reference `util.h` based on where it actually lives. --- src/xdiff/xdiff.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xdiff/xdiff.h b/src/xdiff/xdiff.h index db5d59884a4..f08f72e16a3 100644 --- a/src/xdiff/xdiff.h +++ b/src/xdiff/xdiff.h @@ -20,7 +20,7 @@ * */ -#include "util.h" +#include "../util.h" #if !defined(XDIFF_H) #define XDIFF_H From 8683d31f080eb12fe769ab2363165ec17562c840 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 22 Oct 2015 14:39:20 -0400 Subject: [PATCH 22/23] merge: add GIT_MERGE_TREE_FAIL_ON_CONFLICT Provide a new merge option, GIT_MERGE_TREE_FAIL_ON_CONFLICT, which will stop on the first conflict and fail the merge operation with GIT_EMERGECONFLICT. --- include/git2/errors.h | 1 + include/git2/merge.h | 7 +++++++ src/merge.c | 9 ++++++++- tests/merge/merge_helpers.c | 9 +++++---- tests/merge/trees/commits.c | 18 +++++++++++++++++- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/include/git2/errors.h b/include/git2/errors.h index 4698366d852..1b528cf25ac 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -49,6 +49,7 @@ typedef enum { GIT_EINVALID = -21, /**< Invalid operation or input */ GIT_EUNCOMMITTED = -22, /**< Uncommitted changes in index prevented operation */ GIT_EDIRECTORY = -23, /**< The operation is not valid for a directory */ + GIT_EMERGECONFLICT = -24, /**< A merge conflict exists and cannot continue */ GIT_PASSTHROUGH = -30, /**< Internal only */ GIT_ITEROVER = -31, /**< Signals end of iteration with iterator */ diff --git a/include/git2/merge.h b/include/git2/merge.h index 5fef452b9fd..ced9e51ffe5 100644 --- a/include/git2/merge.h +++ b/include/git2/merge.h @@ -72,6 +72,13 @@ typedef enum { * the ability to merge between a modified and renamed file. */ GIT_MERGE_TREE_FIND_RENAMES = (1 << 0), + + /** + * If a conflict occurs, exit immediately instead of attempting to + * continue resolving conflicts. The merge operation will fail with + * GIT_EMERGECONFLICT and no index will be returned. + */ + GIT_MERGE_TREE_FAIL_ON_CONFLICT = (1 << 1), } git_merge_tree_flag_t; /** diff --git a/src/merge.c b/src/merge.c index 930457bdb27..3bed0fd3b60 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1701,8 +1701,15 @@ int git_merge__iterators( if ((error = merge_conflict_resolve(&resolved, diff_list, conflict, opts.file_favor, opts.file_flags)) < 0) goto done; - if (!resolved) + if (!resolved) { + if ((opts.tree_flags & GIT_MERGE_TREE_FAIL_ON_CONFLICT)) { + giterr_set(GITERR_MERGE, "merge conflicts exist"); + error = GIT_EMERGECONFLICT; + goto done; + } + git_vector_insert(&diff_list->conflicts, conflict); + } } if (!given_opts || !given_opts->metric) diff --git a/tests/merge/merge_helpers.c b/tests/merge/merge_helpers.c index f8147142410..986a365db85 100644 --- a/tests/merge/merge_helpers.c +++ b/tests/merge/merge_helpers.c @@ -40,7 +40,7 @@ int merge_trees_from_branches( cl_git_pass(git_commit_tree(&our_tree, our_commit)); cl_git_pass(git_commit_tree(&their_tree, their_commit)); - cl_git_pass(git_merge_trees(index, repo, ancestor_tree, our_tree, their_tree, opts)); + error = git_merge_trees(index, repo, ancestor_tree, our_tree, their_tree, opts); git_buf_free(&branch_buf); git_tree_free(our_tree); @@ -50,7 +50,7 @@ int merge_trees_from_branches( git_commit_free(their_commit); git_commit_free(ancestor_commit); - return 0; + return error; } int merge_commits_from_branches( @@ -61,6 +61,7 @@ int merge_commits_from_branches( git_commit *our_commit, *their_commit; git_oid our_oid, their_oid; git_buf branch_buf = GIT_BUF_INIT; + int error; git_buf_printf(&branch_buf, "%s%s", GIT_REFS_HEADS_DIR, ours_name); cl_git_pass(git_reference_name_to_id(&our_oid, repo, branch_buf.ptr)); @@ -71,13 +72,13 @@ int merge_commits_from_branches( cl_git_pass(git_reference_name_to_id(&their_oid, repo, branch_buf.ptr)); cl_git_pass(git_commit_lookup(&their_commit, repo, &their_oid)); - cl_git_pass(git_merge_commits(index, repo, our_commit, their_commit, opts)); + error = git_merge_commits(index, repo, our_commit, their_commit, opts); git_buf_free(&branch_buf); git_commit_free(our_commit); git_commit_free(their_commit); - return 0; + return error; } int merge_branches(git_repository *repo, diff --git a/tests/merge/trees/commits.c b/tests/merge/trees/commits.c index c4e4709977c..2e3c4578b4d 100644 --- a/tests/merge/trees/commits.c +++ b/tests/merge/trees/commits.c @@ -94,7 +94,6 @@ void test_merge_trees_commits__no_ancestor(void) git_index_free(index); } - void test_merge_trees_commits__df_conflict(void) { git_index *index; @@ -129,3 +128,20 @@ void test_merge_trees_commits__df_conflict(void) git_index_free(index); } + +void test_merge_trees_commits__fail_on_conflict(void) +{ + git_index *index; + git_merge_options opts = GIT_MERGE_OPTIONS_INIT; + + opts.tree_flags |= GIT_MERGE_TREE_FAIL_ON_CONFLICT; + + cl_git_fail_with(GIT_EMERGECONFLICT, + merge_trees_from_branches(&index, repo, "df_side1", "df_side2", &opts)); + + cl_git_fail_with(GIT_EMERGECONFLICT, + merge_commits_from_branches(&index, repo, "master", "unrelated", &opts)); + cl_git_fail_with(GIT_EMERGECONFLICT, + merge_commits_from_branches(&index, repo, "master", "branch", &opts)); +} + From 7208ff4d7dd744edcdc4345c97130568e3ea7638 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 22 Oct 2015 20:17:19 -0500 Subject: [PATCH 23/23] cmake: split sources into original paths for Xcode and MSVC The MSVC_SPLIT_SOURCES function is helpful for other IDEs, like Xcode, and will split the source files up into their target directories, instead of merely placing them all in a "Sources" directory. Rename MSVC_SPLIT_SOURCES to IDE_SPLIT_SOURCES and enable it for Xcode. --- CMakeLists.txt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8f58eb0e785..92e0081cabe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -137,13 +137,13 @@ FUNCTION(TARGET_OS_LIBRARIES target) ENDIF() ENDFUNCTION() -# For the MSVC IDE, this function splits up the source files like windows -# explorer does. This is esp. useful with the libgit2_clar project, were -# usually 2 or more files share the same name. Sadly, this file grouping -# is a per-directory option in cmake and not per-target, resulting in -# empty virtual folders "tests" for the git2.dll -FUNCTION(MSVC_SPLIT_SOURCES target) - IF(MSVC_IDE) +# This function splits the sources files up into their appropriate +# subdirectories. This is especially useful for IDEs like Xcode and +# Visual Studio, so that you can navigate into the libgit2_clar project, +# and see the folders within the tests folder (instead of just seeing all +# source and tests in a single folder.) +FUNCTION(IDE_SPLIT_SOURCES target) + IF(MSVC_IDE OR CMAKE_GENERATOR STREQUAL Xcode) GET_TARGET_PROPERTY(sources ${target} SOURCES) FOREACH(source ${sources}) IF(source MATCHES ".*/") @@ -560,7 +560,7 @@ IF(MSVC AND GIT_ARCH_64 AND NOT BUILD_SHARED_LIBS) SET_TARGET_PROPERTIES(git2 PROPERTIES STATIC_LIBRARY_FLAGS "/MACHINE:x64") ENDIF() -MSVC_SPLIT_SOURCES(git2) +IDE_SPLIT_SOURCES(git2) IF (SONAME) SET_TARGET_PROPERTIES(git2 PROPERTIES VERSION ${LIBGIT2_VERSION_STRING}) @@ -629,7 +629,7 @@ IF (BUILD_CLAR) TARGET_LINK_LIBRARIES(libgit2_clar ${GSSAPI_LIBRARIES}) TARGET_LINK_LIBRARIES(libgit2_clar ${ICONV_LIBRARIES}) TARGET_OS_LIBRARIES(libgit2_clar) - MSVC_SPLIT_SOURCES(libgit2_clar) + IDE_SPLIT_SOURCES(libgit2_clar) IF (MSVC_IDE) # Precompiled headers