From 16541b864d960c6a600938d82c7472f5a188206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 25 Apr 2016 12:16:05 +0200 Subject: [PATCH 01/67] tag: ignore extra header fields While no extra header fields are defined for tags, git accepts them by ignoring them and continuing the search for the message. There are a few tags like this in the wild which git parses just fine, so we should do the same. --- src/tag.c | 10 ++++++++-- tests/object/tag/read.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/tag.c b/src/tag.c index c4bce1f22..fe840fe82 100644 --- a/src/tag.c +++ b/src/tag.c @@ -137,8 +137,14 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) tag->message = NULL; if (buffer < buffer_end) { - if( *buffer != '\n' ) - return tag_error("No new line before message"); + /* If we're not at the end of the header, search for it */ + if( *buffer != '\n' ) { + search = strstr(buffer, "\n\n"); + if (search) + buffer = search + 1; + else + return tag_error("tag contains no message"); + } text_len = buffer_end - ++buffer; diff --git a/tests/object/tag/read.c b/tests/object/tag/read.c index c9787a413..8f28afd54 100644 --- a/tests/object/tag/read.c +++ b/tests/object/tag/read.c @@ -140,3 +140,40 @@ void test_object_tag_read__without_tagger_nor_message(void) git_tag_free(tag); git_repository_free(repo); } + +static const char *silly_tag = "object c054ccaefbf2da31c3b19178f9e3ef20a3867924\n\ +type commit\n\ +tag v1_0_1\n\ +tagger Jamis Buck 1107717917\n\ +diff --git a/lib/sqlite3/version.rb b/lib/sqlite3/version.rb\n\ +index 0b3bf69..4ee8fc2 100644\n\ +--- a/lib/sqlite3/version.rb\n\ ++++ b/lib/sqlite3/version.rb\n\ +@@ -36,7 +36,7 @@ module SQLite3\n\ + \n\ + MAJOR = 1\n\ + MINOR = 0\n\ +- TINY = 0\n\ ++ TINY = 1\n\ + \n\ + STRING = [ MAJOR, MINOR, TINY ].join( \".\" )\n\ + \n\ + -0600\n\ +\n\ +v1_0_1 release\n"; + +void test_object_tag_read__extra_header_fields(void) +{ + git_tag *tag; + git_odb *odb; + git_oid id; + + cl_git_pass(git_repository_odb__weakptr(&odb, g_repo)); + + cl_git_pass(git_odb_write(&id, odb, silly_tag, strlen(silly_tag), GIT_OBJ_TAG)); + cl_git_pass(git_tag_lookup(&tag, g_repo, &id)); + + cl_assert_equal_s("v1_0_1 release\n", git_tag_message(tag)); + + git_tag_free(tag); +} From b726c539918dd44162a2e817ed86dbd152fba83c Mon Sep 17 00:00:00 2001 From: Christian Schlack Date: Tue, 26 Apr 2016 18:04:03 +0200 Subject: [PATCH 02/67] Fix return value of openssl_read (infinite loop) openssl_read should return -1 in case of error. SSL_read returns values <= 0 in case of error. A return value of 0 can lead to an infinite loop, so the return value of ssl_set_error will be returned if SSL_read is not successful (analog to openssl_write). --- src/openssl_stream.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/openssl_stream.c b/src/openssl_stream.c index a65f5586e..9d97bae00 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -522,8 +522,9 @@ ssize_t openssl_read(git_stream *stream, void *data, size_t len) openssl_stream *st = (openssl_stream *) stream; int ret; - if ((ret = SSL_read(st->ssl, data, len)) <= 0) - ssl_set_error(st->ssl, ret); + if ((ret = SSL_read(st->ssl, data, len)) <= 0) { + return ssl_set_error(st->ssl, ret); + } return ret; } From 1aacaa315494ac5df1f35fc012e556072188c773 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 29 Apr 2016 10:18:04 -0400 Subject: [PATCH 03/67] cmake: include threading libraries in pkg-config Include any required threading libraries in our `libgit2.pc`. --- CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c79b2637c..89c68128c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -161,6 +161,8 @@ FUNCTION(TARGET_OS_LIBRARIES target) IF(THREADSAFE) TARGET_LINK_LIBRARIES(${target} ${CMAKE_THREAD_LIBS_INIT}) + LIST(APPEND LIBGIT2_PC_LIBS ${CMAKE_THREAD_LIBS_INIT}) + SET(LIBGIT2_PC_LIBS ${LIBGIT2_PC_LIBS} PARENT_SCOPE) ENDIF() ENDFUNCTION() From fc2ef5143b5ee5aaddb56bbff98815cea5241244 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 May 2016 14:30:14 +0200 Subject: [PATCH 04/67] index: fix memory leak on error case --- src/index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.c b/src/index.c index 63e47965a..31cb27d6c 100644 --- a/src/index.c +++ b/src/index.c @@ -3008,7 +3008,7 @@ int git_index_read_index( if (error < 0) { giterr_set(GITERR_INDEX, "failed to insert entry"); - return error; + goto done; } if (diff <= 0) { From 66633e836f192054746941e274e9e9008ee5cdb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 27 Apr 2016 12:00:31 +0200 Subject: [PATCH 05/67] odb: avoid inflating the full delta to read the header When we read the header, we want to know the size and type of the object. We're currently inflating the full delta in order to read the first few bytes. This can mean hundreds of kB needlessly inflated for large objects. Instead use a packfile stream to read just enough so we can read the two varints in the header and avoid inflating most of the delta. --- src/delta-apply.c | 31 +++++++++++++++++++++++++++++++ src/delta-apply.h | 12 ++++++++++++ src/pack.c | 11 +++++------ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/delta-apply.c b/src/delta-apply.c index 89745faa0..6e86a81db 100644 --- a/src/delta-apply.c +++ b/src/delta-apply.c @@ -49,6 +49,37 @@ int git__delta_read_header( return 0; } +#define DELTA_HEADER_BUFFER_LEN 16 +int git__delta_read_header_fromstream(size_t *base_sz, size_t *res_sz, git_packfile_stream *stream) +{ + static const size_t buffer_len = DELTA_HEADER_BUFFER_LEN; + unsigned char buffer[DELTA_HEADER_BUFFER_LEN]; + const unsigned char *delta, *delta_end; + size_t len; + ssize_t read; + + len = read = 0; + while (len < buffer_len) { + read = git_packfile_stream_read(stream, &buffer[len], buffer_len - len); + + if (read == 0) + break; + + if (read == GIT_EBUFS) + continue; + + len += read; + } + + delta = buffer; + delta_end = delta + len; + if ((hdr_sz(base_sz, &delta, delta_end) < 0) || + (hdr_sz(res_sz, &delta, delta_end) < 0)) + return -1; + + return 0; +} + int git__delta_apply( git_rawobj *out, const unsigned char *base, diff --git a/src/delta-apply.h b/src/delta-apply.h index d7d99d04c..eeeb78682 100644 --- a/src/delta-apply.h +++ b/src/delta-apply.h @@ -8,6 +8,7 @@ #define INCLUDE_delta_apply_h__ #include "odb.h" +#include "pack.h" /** * Apply a git binary delta to recover the original content. @@ -47,4 +48,15 @@ extern int git__delta_read_header( size_t *base_sz, size_t *res_sz); +/** + * Read the header of a git binary delta + * + * This variant reads just enough from the packfile stream to read the + * delta header. + */ +extern int git__delta_read_header_fromstream( + size_t *base_sz, + size_t *res_sz, + git_packfile_stream *stream); + #endif diff --git a/src/pack.c b/src/pack.c index e7003e66d..6a700e29f 100644 --- a/src/pack.c +++ b/src/pack.c @@ -499,15 +499,14 @@ int git_packfile_resolve_header( if (type == GIT_OBJ_OFS_DELTA || type == GIT_OBJ_REF_DELTA) { size_t base_size; - git_rawobj delta; + git_packfile_stream stream; + base_offset = get_delta_base(p, &w_curs, &curpos, type, offset); git_mwindow_close(&w_curs); - error = packfile_unpack_compressed(&delta, p, &w_curs, &curpos, size, type); - git_mwindow_close(&w_curs); - if (error < 0) + if ((error = git_packfile_stream_open(&stream, p, curpos)) < 0) return error; - error = git__delta_read_header(delta.data, delta.len, &base_size, size_p); - git__free(delta.data); + error = git__delta_read_header_fromstream(&base_size, size_p, &stream); + git_packfile_stream_free(&stream); if (error < 0) return error; } else From f627e196627f2d2284186140d5d5103421664a13 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 May 2016 15:47:54 +0200 Subject: [PATCH 06/67] checkout: set ignorecase=0 when config lookup fails When `git_repository__cvar` fails we may end up with a `ignorecase` value of `-1`. As we subsequently check if `ignorecase` is non-zero, we may end up reporting that data should be removed when in fact it should not. Err on the safer side and set `ignorecase = 0` when `git_repository__cvar` fails. --- src/checkout.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index deeee62e0..a7dddceec 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1342,9 +1342,11 @@ fail: static bool should_remove_existing(checkout_data *data) { - int ignorecase = 0; + int ignorecase; - git_repository__cvar(&ignorecase, data->repo, GIT_CVAR_IGNORECASE); + if (git_repository__cvar(&ignorecase, data->repo, GIT_CVAR_IGNORECASE) < 0) { + ignorecase = 0; + } return (ignorecase && (data->strategy & GIT_CHECKOUT_DONT_REMOVE_EXISTING) == 0); From 1fb8a951b6ce2472753c502ff703e29be1307604 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 May 2016 16:24:14 +0200 Subject: [PATCH 07/67] odb_loose: fix undefined behavior when computing size An object's size is computed by reading the object header's size field until the most significant bit is not set anymore. To get the total size, we increase the shift on each iteration and add the shifted value to the total size. We read the current value into a variable of type `unsigned char`, from which we then take all bits except the most significant bit and shift the result. We will end up with a maximum shift of 60, but this exceeds the width of the value's type, resulting in undefined behavior. Fix the issue by instead reading the values into a variable of type `unsigned long`, which matches the required width. This is equivalent to git.git, which uses an `unsigned long` as well. --- src/odb_loose.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odb_loose.c b/src/odb_loose.c index 9d9bffd21..3c33160d0 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -91,7 +91,7 @@ static int object_mkdir(const git_buf *name, const loose_backend *be) static size_t get_binary_object_header(obj_hdr *hdr, git_buf *obj) { - unsigned char c; + unsigned long c; unsigned char *data = (unsigned char *)obj->ptr; size_t shift, size, used = 0; From cf0396a563ce9ac81eb550d83d53fda519fa4a6f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 2 May 2016 16:49:59 +0200 Subject: [PATCH 08/67] delta-apply: fix sign extension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We compute offsets by executing `off |= (*delta++ << 24)` for multiple constants, where `off` is of type `size_t` and `delta` is of type `unsigned char`. The usual arithmetic conversions (see ISO C89 §3.2.1.5 "Usual arithmetic conversions") kick in here, causing us to promote both operands to `int` and then extending the result to an `unsigned long` when OR'ing it with `off`. The integer promotion to `int` may result in wrong size calculations for big values. Fix the issue by making the constants `unsigned long`, causing both operands to be promoted to `unsigned long`. --- src/delta-apply.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/delta-apply.c b/src/delta-apply.c index 6e86a81db..02ec7b75e 100644 --- a/src/delta-apply.c +++ b/src/delta-apply.c @@ -121,13 +121,13 @@ int git__delta_apply( size_t off = 0, len = 0; if (cmd & 0x01) off = *delta++; - if (cmd & 0x02) off |= *delta++ << 8; - if (cmd & 0x04) off |= *delta++ << 16; - if (cmd & 0x08) off |= *delta++ << 24; + if (cmd & 0x02) off |= *delta++ << 8UL; + if (cmd & 0x04) off |= *delta++ << 16UL; + if (cmd & 0x08) off |= *delta++ << 24UL; if (cmd & 0x10) len = *delta++; - if (cmd & 0x20) len |= *delta++ << 8; - if (cmd & 0x40) len |= *delta++ << 16; + if (cmd & 0x20) len |= *delta++ << 8UL; + if (cmd & 0x40) len |= *delta++ << 16UL; if (!len) len = 0x10000; if (base_len < off + len || res_sz < len) From 849a1a4345c6392b491ad0d326095638e0ef9e73 Mon Sep 17 00:00:00 2001 From: Lucas Derraugh Date: Thu, 5 May 2016 23:34:23 -0400 Subject: [PATCH 09/67] Fix unused variable 'message' warning --- src/stransport_stream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 33b6c5c38..832f66b45 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -33,6 +33,7 @@ int stransport_error(OSStatus ret) CFRelease(message); #else giterr_set(GITERR_NET, "SecureTransport error: OSStatus %d", (unsigned int)ret); + GIT_UNUSED(message); #endif return -1; From 78b5702ed5fe2b4dd7010e0e64640cdd7d564867 Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Wed, 18 May 2016 16:00:01 -0500 Subject: [PATCH 10/67] Fix comment for GIT_FILEMODE_LINK 0120000 is symbolic link, not commit --- src/tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tree.c b/src/tree.c index 6ce460c6d..3874e45f4 100644 --- a/src/tree.c +++ b/src/tree.c @@ -45,7 +45,7 @@ GIT_INLINE(git_filemode_t) normalize_filemode(git_filemode_t filemode) if (GIT_MODE_TYPE(filemode) == GIT_FILEMODE_COMMIT) return GIT_FILEMODE_COMMIT; - /* 12XXXX means commit */ + /* 12XXXX means symlink */ if (GIT_MODE_TYPE(filemode) == GIT_FILEMODE_LINK) return GIT_FILEMODE_LINK; From 488937c22d840ec04b189beb52150a750107f1ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Revol?= Date: Sun, 22 May 2016 23:23:58 +0200 Subject: [PATCH 11/67] CMakeLists: Add libnetwork for Haiku --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 89c68128c..c9d72d331 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -151,6 +151,10 @@ FUNCTION(TARGET_OS_LIBRARIES target) TARGET_LINK_LIBRARIES(${target} socket nsl) LIST(APPEND LIBGIT2_PC_LIBS "-lsocket" "-lnsl") SET(LIBGIT2_PC_LIBS ${LIBGIT2_PC_LIBS} PARENT_SCOPE) + ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Haiku") + TARGET_LINK_LIBRARIES(${target} network) + LIST(APPEND LIBGIT2_PC_LIBS "-lnetwork") + SET(LIBGIT2_PC_LIBS ${LIBGIT2_PC_LIBS} PARENT_SCOPE) ENDIF() CHECK_LIBRARY_EXISTS(rt clock_gettime "time.h" NEED_LIBRT) IF(NEED_LIBRT) From 26917fd9d1acfb215f673c77e0e1c40b06ec37fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Revol?= Date: Tue, 24 May 2016 19:07:09 +0200 Subject: [PATCH 12/67] test: Fix stat() test to mask out unwanted bits Haiku and Hurd both pass extra bits in struct stat::st_mode. --- tests/checkout/index.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/checkout/index.c b/tests/checkout/index.c index 8af3e5684..ca63dc3f8 100644 --- a/tests/checkout/index.c +++ b/tests/checkout/index.c @@ -294,11 +294,12 @@ void test_checkout_index__options_dir_modes(void) (void)p_umask(um = p_umask(022)); cl_git_pass(p_stat("./testrepo/a", &st)); - cl_assert_equal_i_fmt(st.st_mode, (GIT_FILEMODE_TREE | 0701) & ~um, "%07o"); + /* Haiku & Hurd use other mode bits, so we must mask them out */ + cl_assert_equal_i_fmt(st.st_mode & (S_IFMT | 07777), (GIT_FILEMODE_TREE | 0701) & ~um, "%07o"); /* File-mode test, since we're on the 'dir' branch */ cl_git_pass(p_stat("./testrepo/a/b.txt", &st)); - cl_assert_equal_i_fmt(st.st_mode, GIT_FILEMODE_BLOB_EXECUTABLE & ~um, "%07o"); + cl_assert_equal_i_fmt(st.st_mode & (S_IFMT | 07777), GIT_FILEMODE_BLOB_EXECUTABLE & ~um, "%07o"); git_commit_free(commit); } From 70681ff740c061e38f21abd33023d636d99621ce Mon Sep 17 00:00:00 2001 From: Jason Haslam Date: Tue, 16 Feb 2016 21:02:41 -0700 Subject: [PATCH 13/67] checkout: handle dirty submodules correctly Don't generate conflicts when checking out a modified submodule and the submodule is dirty or modified in the workdir. --- docs/checkout-internals.md | 29 ++++++++++++- src/checkout.c | 3 +- tests/checkout/typechange.c | 82 +++++++++++++++++++++++++++++++++---- 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/docs/checkout-internals.md b/docs/checkout-internals.md index 6147ffdd8..e0b2583b5 100644 --- a/docs/checkout-internals.md +++ b/docs/checkout-internals.md @@ -66,6 +66,8 @@ Key - Bi - ignored blob (WD only) - T1,T2,T3 - trees with different SHAs, - Ti - ignored tree (WD only) +- S1,S2 - submodules with different SHAs +- Sd - dirty submodule (WD only) - x - nothing Diff with 2 non-workdir iterators @@ -162,6 +164,27 @@ Checkout From 3 Iterators (2 not workdir, 1 workdir) | 35+ | T1 | T2 | x | update locally deleted tree (SAFE+MISSING) | | 36* | T1 | T2 | B1/Bi | update to tree with typechanged tree->blob conflict (F-1) | | 37 | T1 | T2 | T1/T2/T3 | update to existing tree (MAYBE SAFE) | +| 38+ | x | S1 | x | add submodule (SAFE) | +| 39 | x | S1 | S1/Sd | independently added submodule (SUBMODULE) | +| 40* | x | S1 | B1 | add submodule with blob confilct (FORCEABLE) | +| 41* | x | S1 | T1 | add submodule with tree conflict (FORCEABLE) | +| 42 | S1 | x | S1/Sd | deleted submodule (SUBMODULE) | +| 43 | S1 | x | x | independently deleted submodule (SUBMODULE) | +| 44 | S1 | x | B1 | independently deleted submodule with added blob (SAFE+MISSING) | +| 45 | S1 | x | T1 | independently deleted submodule with added tree (SAFE+MISSING) | +| 46 | S1 | S1 | x | locally deleted submodule (SUBMODULE) | +| 47+ | S1 | S2 | x | update locally deleted submodule (SAFE) | +| 48 | S1 | S1 | S2 | locally updated submodule commit (SUBMODULE) | +| 49 | S1 | S2 | S1 | updated submodule commit (SUBMODULE) | +| 50+ | S1 | B1 | x | add blob with locally deleted submodule (SAFE+MISSING) | +| 51* | S1 | B1 | S1 | typechange submodule->blob (SAFE) | +| 52* | S1 | B1 | Sd | typechange dirty submodule->blob (SAFE!?!?) | +| 53+ | S1 | T1 | x | add tree with locally deleted submodule (SAFE+MISSING) | +| 54* | S1 | T1 | S1/Sd | typechange submodule->tree (MAYBE SAFE) | +| 55+ | B1 | S1 | x | add submodule with locally deleted blob (SAFE+MISSING) | +| 56* | B1 | S1 | B1 | typechange blob->submodule (SAFE) | +| 57+ | T1 | S1 | x | add submodule with locally deleted tree (SAFE+MISSING) | +| 58* | T1 | S1 | T1 | typechange tree->submodule (SAFE) | The number is followed by ' ' if no change is needed or '+' if the case @@ -176,6 +199,8 @@ There are four tiers of safe cases: content, which is unknown at this point * FORCEABLE == conflict unless FORCE is given * DIRTY == no conflict but change is not applied unless FORCE +* SUBMODULE == no conflict and no change is applied unless a deleted + submodule dir is empty Some slightly unusual circumstances: @@ -198,7 +223,9 @@ Some slightly unusual circumstances: cases, if baseline == target, we don't touch the workdir (it is either already right or is "dirty"). However, since this case also implies that a ?/B1/x case will exist as well, it can be skipped. +* 41 - It's not clear how core git distinguishes this case from 39 (mode?). +* 52 - Core git makes destructive changes without any warning when the + submodule is dirty and the type changes to a blob. Cases 3, 17, 24, 26, and 29 are all considered conflicts even though none of them will require making any updates to the working directory. - diff --git a/src/checkout.c b/src/checkout.c index a7dddceec..a40615945 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -464,7 +464,8 @@ static int checkout_action_with_wd( *action = CHECKOUT_ACTION_IF(SAFE, REMOVE, NONE); break; case GIT_DELTA_MODIFIED: /* case 16, 17, 18 (or 36 but not really) */ - if (checkout_is_workdir_modified(data, &delta->old_file, &delta->new_file, wd)) + if (wd->mode != GIT_FILEMODE_COMMIT && + checkout_is_workdir_modified(data, &delta->old_file, &delta->new_file, wd)) *action = CHECKOUT_ACTION_IF(FORCE, UPDATE_BLOB, CONFLICT); else *action = CHECKOUT_ACTION_IF(SAFE, UPDATE_BLOB, NONE); diff --git a/tests/checkout/typechange.c b/tests/checkout/typechange.c index b4959a351..c2949e3da 100644 --- a/tests/checkout/typechange.c +++ b/tests/checkout/typechange.c @@ -6,6 +6,36 @@ static git_repository *g_repo = NULL; +/* +From the test repo used for this test: +-------------------------------------- + +This is a test repo for libgit2 where tree entries have type changes + +The key types that could be found in tree entries are: + +1 - GIT_FILEMODE_NEW = 0000000 +2 - GIT_FILEMODE_TREE = 0040000 +3 - GIT_FILEMODE_BLOB = 0100644 +4 - GIT_FILEMODE_BLOB_EXECUTABLE = 0100755 +5 - GIT_FILEMODE_LINK = 0120000 +6 - GIT_FILEMODE_COMMIT = 0160000 + +I will try to have every type of transition somewhere in the history +of this repo. + +Commits +------- +Initial commit - a(1) b(1) c(1) d(1) e(1) +Create content - a(1->2) b(1->3) c(1->4) d(1->5) e(1->6) +Changes #1 - a(2->3) b(3->4) c(4->5) d(5->6) e(6->2) +Changes #2 - a(3->5) b(4->6) c(5->2) d(6->3) e(2->4) +Changes #3 - a(5->3) b(6->4) c(2->5) d(3->6) e(4->2) +Changes #4 - a(3->2) b(4->3) c(5->4) d(6->5) e(2->6) +Changes #5 - a(2->1) b(3->1) c(4->1) d(5->1) e(6->1) + +*/ + static const char *g_typechange_oids[] = { "79b9f23e85f55ea36a472a902e875bc1121a94cb", "9bdb75b73836a99e3dbeea640a81de81031fdc29", @@ -21,6 +51,14 @@ static bool g_typechange_empty[] = { true, false, false, false, false, false, true, true }; +static const int g_typechange_expected_conflicts[] = { + 1, 2, 3, 3, 2, 3, 2 +}; + +static const int g_typechange_expected_untracked[] = { + 6, 4, 3, 2, 3, 2, 5 +}; + void test_checkout_typechange__initialize(void) { g_repo = cl_git_sandbox_init("typechanges"); @@ -112,12 +150,7 @@ void test_checkout_typechange__checkout_typechanges_safe(void) for (i = 0; g_typechange_oids[i] != NULL; ++i) { cl_git_pass(git_revparse_single(&obj, g_repo, g_typechange_oids[i])); - opts.checkout_strategy = GIT_CHECKOUT_FORCE; - - /* There are bugs in some submodule->tree changes that prevent - * SAFE from passing here, even though the following should work: - */ - /* !i ? GIT_CHECKOUT_FORCE : GIT_CHECKOUT_SAFE; */ + opts.checkout_strategy = !i ? GIT_CHECKOUT_FORCE : GIT_CHECKOUT_SAFE; cl_git_pass(git_checkout_tree(g_repo, obj, &opts)); @@ -190,6 +223,35 @@ static void force_create_file(const char *file) cl_git_rewritefile(file, "yowza!!"); } +static int make_submodule_dirty(git_submodule *sm, const char *name, void *payload) +{ + git_buf submodulepath = GIT_BUF_INIT; + git_buf dirtypath = GIT_BUF_INIT; + git_repository *submodule_repo; + + /* remove submodule directory in preparation for init and repo_init */ + cl_git_pass(git_buf_joinpath( + &submodulepath, + git_repository_workdir(g_repo), + git_submodule_path(sm) + )); + git_futils_rmdir_r(git_buf_cstr(&submodulepath), NULL, GIT_RMDIR_REMOVE_FILES); + + /* initialize submodule and its repository */ + cl_git_pass(git_submodule_init(sm, 1)); + cl_git_pass(git_submodule_repo_init(&submodule_repo, sm, 0)); + + /* create a file in the submodule workdir to make it dirty */ + cl_git_pass( + git_buf_joinpath(&dirtypath, git_repository_workdir(submodule_repo), "dirty")); + force_create_file(git_buf_cstr(&dirtypath)); + + git_buf_free(&dirtypath); + git_buf_free(&submodulepath); + + return 0; +} + void test_checkout_typechange__checkout_with_conflicts(void) { int i; @@ -211,13 +273,17 @@ void test_checkout_typechange__checkout_with_conflicts(void) git_futils_rmdir_r("typechanges/d", NULL, GIT_RMDIR_REMOVE_FILES); p_mkdir("typechanges/d", 0777); /* intentionally empty dir */ force_create_file("typechanges/untracked"); + cl_git_pass(git_submodule_foreach(g_repo, make_submodule_dirty, NULL)); opts.checkout_strategy = GIT_CHECKOUT_SAFE; memset(&cts, 0, sizeof(cts)); cl_git_fail(git_checkout_tree(g_repo, obj, &opts)); - cl_assert(cts.conflicts > 0); - cl_assert(cts.untracked > 0); + cl_assert_equal_i(cts.conflicts, g_typechange_expected_conflicts[i]); + cl_assert_equal_i(cts.untracked, g_typechange_expected_untracked[i]); + cl_assert_equal_i(cts.dirty, 0); + cl_assert_equal_i(cts.updates, 0); + cl_assert_equal_i(cts.ignored, 0); opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_REMOVE_UNTRACKED; From 85ef6ec5f02b15163b10ad9153516d7fb3e42990 Mon Sep 17 00:00:00 2001 From: Jason Haslam Date: Thu, 12 May 2016 13:18:07 -0600 Subject: [PATCH 14/67] Ignore submodules when checking for merge conflicts in the workdir. --- src/merge.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/merge.c b/src/merge.c index d2f92ccce..174cbfb58 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2730,6 +2730,7 @@ static int merge_check_workdir(size_t *conflicts, git_repository *repo, git_inde opts.flags |= GIT_DIFF_DISABLE_PATHSPEC_MATCH; opts.pathspec.count = merged_paths->length; opts.pathspec.strings = (char **)merged_paths->contents; + opts.ignore_submodules = GIT_SUBMODULE_IGNORE_ALL; if ((error = git_diff_index_to_workdir(&wd_diff_list, repo, NULL, &opts)) < 0) goto done; From 68227c431a4ff07fc4b3272b035395949fae8036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 27 May 2016 10:20:35 +0300 Subject: [PATCH 15/67] Update CMakeLists.txt typo fix --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c9d72d331..fd683073b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,7 +40,7 @@ OPTION( USE_ICONV "Link with and use iconv library" OFF ) OPTION( USE_SSH "Link with libssh to enable SSH support" ON ) OPTION( USE_GSSAPI "Link with libgssapi for SPNEGO auth" OFF ) OPTION( VALGRIND "Configure build for valgrind" OFF ) -OPTION( CURL "User curl for HTTP if available" ON) +OPTION( CURL "Use curl for HTTP if available" ON) OPTION( DEBUG_POOL "Enable debug pool allocator" OFF ) IF(DEBUG_POOL) From bcef008f772f6530c7b6b138fcde0527016c6e61 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 26 May 2016 10:51:16 -0500 Subject: [PATCH 16/67] cleanup: unused warning --- tests/checkout/typechange.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/checkout/typechange.c b/tests/checkout/typechange.c index c2949e3da..1efea931a 100644 --- a/tests/checkout/typechange.c +++ b/tests/checkout/typechange.c @@ -229,6 +229,9 @@ static int make_submodule_dirty(git_submodule *sm, const char *name, void *paylo git_buf dirtypath = GIT_BUF_INIT; git_repository *submodule_repo; + GIT_UNUSED(name); + GIT_UNUSED(payload); + /* remove submodule directory in preparation for init and repo_init */ cl_git_pass(git_buf_joinpath( &submodulepath, From efadf28d846329ef2ac1a74a632942d5234b8cd8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 26 May 2016 12:39:09 -0500 Subject: [PATCH 17/67] filebuf: fix uninitialized warning --- src/filebuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filebuf.c b/src/filebuf.c index 101d5082a..507f6a942 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -70,7 +70,7 @@ static int lock_file(git_filebuf *file, int flags, mode_t mode) git_file source; char buffer[FILEIO_BUFSIZE]; ssize_t read_bytes; - int error; + int error = 0; source = p_open(file->path_original, O_RDONLY); if (source < 0) { From feea2849f980035dabdcdc1dc82f5de342821632 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 26 May 2016 12:52:29 -0500 Subject: [PATCH 18/67] win32: clean up unused warnings in DllMain --- src/global.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/global.c b/src/global.c index adf353d35..967944fa1 100644 --- a/src/global.c +++ b/src/global.c @@ -226,6 +226,9 @@ void git__free_tls_data(void) BOOL WINAPI DllMain(HINSTANCE hInstDll, DWORD fdwReason, LPVOID lpvReserved) { + GIT_UNUSED(hInstDll); + GIT_UNUSED(lpvReserved); + /* This is how Windows lets us know our thread is being shut down */ if (fdwReason == DLL_THREAD_DETACH) { git__free_tls_data(); From 6c133a75755e14ae014fc75c99fa7a0848d5606e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 1 Jun 2016 14:52:25 -0500 Subject: [PATCH 19/67] round-trip trees through index_read_index Read a tree into an index using `git_index_read_index` (by reading a tree into a new index, then reading that index into the current index), then write the index back out, ensuring that our new index is treesame to the tree that we read. --- tests/index/read_index.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/index/read_index.c b/tests/index/read_index.c index 82a771d54..30adc0388 100644 --- a/tests/index/read_index.c +++ b/tests/index/read_index.c @@ -71,3 +71,35 @@ void test_index_read_index__maintains_stat_cache(void) } } } + +static bool roundtrip_with_read_index(const char *tree_idstr) +{ + git_oid tree_id, new_tree_id; + git_tree *tree; + git_index *tree_index; + + cl_git_pass(git_oid_fromstr(&tree_id, tree_idstr)); + cl_git_pass(git_tree_lookup(&tree, _repo, &tree_id)); + cl_git_pass(git_index_new(&tree_index)); + cl_git_pass(git_index_read_tree(tree_index, tree)); + cl_git_pass(git_index_read_index(_index, tree_index)); + cl_git_pass(git_index_write_tree(&new_tree_id, _index)); + + git_tree_free(tree); + git_index_free(tree_index); + + return git_oid_equal(&tree_id, &new_tree_id); +} + +void test_index_read_index__produces_treesame_indexes(void) +{ + roundtrip_with_read_index("53fc32d17276939fc79ed05badaef2db09990016"); + roundtrip_with_read_index("944c0f6e4dfa41595e6eb3ceecdb14f50fe18162"); + roundtrip_with_read_index("1810dff58d8a660512d4832e740f692884338ccd"); + roundtrip_with_read_index("d52a8fe84ceedf260afe4f0287bbfca04a117e83"); + roundtrip_with_read_index("c36d8ea75da8cb510fcb0c408c1d7e53f9a99dbe"); + roundtrip_with_read_index("7b2417a23b63e1fdde88c80e14b33247c6e5785a"); + roundtrip_with_read_index("f82a8eb4cb20e88d1030fd10d89286215a715396"); + roundtrip_with_read_index("fd093bff70906175335656e6ce6ae05783708765"); + roundtrip_with_read_index("ae90f12eea699729ed24555e40b9fd669da12a12"); +} From e6a0a8509151326f98fca3e8d18108e937d9168e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 1 Jun 2016 14:56:27 -0500 Subject: [PATCH 20/67] index_read_index: reset error correctly Clear any error state upon each iteration. If one of the iterations ends (with an error of `GIT_ITEROVER`) we need to reset that error to 0, lest we stop the whole process prematurely. --- src/index.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.c b/src/index.c index 31cb27d6c..135bf9ff5 100644 --- a/src/index.c +++ b/src/index.c @@ -2968,6 +2968,8 @@ int git_index_read_index( *remove_entry = NULL; int diff; + error = 0; + if (old_entry && new_entry) diff = git_index_entry_cmp(old_entry, new_entry); else if (!old_entry && new_entry) From e755f79fd7b95ed58594b75bdf595b07c3b395f8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jun 2016 00:47:51 -0500 Subject: [PATCH 21/67] index_read_index: differentiate on mode Treat index entries with different modes as different, which they are, at least for the purposes of up-to-date calculations. --- src/index.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.c b/src/index.c index 135bf9ff5..b1ee65ffe 100644 --- a/src/index.c +++ b/src/index.c @@ -2987,7 +2987,8 @@ int git_index_read_index( /* Path and stage are equal, if the OID is equal, keep it to * keep the stat cache data. */ - if (git_oid_equal(&old_entry->id, &new_entry->id)) { + if (git_oid_equal(&old_entry->id, &new_entry->id) && + old_entry->mode == new_entry->mode) { add_entry = (git_index_entry *)old_entry; } else { dup_entry = (git_index_entry *)new_entry; From 80745b1256f4cf04f72f90e42a07ce9ac80bfe19 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jun 2016 01:04:58 -0500 Subject: [PATCH 22/67] index_read_index: set flags for path_len correctly Update the flags to reset the path_len (to emulate `index_insert`) --- src/index.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/index.c b/src/index.c index b1ee65ffe..430a34f8e 100644 --- a/src/index.c +++ b/src/index.c @@ -2999,6 +2999,9 @@ int git_index_read_index( if (dup_entry) { if ((error = index_entry_dup_nocache(&add_entry, index, dup_entry)) < 0) goto done; + + index_entry_adjust_namemask(add_entry, + ((struct entry_internal *)add_entry)->pathlen); } if (add_entry) { From 8be49681a68ab77d87b9d90b715314caa455ea7b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 1 Jun 2016 22:31:16 -0500 Subject: [PATCH 23/67] test: ensure we can round-trip a written tree Read a tree into an index, write the index, then re-open the index and ensure that we are treesame to the original. --- tests/index/read_index.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/index/read_index.c b/tests/index/read_index.c index 30adc0388..6d14bc9fd 100644 --- a/tests/index/read_index.c +++ b/tests/index/read_index.c @@ -103,3 +103,26 @@ void test_index_read_index__produces_treesame_indexes(void) roundtrip_with_read_index("fd093bff70906175335656e6ce6ae05783708765"); roundtrip_with_read_index("ae90f12eea699729ed24555e40b9fd669da12a12"); } + +void test_index_read_index__read_and_writes(void) +{ + git_oid tree_id, new_tree_id; + git_tree *tree; + git_index *tree_index, *new_index; + + cl_git_pass(git_oid_fromstr(&tree_id, "ae90f12eea699729ed24555e40b9fd669da12a12")); + cl_git_pass(git_tree_lookup(&tree, _repo, &tree_id)); + cl_git_pass(git_index_new(&tree_index)); + cl_git_pass(git_index_read_tree(tree_index, tree)); + cl_git_pass(git_index_read_index(_index, tree_index)); + cl_git_pass(git_index_write(_index)); + + cl_git_pass(git_index_open(&new_index, git_index_path(_index))); + cl_git_pass(git_index_write_tree_to(&new_tree_id, new_index, _repo)); + + cl_assert_equal_oid(&tree_id, &new_tree_id); + + git_tree_free(tree); + git_index_free(tree_index); + git_index_free(new_index); +} From 11408f0e43ba38829863a992fc73a77c6c18e035 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 2 Jun 2016 02:34:03 -0500 Subject: [PATCH 24/67] index_read_index: invalidate new paths in tree cache When adding a new entry to an existing index via `git_index_read_index`, be sure to remove the tree cache entry for that new path. This will mark all parent trees as dirty. --- src/index.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/index.c b/src/index.c index 430a34f8e..20ab6a19d 100644 --- a/src/index.c +++ b/src/index.c @@ -3004,6 +3004,12 @@ int git_index_read_index( ((struct entry_internal *)add_entry)->pathlen); } + /* invalidate this path in the tree cache if this is new (to + * invalidate the parent trees) + */ + if (dup_entry && !remove_entry && index->tree) + git_tree_cache_invalidate_path(index->tree, dup_entry->path); + if (add_entry) { if ((error = git_vector_insert(&new_entries, add_entry)) == 0) INSERT_IN_MAP_EX(index, new_entries_map, add_entry, error); From 1a7096043645da9f44dad15a51a8034f3e716b5b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 6 Jun 2016 12:59:17 +0200 Subject: [PATCH 25/67] transports: smart: fix potential invalid memory dereferences When we receive a packet of exactly four bytes encoding its length as those four bytes it can be treated as an empty line. While it is not really specified how those empty lines should be treated, we currently ignore them and do not return an error when trying to parse it but simply advance the data pointer. Callers invoking `git_pkt_parse_line` are currently not prepared to handle this case as they do not explicitly check this case. While they could always reset the passed out-pointer to `NULL` before calling `git_pkt_parse_line` and determine if the pointer has been set afterwards, it makes more sense to update `git_pkt_parse_line` to set the out-pointer to `NULL` itself when it encounters such an empty packet. Like this it is guaranteed that there will be no invalid memory references to free'd pointers. As such, the issue has been fixed such that `git_pkt_parse_line` always sets the packet out pointer to `NULL` when an empty packet has been received and callers check for this condition, skipping such packets. --- src/transports/smart_pkt.c | 1 + src/transports/smart_protocol.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 2ea57bb64..2297cc94f 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -433,6 +433,7 @@ int git_pkt_parse_line( * line? */ if (len == PKT_LEN_SIZE) { + *head = NULL; *out = line; return 0; } diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 02e1ecf74..3448fa7fb 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -759,6 +759,14 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt, line_len -= (line_end - line); line = line_end; + /* When a valid packet with no content has been + * read, git_pkt_parse_line does not report an + * error, but the pkt pointer has not been set. + * Handle this by skipping over empty packets. + */ + if (pkt == NULL) + continue; + error = add_push_report_pkt(push, pkt); git_pkt_free(pkt); @@ -813,6 +821,9 @@ static int parse_report(transport_smart *transport, git_push *push) error = 0; + if (pkt == NULL) + continue; + switch (pkt->type) { case GIT_PKT_DATA: /* This is a sideband packet which contains other packets */ From 246d25b3ce218d2d300f60e75528c2fbe83f8db5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Jun 2016 08:35:26 +0200 Subject: [PATCH 26/67] index: fix NULL pointer access in index_remove_entry When removing an entry from the index by its position, we first retrieve the position from the index's entries and then try to remove the retrieved value from the index map with `DELETE_IN_MAP`. When `index_remove_entry` returns `NULL` we try to feed it into the `DELETE_IN_MAP` macro, which will unconditionally call `idxentry_hash` and then happily dereference the `NULL` entry pointer. Fix the issue by not passing a `NULL` entry into `DELETE_IN_MAP`. --- src/index.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/index.c b/src/index.c index 20ab6a19d..32f585faf 100644 --- a/src/index.c +++ b/src/index.c @@ -505,10 +505,11 @@ static int index_remove_entry(git_index *index, size_t pos) int error = 0; git_index_entry *entry = git_vector_get(&index->entries, pos); - if (entry != NULL) + if (entry != NULL) { git_tree_cache_invalidate_path(index->tree, entry->path); + DELETE_IN_MAP(index, entry); + } - DELETE_IN_MAP(index, entry); error = git_vector_remove(&index->entries, pos); if (!error) { From 6e0d473bee5f7dcc142c7d2542d24e5a402611fa Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Jun 2016 12:29:16 +0200 Subject: [PATCH 27/67] tests: fix memory leaks in checkout::typechange --- tests/checkout/typechange.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/checkout/typechange.c b/tests/checkout/typechange.c index 1efea931a..8a5110caa 100644 --- a/tests/checkout/typechange.c +++ b/tests/checkout/typechange.c @@ -240,8 +240,7 @@ static int make_submodule_dirty(git_submodule *sm, const char *name, void *paylo )); git_futils_rmdir_r(git_buf_cstr(&submodulepath), NULL, GIT_RMDIR_REMOVE_FILES); - /* initialize submodule and its repository */ - cl_git_pass(git_submodule_init(sm, 1)); + /* initialize submodule's repository */ cl_git_pass(git_submodule_repo_init(&submodule_repo, sm, 0)); /* create a file in the submodule workdir to make it dirty */ @@ -251,6 +250,7 @@ static int make_submodule_dirty(git_submodule *sm, const char *name, void *paylo git_buf_free(&dirtypath); git_buf_free(&submodulepath); + git_repository_free(submodule_repo); return 0; } From d1fb89dd2f28663192386236a912e7c3d30fa2a2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Jun 2016 12:55:17 +0200 Subject: [PATCH 28/67] global: clean up crt only after freeing tls data The thread local storage is used to hold some global state that is dynamically allocated and should be freed upon exit. On Windows, we clean up the C run-time right after execution of registered shutdown callbacks and before cleaning up the TLS. When we clean up the CRT, we also cause it to analyze for memory leaks. As we did not free the TLS yet this will lead to false positives. Fix the issue by first freeing the TLS and cleaning up the CRT only afterwards. --- src/global.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/global.c b/src/global.c index 967944fa1..603135e07 100644 --- a/src/global.c +++ b/src/global.c @@ -85,11 +85,6 @@ static void shutdown_common(void) git__free(git__user_agent); git__free(git__ssl_ciphers); - -#if defined(GIT_MSVC_CRTDBG) - git_win32__crtdbg_stacktrace_cleanup(); - git_win32__stack_cleanup(); -#endif } /** @@ -181,6 +176,11 @@ int git_libgit2_shutdown(void) TlsFree(_tls_index); git_mutex_free(&git__mwindow_mutex); + +#if defined(GIT_MSVC_CRTDBG) + git_win32__crtdbg_stacktrace_cleanup(); + git_win32__stack_cleanup(); +#endif } /* Exit the lock */ From 27008e849f510622ac880bb818f0e928a9d1bdf6 Mon Sep 17 00:00:00 2001 From: Jason Haslam Date: Tue, 14 Jun 2016 14:46:12 -0600 Subject: [PATCH 29/67] fetch: Fixed spurious update callback for existing tags. --- src/remote.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/remote.c b/src/remote.c index 8b7203ee2..db7564a59 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1414,7 +1414,11 @@ static int update_tips_for_spec( /* In autotag mode, don't overwrite any locally-existing tags */ error = git_reference_create(&ref, remote->repo, refname.ptr, &head->oid, !autotag, log_message); - if (error < 0 && error != GIT_EEXISTS) + + if (error == GIT_EEXISTS) + continue; + + if (error < 0) goto on_error; git_reference_free(ref); From a574d84352fb9d21860f371b1dec23f55b90c873 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 14 Jun 2016 12:27:03 -0500 Subject: [PATCH 30/67] documentation: improve docs for `checkout_head` `git_checkout_head` is sadly misunderstood as something that can switch branches. It cannot. Update the documentation to reflect this. --- include/git2/checkout.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index 6cf9ed8bd..4a9dbb021 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -313,6 +313,13 @@ GIT_EXTERN(int) git_checkout_init_options( * Updates files in the index and the working tree to match the content of * the commit pointed at by HEAD. * + * Note that this is _not_ the correct mechanism used to switch branches; + * do not change your `HEAD` and then call this method, that would leave + * you with checkout conflicts since your working directory would then + * appear to be dirty. Instead, checkout the target of the branch and + * then update `HEAD` using `git_repository_set_head` to point to the + * branch you checked out. + * * @param repo repository to check out (must be non-bare) * @param opts specifies checkout options (may be NULL) * @return 0 on success, GIT_EUNBORNBRANCH if HEAD points to a non From ac44d354c876bc519f5a8dd7f133b2b45735dec1 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 15 Jun 2016 15:47:28 -0500 Subject: [PATCH 31/67] checkout: use empty baseline when no index When no index file exists and a baseline is not explicitly provided, use an empty baseline instead of trying to load `HEAD`. --- src/checkout.c | 7 ++++- tests/checkout/tree.c | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/checkout.c b/src/checkout.c index a40615945..c6d91a938 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -2408,8 +2408,13 @@ static int checkout_data_init( if (!data->opts.baseline && !data->opts.baseline_index) { data->opts_free_baseline = true; + error = 0; - error = checkout_lookup_head_tree(&data->opts.baseline, repo); + /* if we don't have an index, this is an initial checkout and + * should be against an empty baseline + */ + if (data->index->on_disk) + error = checkout_lookup_head_tree(&data->opts.baseline, repo); if (error == GIT_EUNBORNBRANCH) { error = 0; diff --git a/tests/checkout/tree.c b/tests/checkout/tree.c index 5680b86df..7df4d7ef0 100644 --- a/tests/checkout/tree.c +++ b/tests/checkout/tree.c @@ -1416,3 +1416,66 @@ void test_checkout_tree__safe_proceeds_if_no_index(void) git_object_free(obj); } +static int checkout_conflict_count_cb( + git_checkout_notify_t why, + const char *path, + const git_diff_file *b, + const git_diff_file *t, + const git_diff_file *w, + void *payload) +{ + size_t *n = payload; + + GIT_UNUSED(why); + GIT_UNUSED(path); + GIT_UNUSED(b); + GIT_UNUSED(t); + GIT_UNUSED(w); + + (*n)++; + + return 0; +} + +/* A repo that has a HEAD (even a properly born HEAD that peels to + * a commit) but no index should be treated as if it's an empty baseline + */ +void test_checkout_tree__baseline_is_empty_when_no_index(void) +{ + git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; + git_reference *head; + git_object *obj; + git_status_list *status; + size_t conflicts = 0; + + assert_on_branch(g_repo, "master"); + cl_git_pass(git_repository_head(&head, g_repo)); + cl_git_pass(git_reference_peel(&obj, head, GIT_OBJ_COMMIT)); + + cl_git_pass(git_reset(g_repo, obj, GIT_RESET_HARD, NULL)); + + cl_must_pass(p_unlink("testrepo/.git/index")); + + /* for a safe checkout, we should have checkout conflicts with + * the existing untracked files. + */ + opts.checkout_strategy &= ~GIT_CHECKOUT_FORCE; + opts.notify_flags = GIT_CHECKOUT_NOTIFY_CONFLICT; + opts.notify_cb = checkout_conflict_count_cb; + opts.notify_payload = &conflicts; + + cl_git_fail_with(GIT_ECONFLICT, git_checkout_tree(g_repo, obj, &opts)); + cl_assert_equal_i(4, conflicts); + + /* but force should succeed and update the index */ + opts.checkout_strategy |= GIT_CHECKOUT_FORCE; + cl_git_pass(git_checkout_tree(g_repo, obj, &opts)); + + cl_git_pass(git_status_list_new(&status, g_repo, NULL)); + cl_assert_equal_i(0, git_status_list_entrycount(status)); + git_status_list_free(status); + + git_object_free(obj); + git_reference_free(head); +} + From 4c06f3e7dcc0fb7db635ae655ea982b22adb9bd0 Mon Sep 17 00:00:00 2001 From: David Brooks Date: Sun, 19 Jun 2016 11:46:43 +0100 Subject: [PATCH 32/67] HTTP authentication scheme name is case insensitive. --- src/transports/http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/http.c b/src/transports/http.c index 88b124bf7..4bf1d91bf 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -114,7 +114,7 @@ static bool challenge_match(git_http_auth_scheme *scheme, void *data) size_t scheme_len; scheme_len = strlen(scheme_name); - return (strncmp(challenge, scheme_name, scheme_len) == 0 && + return (strncasecmp(challenge, scheme_name, scheme_len) == 0 && (challenge[scheme_len] == '\0' || challenge[scheme_len] == ' ')); } From 286e7dbd4be38b9a72c867da87c77200062c25e5 Mon Sep 17 00:00:00 2001 From: Sim Domingo Date: Thu, 9 Jun 2016 22:50:53 +0800 Subject: [PATCH 33/67] fix error message SHA truncation in git_odb__error_notfound() --- src/odb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odb.c b/src/odb.c index cb0f70623..f396149b3 100644 --- a/src/odb.c +++ b/src/odb.c @@ -1229,7 +1229,7 @@ int git_odb__error_notfound( { if (oid != NULL) { char oid_str[GIT_OID_HEXSZ + 1]; - git_oid_tostr(oid_str, oid_len, oid); + git_oid_tostr(oid_str, oid_len+1, oid); giterr_set(GITERR_ODB, "Object not found - %s (%.*s)", message, oid_len, oid_str); } else From fc2b97dd4ea19768bfa97e9391ebe8f1ab59aef4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 17:44:04 +0200 Subject: [PATCH 34/67] threads: split up OS-dependent thread code --- src/pack-objects.c | 2 +- src/thread-utils.h | 13 ++----------- src/unix/pthread.h | 20 ++++++++++++++++++++ src/win32/pthread.c | 13 +++++-------- src/win32/pthread.h | 22 +++------------------- tests/object/cache.c | 4 ++-- tests/threads/refdb.c | 6 +++--- tests/threads/thread_helpers.c | 2 +- 8 files changed, 37 insertions(+), 45 deletions(-) create mode 100644 src/unix/pthread.h diff --git a/src/pack-objects.c b/src/pack-objects.c index 11e13f7d4..29231e028 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -1186,7 +1186,7 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list, git_mutex_init(&p[i].mutex); git_cond_init(&p[i].cond); - ret = git_thread_create(&p[i].thread, NULL, + ret = git_thread_create(&p[i].thread, threaded_find_deltas, &p[i]); if (ret) { giterr_set(GITERR_THREAD, "unable to create thread"); diff --git a/src/thread-utils.h b/src/thread-utils.h index 14c8a41ff..11c026f33 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -41,16 +41,7 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS #if !defined(GIT_WIN32) - -typedef struct { - pthread_t thread; -} git_thread; - -#define git_thread_create(git_thread_ptr, attr, start_routine, arg) \ - pthread_create(&(git_thread_ptr)->thread, attr, start_routine, arg) -#define git_thread_join(git_thread_ptr, status) \ - pthread_join((git_thread_ptr)->thread, status) - +# include "unix/pthread.h" #endif /* Pthreads Mutex */ @@ -178,7 +169,7 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend) #else #define git_thread unsigned int -#define git_thread_create(thread, attr, start_routine, arg) 0 +#define git_thread_create(thread, start_routine, arg) 0 #define git_thread_join(id, status) (void)0 /* Pthreads Mutex */ diff --git a/src/unix/pthread.h b/src/unix/pthread.h new file mode 100644 index 000000000..87384a4ab --- /dev/null +++ b/src/unix/pthread.h @@ -0,0 +1,20 @@ +/* + * 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_unix_pthread_h__ +#define INCLUDE_unix_pthread_h__ + +typedef struct { + pthread_t thread; +} git_thread; + +#define git_thread_create(git_thread_ptr, start_routine, arg) \ + pthread_create(&(git_thread_ptr)->thread, NULL, start_routine, arg) +#define git_thread_join(git_thread_ptr, status) \ + pthread_join((git_thread_ptr)->thread, status) + +#endif /* INCLUDE_unix_pthread_h__ */ diff --git a/src/win32/pthread.c b/src/win32/pthread.c index a1cc18932..d8ed4bb1b 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -16,7 +16,7 @@ * void pointer. This requires the indirection. */ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) { - git_win32_thread *thread = lpParameter; + git_thread *thread = lpParameter; thread->result = thread->proc(thread->param); @@ -25,14 +25,11 @@ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) return CLEAN_THREAD_EXIT; } -int git_win32__thread_create( - git_win32_thread *GIT_RESTRICT thread, - const pthread_attr_t *GIT_RESTRICT attr, +int git_thread_create( + git_thread *GIT_RESTRICT thread, void *(*start_routine)(void*), void *GIT_RESTRICT arg) { - GIT_UNUSED(attr); - thread->result = NULL; thread->param = arg; thread->proc = start_routine; @@ -42,8 +39,8 @@ int git_win32__thread_create( return thread->thread ? 0 : -1; } -int git_win32__thread_join( - git_win32_thread *thread, +int git_thread_join( + git_thread *thread, void **value_ptr) { DWORD exit; diff --git a/src/win32/pthread.h b/src/win32/pthread.h index e4826ca7f..fa99b01d6 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -21,7 +21,7 @@ typedef struct { void *(*proc)(void *); void *param; void *result; -} git_win32_thread; +} git_thread; typedef int pthread_mutexattr_t; typedef int pthread_condattr_t; @@ -42,26 +42,10 @@ typedef struct { #define PTHREAD_MUTEX_INITIALIZER {(void*)-1} -int git_win32__thread_create( - git_win32_thread *GIT_RESTRICT, - const pthread_attr_t *GIT_RESTRICT, +int git_thread_create(git_thread *GIT_RESTRICT, void *(*) (void *), void *GIT_RESTRICT); - -int git_win32__thread_join( - git_win32_thread *, - void **); - -#ifdef GIT_THREADS - -typedef git_win32_thread git_thread; - -#define git_thread_create(git_thread_ptr, attr, start_routine, arg) \ - git_win32__thread_create(git_thread_ptr, attr, start_routine, arg) -#define git_thread_join(git_thread_ptr, status) \ - git_win32__thread_join(git_thread_ptr, status) - -#endif +int git_thread_join(git_thread *, void **); int pthread_mutex_init( pthread_mutex_t *GIT_RESTRICT mutex, diff --git a/tests/object/cache.c b/tests/object/cache.c index bdf12da7a..680f23630 100644 --- a/tests/object/cache.c +++ b/tests/object/cache.c @@ -220,7 +220,7 @@ void test_object_cache__threadmania(void) fn = (th & 1) ? cache_parsed : cache_raw; #ifdef GIT_THREADS - cl_git_pass(git_thread_create(&t[th], NULL, fn, data)); + cl_git_pass(git_thread_create(&t[th], fn, data)); #else cl_assert(fn(data) == data); git__free(data); @@ -267,7 +267,7 @@ void test_object_cache__fast_thread_rush(void) data[th] = th; #ifdef GIT_THREADS cl_git_pass( - git_thread_create(&t[th], NULL, cache_quick, &data[th])); + git_thread_create(&t[th], cache_quick, &data[th])); #else cl_assert(cache_quick(&data[th]) == &data[th]); #endif diff --git a/tests/threads/refdb.c b/tests/threads/refdb.c index 6589e3922..f869bcb44 100644 --- a/tests/threads/refdb.c +++ b/tests/threads/refdb.c @@ -75,7 +75,7 @@ void test_threads_refdb__iterator(void) for (t = 0; t < THREADS; ++t) { id[t] = t; #ifdef GIT_THREADS - cl_git_pass(git_thread_create(&th[t], NULL, iterate_refs, &id[t])); + cl_git_pass(git_thread_create(&th[t], iterate_refs, &id[t])); #else th[t] = t; iterate_refs(&id[t]); @@ -196,7 +196,7 @@ void test_threads_refdb__edit_while_iterate(void) * for now by just running on a single thread... */ /* #ifdef GIT_THREADS */ -/* cl_git_pass(git_thread_create(&th[t], NULL, fn, &id[t])); */ +/* cl_git_pass(git_thread_create(&th[t], fn, &id[t])); */ /* #else */ fn(&id[t]); /* #endif */ @@ -211,7 +211,7 @@ void test_threads_refdb__edit_while_iterate(void) for (t = 0; t < THREADS; ++t) { id[t] = t; - cl_git_pass(git_thread_create(&th[t], NULL, iterate_refs, &id[t])); + cl_git_pass(git_thread_create(&th[t], iterate_refs, &id[t])); } for (t = 0; t < THREADS; ++t) { diff --git a/tests/threads/thread_helpers.c b/tests/threads/thread_helpers.c index 760a7bd33..54bf6097d 100644 --- a/tests/threads/thread_helpers.c +++ b/tests/threads/thread_helpers.c @@ -24,7 +24,7 @@ void run_in_parallel( for (t = 0; t < threads; ++t) { id[t] = t; #ifdef GIT_THREADS - cl_git_pass(git_thread_create(&th[t], NULL, func, &id[t])); + cl_git_pass(git_thread_create(&th[t], func, &id[t])); #else cl_assert(func(&id[t]) == &id[t]); #endif From 40b243bf5a6c77e01306f447b4bef7642013b9dc Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 17:07:14 +0200 Subject: [PATCH 35/67] threads: split up OS-dependent mutex code --- src/thread-utils.h | 11 +++-------- src/unix/pthread.h | 7 +++++++ src/win32/pthread.c | 17 +++++++---------- src/win32/pthread.h | 14 ++++++-------- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index 11c026f33..29b5d1eee 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -40,17 +40,12 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS -#if !defined(GIT_WIN32) +#ifdef GIT_WIN32 +# include "win32/pthread.h" +#else # include "unix/pthread.h" #endif -/* Pthreads Mutex */ -#define git_mutex pthread_mutex_t -#define git_mutex_init(a) pthread_mutex_init(a, NULL) -#define git_mutex_lock(a) pthread_mutex_lock(a) -#define git_mutex_unlock(a) pthread_mutex_unlock(a) -#define git_mutex_free(a) pthread_mutex_destroy(a) - /* Pthreads condition vars */ #define git_cond pthread_cond_t #define git_cond_init(c) pthread_cond_init(c, NULL) diff --git a/src/unix/pthread.h b/src/unix/pthread.h index 87384a4ab..7487cb5c5 100644 --- a/src/unix/pthread.h +++ b/src/unix/pthread.h @@ -17,4 +17,11 @@ typedef struct { #define git_thread_join(git_thread_ptr, status) \ pthread_join((git_thread_ptr)->thread, status) +/* Git Mutex */ +#define git_mutex pthread_mutex_t +#define git_mutex_init(a) pthread_mutex_init(a, NULL) +#define git_mutex_lock(a) pthread_mutex_lock(a) +#define git_mutex_unlock(a) pthread_mutex_unlock(a) +#define git_mutex_free(a) pthread_mutex_destroy(a) + #endif /* INCLUDE_unix_pthread_h__ */ diff --git a/src/win32/pthread.c b/src/win32/pthread.c index d8ed4bb1b..142c1afdf 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -67,28 +67,25 @@ int git_thread_join( return 0; } -int pthread_mutex_init( - pthread_mutex_t *GIT_RESTRICT mutex, - const pthread_mutexattr_t *GIT_RESTRICT mutexattr) +int git_mutex_init(git_mutex *GIT_RESTRICT mutex) { - GIT_UNUSED(mutexattr); InitializeCriticalSection(mutex); return 0; } -int pthread_mutex_destroy(pthread_mutex_t *mutex) +int git_mutex_free(git_mutex *mutex) { DeleteCriticalSection(mutex); return 0; } -int pthread_mutex_lock(pthread_mutex_t *mutex) +int git_mutex_lock(git_mutex *mutex) { EnterCriticalSection(mutex); return 0; } -int pthread_mutex_unlock(pthread_mutex_t *mutex) +int git_mutex_unlock(git_mutex *mutex) { LeaveCriticalSection(mutex); return 0; @@ -124,7 +121,7 @@ int pthread_cond_destroy(pthread_cond_t *cond) return 0; } -int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) +int pthread_cond_wait(pthread_cond_t *cond, git_mutex *mutex) { int error; DWORD wait_result; @@ -133,7 +130,7 @@ int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) return EINVAL; /* The caller must be holding the mutex. */ - error = pthread_mutex_unlock(mutex); + error = git_mutex_unlock(mutex); if (error) return error; @@ -142,7 +139,7 @@ int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) assert(WAIT_OBJECT_0 == wait_result); GIT_UNUSED(wait_result); - return pthread_mutex_lock(mutex); + return git_mutex_lock(mutex); } int pthread_cond_signal(pthread_cond_t *cond) diff --git a/src/win32/pthread.h b/src/win32/pthread.h index fa99b01d6..3ff95e89b 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -28,7 +28,7 @@ typedef int pthread_condattr_t; typedef int pthread_attr_t; typedef int pthread_rwlockattr_t; -typedef CRITICAL_SECTION pthread_mutex_t; +typedef CRITICAL_SECTION git_mutex; typedef HANDLE pthread_cond_t; typedef struct { void *Ptr; } GIT_SRWLOCK; @@ -47,16 +47,14 @@ int git_thread_create(git_thread *GIT_RESTRICT, void *GIT_RESTRICT); int git_thread_join(git_thread *, void **); -int pthread_mutex_init( - pthread_mutex_t *GIT_RESTRICT mutex, - const pthread_mutexattr_t *GIT_RESTRICT mutexattr); -int pthread_mutex_destroy(pthread_mutex_t *); -int pthread_mutex_lock(pthread_mutex_t *); -int pthread_mutex_unlock(pthread_mutex_t *); +int git_mutex_init(git_mutex *GIT_RESTRICT mutex); +int git_mutex_free(git_mutex *); +int git_mutex_lock(git_mutex *); +int git_mutex_unlock(git_mutex *); int pthread_cond_init(pthread_cond_t *, const pthread_condattr_t *); int pthread_cond_destroy(pthread_cond_t *); -int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *); +int pthread_cond_wait(pthread_cond_t *, git_mutex *); int pthread_cond_signal(pthread_cond_t *); /* pthread_cond_broadcast is not supported on Win32 yet. */ From 1b8253168a3159b6822f1f8d41d1c32cf6d29b42 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 19:48:19 +0200 Subject: [PATCH 36/67] threads: remove unused function pthread_cond_broadcast --- src/thread-utils.h | 1 - src/win32/pthread.c | 4 ---- src/win32/pthread.h | 1 - 3 files changed, 6 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index 29b5d1eee..5073c2a8b 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -52,7 +52,6 @@ typedef git_atomic git_atomic_ssize; #define git_cond_free(c) pthread_cond_destroy(c) #define git_cond_wait(c, l) pthread_cond_wait(c, l) #define git_cond_signal(c) pthread_cond_signal(c) -#define git_cond_broadcast(c) pthread_cond_broadcast(c) /* Pthread (-ish) rwlock * diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 142c1afdf..abf047025 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -156,10 +156,6 @@ int pthread_cond_signal(pthread_cond_t *cond) return 0; } -/* pthread_cond_broadcast is not implemented because doing so with just - * Win32 events is quite complicated, and no caller in libgit2 uses it - * yet. - */ int pthread_num_processors_np(void) { DWORD_PTR p, s; diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 3ff95e89b..9d314c89e 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -56,7 +56,6 @@ int pthread_cond_init(pthread_cond_t *, const pthread_condattr_t *); int pthread_cond_destroy(pthread_cond_t *); int pthread_cond_wait(pthread_cond_t *, git_mutex *); int pthread_cond_signal(pthread_cond_t *); -/* pthread_cond_broadcast is not supported on Win32 yet. */ int pthread_num_processors_np(void); From fabd477125ba15dbe7b8b56018020df9c4967b80 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 17:20:13 +0200 Subject: [PATCH 37/67] threads: split up OS-dependent thread-condition code --- src/thread-utils.h | 7 ------- src/unix/pthread.h | 8 ++++++++ src/win32/pthread.c | 12 ++++-------- src/win32/pthread.h | 10 +++++----- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index 5073c2a8b..1eb51de25 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -46,13 +46,6 @@ typedef git_atomic git_atomic_ssize; # include "unix/pthread.h" #endif -/* Pthreads condition vars */ -#define git_cond pthread_cond_t -#define git_cond_init(c) pthread_cond_init(c, NULL) -#define git_cond_free(c) pthread_cond_destroy(c) -#define git_cond_wait(c, l) pthread_cond_wait(c, l) -#define git_cond_signal(c) pthread_cond_signal(c) - /* Pthread (-ish) rwlock * * This differs from normal pthreads rwlocks in two ways: diff --git a/src/unix/pthread.h b/src/unix/pthread.h index 7487cb5c5..0cba59b9c 100644 --- a/src/unix/pthread.h +++ b/src/unix/pthread.h @@ -24,4 +24,12 @@ typedef struct { #define git_mutex_unlock(a) pthread_mutex_unlock(a) #define git_mutex_free(a) pthread_mutex_destroy(a) +/* Git condition vars */ +#define git_cond pthread_cond_t +#define git_cond_init(c) pthread_cond_init(c, NULL) +#define git_cond_free(c) pthread_cond_destroy(c) +#define git_cond_wait(c, l) pthread_cond_wait(c, l) +#define git_cond_signal(c) pthread_cond_signal(c) +#define git_cond_broadcast(c) pthread_cond_broadcast(c) + #endif /* INCLUDE_unix_pthread_h__ */ diff --git a/src/win32/pthread.c b/src/win32/pthread.c index abf047025..9ce062ab8 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -91,12 +91,8 @@ int git_mutex_unlock(git_mutex *mutex) return 0; } -int pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr) +int git_cond_init(git_cond *cond) { - /* We don't support non-default attributes. */ - if (attr) - return EINVAL; - /* This is an auto-reset event. */ *cond = CreateEventW(NULL, FALSE, FALSE, NULL); assert(*cond); @@ -106,7 +102,7 @@ int pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr) return *cond ? 0 : ENOMEM; } -int pthread_cond_destroy(pthread_cond_t *cond) +int git_cond_free(git_cond *cond) { BOOL closed; @@ -121,7 +117,7 @@ int pthread_cond_destroy(pthread_cond_t *cond) return 0; } -int pthread_cond_wait(pthread_cond_t *cond, git_mutex *mutex) +int git_cond_wait(git_cond *cond, git_mutex *mutex) { int error; DWORD wait_result; @@ -142,7 +138,7 @@ int pthread_cond_wait(pthread_cond_t *cond, git_mutex *mutex) return git_mutex_lock(mutex); } -int pthread_cond_signal(pthread_cond_t *cond) +int git_cond_signal(git_cond *cond) { BOOL signaled; diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 9d314c89e..35cb810e7 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -29,7 +29,7 @@ typedef int pthread_attr_t; typedef int pthread_rwlockattr_t; typedef CRITICAL_SECTION git_mutex; -typedef HANDLE pthread_cond_t; +typedef HANDLE git_cond; typedef struct { void *Ptr; } GIT_SRWLOCK; @@ -52,10 +52,10 @@ int git_mutex_free(git_mutex *); int git_mutex_lock(git_mutex *); int git_mutex_unlock(git_mutex *); -int pthread_cond_init(pthread_cond_t *, const pthread_condattr_t *); -int pthread_cond_destroy(pthread_cond_t *); -int pthread_cond_wait(pthread_cond_t *, git_mutex *); -int pthread_cond_signal(pthread_cond_t *); +int git_cond_init(git_cond *); +int git_cond_free(git_cond *); +int git_cond_wait(git_cond *, git_mutex *); +int git_cond_signal(git_cond *); int pthread_num_processors_np(void); From 68343f26dd312a87a7e6139f541602bd759ea04a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 17:49:47 +0200 Subject: [PATCH 38/67] threads: split up OS-dependent rwlock code --- src/thread-utils.h | 24 ------------------------ src/unix/pthread.h | 18 ++++++++++++++++++ src/win32/pthread.c | 16 ++++++---------- src/win32/pthread.h | 16 +++++++--------- 4 files changed, 31 insertions(+), 43 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index 1eb51de25..f75e44087 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -46,30 +46,6 @@ typedef git_atomic git_atomic_ssize; # include "unix/pthread.h" #endif -/* Pthread (-ish) rwlock - * - * This differs from normal pthreads rwlocks in two ways: - * 1. Separate APIs for releasing read locks and write locks (as - * opposed to the pure POSIX API which only has one unlock fn) - * 2. You should not use recursive read locks (i.e. grabbing a read - * lock in a thread that already holds a read lock) because the - * Windows implementation doesn't support it - */ -#define git_rwlock pthread_rwlock_t -#define git_rwlock_init(a) pthread_rwlock_init(a, NULL) -#define git_rwlock_rdlock(a) pthread_rwlock_rdlock(a) -#define git_rwlock_rdunlock(a) pthread_rwlock_rdunlock(a) -#define git_rwlock_wrlock(a) pthread_rwlock_wrlock(a) -#define git_rwlock_wrunlock(a) pthread_rwlock_wrunlock(a) -#define git_rwlock_free(a) pthread_rwlock_destroy(a) -#define GIT_RWLOCK_STATIC_INIT PTHREAD_RWLOCK_INITIALIZER - -#ifndef GIT_WIN32 -#define pthread_rwlock_rdunlock pthread_rwlock_unlock -#define pthread_rwlock_wrunlock pthread_rwlock_unlock -#endif - - GIT_INLINE(void) git_atomic_set(git_atomic *a, int val) { #if defined(GIT_WIN32) diff --git a/src/unix/pthread.h b/src/unix/pthread.h index 0cba59b9c..773ce22f9 100644 --- a/src/unix/pthread.h +++ b/src/unix/pthread.h @@ -32,4 +32,22 @@ typedef struct { #define git_cond_signal(c) pthread_cond_signal(c) #define git_cond_broadcast(c) pthread_cond_broadcast(c) +/* Pthread (-ish) rwlock + * + * This differs from normal pthreads rwlocks in two ways: + * 1. Separate APIs for releasing read locks and write locks (as + * opposed to the pure POSIX API which only has one unlock fn) + * 2. You should not use recursive read locks (i.e. grabbing a read + * lock in a thread that already holds a read lock) because the + * Windows implementation doesn't support it + */ +#define git_rwlock pthread_rwlock_t +#define git_rwlock_init(a) pthread_rwlock_init(a, NULL) +#define git_rwlock_rdlock(a) pthread_rwlock_rdlock(a) +#define git_rwlock_rdunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_wrlock(a) pthread_rwlock_wrlock(a) +#define git_rwlock_wrunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_free(a) pthread_rwlock_destroy(a) +#define GIT_RWLOCK_STATIC_INIT PTHREAD_RWLOCK_INITIALIZER + #endif /* INCLUDE_unix_pthread_h__ */ diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 9ce062ab8..62a691dba 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -172,12 +172,8 @@ static win32_srwlock_fn win32_srwlock_release_shared; static win32_srwlock_fn win32_srwlock_acquire_exclusive; static win32_srwlock_fn win32_srwlock_release_exclusive; -int pthread_rwlock_init( - pthread_rwlock_t *GIT_RESTRICT lock, - const pthread_rwlockattr_t *GIT_RESTRICT attr) +int git_rwlock_init(git_rwlock *GIT_RESTRICT lock) { - GIT_UNUSED(attr); - if (win32_srwlock_initialize) win32_srwlock_initialize(&lock->native.srwl); else @@ -186,7 +182,7 @@ int pthread_rwlock_init( return 0; } -int pthread_rwlock_rdlock(pthread_rwlock_t *lock) +int git_rwlock_rdlock(git_rwlock *lock) { if (win32_srwlock_acquire_shared) win32_srwlock_acquire_shared(&lock->native.srwl); @@ -196,7 +192,7 @@ int pthread_rwlock_rdlock(pthread_rwlock_t *lock) return 0; } -int pthread_rwlock_rdunlock(pthread_rwlock_t *lock) +int git_rwlock_rdunlock(git_rwlock *lock) { if (win32_srwlock_release_shared) win32_srwlock_release_shared(&lock->native.srwl); @@ -206,7 +202,7 @@ int pthread_rwlock_rdunlock(pthread_rwlock_t *lock) return 0; } -int pthread_rwlock_wrlock(pthread_rwlock_t *lock) +int git_rwlock_wrlock(git_rwlock *lock) { if (win32_srwlock_acquire_exclusive) win32_srwlock_acquire_exclusive(&lock->native.srwl); @@ -216,7 +212,7 @@ int pthread_rwlock_wrlock(pthread_rwlock_t *lock) return 0; } -int pthread_rwlock_wrunlock(pthread_rwlock_t *lock) +int git_rwlock_wrunlock(git_rwlock *lock) { if (win32_srwlock_release_exclusive) win32_srwlock_release_exclusive(&lock->native.srwl); @@ -226,7 +222,7 @@ int pthread_rwlock_wrunlock(pthread_rwlock_t *lock) return 0; } -int pthread_rwlock_destroy(pthread_rwlock_t *lock) +int git_rwlock_free(git_rwlock *lock) { if (!win32_srwlock_initialize) DeleteCriticalSection(&lock->native.csec); diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 35cb810e7..ef9285500 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -38,7 +38,7 @@ typedef struct { GIT_SRWLOCK srwl; CRITICAL_SECTION csec; } native; -} pthread_rwlock_t; +} git_rwlock; #define PTHREAD_MUTEX_INITIALIZER {(void*)-1} @@ -59,14 +59,12 @@ int git_cond_signal(git_cond *); int pthread_num_processors_np(void); -int pthread_rwlock_init( - pthread_rwlock_t *GIT_RESTRICT lock, - const pthread_rwlockattr_t *GIT_RESTRICT attr); -int pthread_rwlock_rdlock(pthread_rwlock_t *); -int pthread_rwlock_rdunlock(pthread_rwlock_t *); -int pthread_rwlock_wrlock(pthread_rwlock_t *); -int pthread_rwlock_wrunlock(pthread_rwlock_t *); -int pthread_rwlock_destroy(pthread_rwlock_t *); +int git_rwlock_init(git_rwlock *GIT_RESTRICT lock); +int git_rwlock_rdlock(git_rwlock *); +int git_rwlock_rdunlock(git_rwlock *); +int git_rwlock_wrlock(git_rwlock *); +int git_rwlock_wrunlock(git_rwlock *); +int git_rwlock_free(git_rwlock *); extern int win32_pthread_initialize(void); From 2aa5c6ff8a0be7893b7b41f3048bf6b7a9c69a49 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 19:40:45 +0200 Subject: [PATCH 39/67] threads: remove unused function pthread_num_processors_np The function pthread_num_processors_np is currently unused and superseded by the function `git_online_cpus`. Remove the function. --- src/win32/pthread.c | 11 ----------- src/win32/pthread.h | 2 -- 2 files changed, 13 deletions(-) diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 62a691dba..80329b2b0 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -152,17 +152,6 @@ int git_cond_signal(git_cond *cond) return 0; } -int pthread_num_processors_np(void) -{ - DWORD_PTR p, s; - int n = 0; - - if (GetProcessAffinityMask(GetCurrentProcess(), &p, &s)) - for (; p; p >>= 1) - n += p&1; - - return n ? n : 1; -} typedef void (WINAPI *win32_srwlock_fn)(GIT_SRWLOCK *); diff --git a/src/win32/pthread.h b/src/win32/pthread.h index ef9285500..821bb64ee 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -57,8 +57,6 @@ int git_cond_free(git_cond *); int git_cond_wait(git_cond *, git_mutex *); int git_cond_signal(git_cond *); -int pthread_num_processors_np(void); - int git_rwlock_init(git_rwlock *GIT_RESTRICT lock); int git_rwlock_rdlock(git_rwlock *); int git_rwlock_rdunlock(git_rwlock *); From 961bdbdfacf4e965216d61fbc9f778c089c2e78a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 18:28:00 +0200 Subject: [PATCH 40/67] threads: remove now-useless typedefs --- src/win32/pthread.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 821bb64ee..977d2dfab 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -23,11 +23,6 @@ typedef struct { void *result; } git_thread; -typedef int pthread_mutexattr_t; -typedef int pthread_condattr_t; -typedef int pthread_attr_t; -typedef int pthread_rwlockattr_t; - typedef CRITICAL_SECTION git_mutex; typedef HANDLE git_cond; @@ -40,8 +35,6 @@ typedef struct { } native; } git_rwlock; -#define PTHREAD_MUTEX_INITIALIZER {(void*)-1} - int git_thread_create(git_thread *GIT_RESTRICT, void *(*) (void *), void *GIT_RESTRICT); From 5d03db81469b871065baecdee2965e2e9d708724 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 18:21:42 +0200 Subject: [PATCH 41/67] win32: rename pthread.{c,h} to thread.{c,h} The old pthread-file did re-implement the pthreads API with exact symbol matching. As the thread-abstraction has now been split up between Unix- and Windows-specific files within the `git_` namespace to avoid symbol-clashes between libgit2 and pthreads, the rewritten wrappers have nothing to do with pthreads anymore. Rename the Windows-specific pthread-files to honor this change. --- src/common.h | 2 +- src/thread-utils.h | 2 +- src/win32/precompiled.h | 2 +- src/win32/{pthread.c => thread.c} | 2 +- src/win32/{pthread.h => thread.h} | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) rename src/win32/{pthread.c => thread.c} (99%) rename src/win32/{pthread.h => thread.h} (92%) diff --git a/src/common.h b/src/common.h index 9abd605cb..51fb9186e 100644 --- a/src/common.h +++ b/src/common.h @@ -45,7 +45,7 @@ # include "win32/error.h" # include "win32/version.h" # ifdef GIT_THREADS -# include "win32/pthread.h" +# include "win32/thread.h" # endif # if defined(GIT_MSVC_CRTDBG) # include "win32/w32_stack.h" diff --git a/src/thread-utils.h b/src/thread-utils.h index f75e44087..f0161989f 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -41,7 +41,7 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS #ifdef GIT_WIN32 -# include "win32/pthread.h" +# include "win32/thread.h" #else # include "unix/pthread.h" #endif diff --git a/src/win32/precompiled.h b/src/win32/precompiled.h index 33ce106d3..10ca0b80c 100644 --- a/src/win32/precompiled.h +++ b/src/win32/precompiled.h @@ -16,7 +16,7 @@ #include #include #ifdef GIT_THREADS - #include "win32/pthread.h" + #include "win32/thread.h" #endif #include "git2.h" diff --git a/src/win32/pthread.c b/src/win32/thread.c similarity index 99% rename from src/win32/pthread.c rename to src/win32/thread.c index 80329b2b0..8222c65fe 100644 --- a/src/win32/pthread.c +++ b/src/win32/thread.c @@ -5,7 +5,7 @@ * a Linking Exception. For full terms see the included COPYING file. */ -#include "pthread.h" +#include "thread.h" #include "../global.h" #define CLEAN_THREAD_EXIT 0x6F012842 diff --git a/src/win32/pthread.h b/src/win32/thread.h similarity index 92% rename from src/win32/pthread.h rename to src/win32/thread.h index 977d2dfab..f5dd41ba3 100644 --- a/src/win32/pthread.h +++ b/src/win32/thread.h @@ -5,8 +5,8 @@ * a Linking Exception. For full terms see the included COPYING file. */ -#ifndef GIT_PTHREAD_H -#define GIT_PTHREAD_H +#ifndef INCLUDE_win32_thread_h__ +#define INCLUDE_win32_thread_h__ #include "../common.h" @@ -59,4 +59,4 @@ int git_rwlock_free(git_rwlock *); extern int win32_pthread_initialize(void); -#endif +#endif /* INCLUDE_win32_thread_h__ */ From bb582f073c7f8356f79b767dc70755a900e54dcd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 20:07:33 +0200 Subject: [PATCH 42/67] threads: add platform-independent thread initialization function --- src/global.c | 2 +- src/unix/pthread.h | 1 + src/win32/thread.c | 57 +++++++++++++++++++++++----------------------- src/win32/thread.h | 4 ++-- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/global.c b/src/global.c index 603135e07..e36912cbe 100644 --- a/src/global.c +++ b/src/global.c @@ -132,7 +132,7 @@ static int synchronized_threads_init(void) _tls_index = TlsAlloc(); - win32_pthread_initialize(); + git_threads_init(); if (git_mutex_init(&git__mwindow_mutex)) return -1; diff --git a/src/unix/pthread.h b/src/unix/pthread.h index 773ce22f9..0f3f17927 100644 --- a/src/unix/pthread.h +++ b/src/unix/pthread.h @@ -12,6 +12,7 @@ typedef struct { pthread_t thread; } git_thread; +#define git_threads_init() (void)0 #define git_thread_create(git_thread_ptr, start_routine, arg) \ pthread_create(&(git_thread_ptr)->thread, NULL, start_routine, arg) #define git_thread_join(git_thread_ptr, status) \ diff --git a/src/win32/thread.c b/src/win32/thread.c index 8222c65fe..80d56ce5d 100644 --- a/src/win32/thread.c +++ b/src/win32/thread.c @@ -10,6 +10,14 @@ #define CLEAN_THREAD_EXIT 0x6F012842 +typedef void (WINAPI *win32_srwlock_fn)(GIT_SRWLOCK *); + +static win32_srwlock_fn win32_srwlock_initialize; +static win32_srwlock_fn win32_srwlock_acquire_shared; +static win32_srwlock_fn win32_srwlock_release_shared; +static win32_srwlock_fn win32_srwlock_acquire_exclusive; +static win32_srwlock_fn win32_srwlock_release_exclusive; + /* The thread procedure stub used to invoke the caller's procedure * and capture the return value for later collection. Windows will * only hold a DWORD, but we need to be able to store an entire @@ -25,6 +33,26 @@ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) return CLEAN_THREAD_EXIT; } +int git_threads_init(void) +{ + HMODULE hModule = GetModuleHandleW(L"kernel32"); + + if (hModule) { + win32_srwlock_initialize = (win32_srwlock_fn) + GetProcAddress(hModule, "InitializeSRWLock"); + win32_srwlock_acquire_shared = (win32_srwlock_fn) + GetProcAddress(hModule, "AcquireSRWLockShared"); + win32_srwlock_release_shared = (win32_srwlock_fn) + GetProcAddress(hModule, "ReleaseSRWLockShared"); + win32_srwlock_acquire_exclusive = (win32_srwlock_fn) + GetProcAddress(hModule, "AcquireSRWLockExclusive"); + win32_srwlock_release_exclusive = (win32_srwlock_fn) + GetProcAddress(hModule, "ReleaseSRWLockExclusive"); + } + + return 0; +} + int git_thread_create( git_thread *GIT_RESTRICT thread, void *(*start_routine)(void*), @@ -152,15 +180,6 @@ int git_cond_signal(git_cond *cond) return 0; } - -typedef void (WINAPI *win32_srwlock_fn)(GIT_SRWLOCK *); - -static win32_srwlock_fn win32_srwlock_initialize; -static win32_srwlock_fn win32_srwlock_acquire_shared; -static win32_srwlock_fn win32_srwlock_release_shared; -static win32_srwlock_fn win32_srwlock_acquire_exclusive; -static win32_srwlock_fn win32_srwlock_release_exclusive; - int git_rwlock_init(git_rwlock *GIT_RESTRICT lock) { if (win32_srwlock_initialize) @@ -218,23 +237,3 @@ int git_rwlock_free(git_rwlock *lock) git__memzero(lock, sizeof(*lock)); return 0; } - -int win32_pthread_initialize(void) -{ - HMODULE hModule = GetModuleHandleW(L"kernel32"); - - if (hModule) { - win32_srwlock_initialize = (win32_srwlock_fn) - GetProcAddress(hModule, "InitializeSRWLock"); - win32_srwlock_acquire_shared = (win32_srwlock_fn) - GetProcAddress(hModule, "AcquireSRWLockShared"); - win32_srwlock_release_shared = (win32_srwlock_fn) - GetProcAddress(hModule, "ReleaseSRWLockShared"); - win32_srwlock_acquire_exclusive = (win32_srwlock_fn) - GetProcAddress(hModule, "AcquireSRWLockExclusive"); - win32_srwlock_release_exclusive = (win32_srwlock_fn) - GetProcAddress(hModule, "ReleaseSRWLockExclusive"); - } - - return 0; -} diff --git a/src/win32/thread.h b/src/win32/thread.h index f5dd41ba3..0d01822a6 100644 --- a/src/win32/thread.h +++ b/src/win32/thread.h @@ -35,6 +35,8 @@ typedef struct { } native; } git_rwlock; +int git_threads_init(void); + int git_thread_create(git_thread *GIT_RESTRICT, void *(*) (void *), void *GIT_RESTRICT); @@ -57,6 +59,4 @@ int git_rwlock_wrlock(git_rwlock *); int git_rwlock_wrunlock(git_rwlock *); int git_rwlock_free(git_rwlock *); -extern int win32_pthread_initialize(void); - #endif /* INCLUDE_win32_thread_h__ */ From 90ec160586c79d91433dab77570a82b2f0102c13 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 20 Jun 2016 14:16:50 -0400 Subject: [PATCH 43/67] README: disambiguate what to distribute source of Indicate that if you make changes to libgit2 that you must distribute the source _to libgit2_, not the source _of your program_. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8ea787b3e..5aa8ccb03 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ License `libgit2` is under GPL2 **with linking exception**. This means you can link to and use the library from any program, proprietary or open source; paid or -gratis. However, you cannot modify libgit2 and distribute it without -supplying the source. +gratis. However, if you modify libgit2 itself, you must distribute the +source to your modified version of libgit2. See the [COPYING file](COPYING) for the full license text. From ce124fc79339efc5702dd6ddc4b291198757f9d5 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 20 Jun 2016 14:24:17 -0400 Subject: [PATCH 44/67] README: improve contributing paragraph --- README.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5aa8ccb03..9d5602244 100644 --- a/README.md +++ b/README.md @@ -235,9 +235,15 @@ we can add it to the list. How Can I Contribute? ================================== -Check the [contribution guidelines](CONTRIBUTING.md) to understand our -workflow, the libgit2 [coding conventions](CONVENTIONS.md), and our list of -[good starting projects](PROJECTS.md). +We welcome new contributors! We have a number of issues marked as +["up for grabs"](https://github.com/libgit2/libgit2/issues?q=is%3Aissue+is%3Aopen+label%3A%22up+for+grabs%22) +and +["easy fix"](https://github.com/libgit2/libgit2/issues?utf8=✓&q=is%3Aissue+is%3Aopen+label%3A%22easy+fix%22) +that are good places to jump in and get started. There's much more detailed +information in our list of [outstanding projects](PROJECTS.md). + +Please be sure to check the [contribution guidelines](CONTRIBUTING.md) to +understand our workflow, and the libgit2 [coding conventions](CONVENTIONS.md). License ================================== From cc43d185e319cf7ce9946574f16611cab1d6d1e0 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 20 Jun 2016 15:05:02 -0400 Subject: [PATCH 45/67] README: update "Getting Help" section --- README.md | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 9d5602244..b69a18a38 100644 --- a/README.md +++ b/README.md @@ -15,18 +15,30 @@ with any kind of software without having to release its source code. Additionally, the example code has been released to the public domain (see the [separate license](examples/COPYING) for more information). -* Website: [libgit2.github.com](http://libgit2.github.com) -* StackOverflow Tag: [libgit2](http://stackoverflow.com/questions/tagged/libgit2) -* Issues: [GitHub Issues](https://github.com/libgit2/libgit2/issues) (Right here!) -* API documentation: -* IRC: [#libgit2](irc://irc.freenode.net/libgit2) on irc.freenode.net. -* Mailing list: The libgit2 mailing list was - traditionally hosted in Librelist but has been deprecated. We encourage you to - [use StackOverflow](http://stackoverflow.com/questions/tagged/libgit2) instead for any questions regarding - the library, or [open an issue](https://github.com/libgit2/libgit2/issues) - on GitHub for bug reports. The mailing list archives are still available at - . +Getting Help +============ +**Join us on Slack** + +Visit [slack.libgit2.org](http://slack.libgit2.org/) to sign up, then join +us in `#libgit2`. If you prefer IRC, you can also point your client to our +slack channel once you've registered. + +**Getting Help** + +If you have questions about the library, please be sure to check out the +[API documentation](http://libgit2.github.com/libgit2/). If you still have +questions, reach out to us on Slack or post a question on +[StackOverflow](http://stackoverflow.com/questions/tagged/libgit2) (with the `libgit2` tag). + +**Reporting Bugs** + +Please open a [GitHub Issue](https://github.com/libgit2/libgit2/issues) and +include as much information as possible. If possible, provide sample code +that illustrates the problem you're seeing. If you're seeing a bug only +on a specific repository, please provide a link to it if possible. + +We ask that you not open a GitHub Issue for help, only for bug reports. What It Can Do ============== From c7a033690e5ec8469f296103f35278602033363b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 20 Jun 2016 11:09:49 +0200 Subject: [PATCH 46/67] cmake: do not use -fPIC for MSYS2 The MSYS2 build system automatically compiles all code with position-independent code. When we manually add the -fPIC flag to the compiler flags, MSYS2 will loudly complain about PIC being the default and thus not required. Fix the annoyance by stripping -fPIC in MSYS2 enviroments like it is already done for MinGW. --- CMakeLists.txt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fd683073b..7baf68b5d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -473,19 +473,21 @@ ELSE () SET(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -D_DEBUG") ENDIF () - IF (MINGW) # MinGW always does PIC and complains if we tell it to + IF (MINGW OR MSYS) # MinGW and MSYS always do PIC and complain if we tell them to STRING(REGEX REPLACE "-fPIC" "" CMAKE_SHARED_LIBRARY_C_FLAGS "${CMAKE_SHARED_LIBRARY_C_FLAGS}") - # MinGW >= 3.14 uses the C99-style stdio functions - # automatically, but forks like mingw-w64 still want - # us to define this in order to use them - ADD_DEFINITIONS(-D__USE_MINGW_ANSI_STDIO=1) - ELSEIF (BUILD_SHARED_LIBS) ADD_C_FLAG_IF_SUPPORTED(-fvisibility=hidden) SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") ENDIF () + IF (MINGW) + # MinGW >= 3.14 uses the C99-style stdio functions + # automatically, but forks like mingw-w64 still want + # us to define this in order to use them + ADD_DEFINITIONS(-D__USE_MINGW_ANSI_STDIO=1) + ENDIF () + ADD_C_FLAG_IF_SUPPORTED(-Wdocumentation) ADD_C_FLAG_IF_SUPPORTED(-Wno-missing-field-initializers) ADD_C_FLAG_IF_SUPPORTED(-Wstrict-aliasing=2) From a200dc9e1d35d44aabda468966933f34dc7b702e Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sun, 3 Apr 2016 19:24:15 -0700 Subject: [PATCH 47/67] Fix repository discovery with ceiling_dirs at current directory git only checks ceiling directories when its search ascends to a parent directory. A ceiling directory matching the starting directory will not prevent git from finding a repository in the starting directory or a parent directory. libgit2 handled the former case correctly, but differed from git in the latter case: given a ceiling directory matching the starting directory, but no repository at the starting directory, libgit2 would stop the search at that point rather than finding a repository in a parent directory. Test case using git command-line tools: /tmp$ git init x Initialized empty Git repository in /tmp/x/.git/ /tmp$ cd x/ /tmp/x$ mkdir subdir /tmp/x$ cd subdir/ /tmp/x/subdir$ GIT_CEILING_DIRECTORIES=/tmp/x git rev-parse --git-dir fatal: Not a git repository (or any of the parent directories): .git /tmp/x/subdir$ GIT_CEILING_DIRECTORIES=/tmp/x/subdir git rev-parse --git-dir /tmp/x/.git Fix the testsuite to test this case (in one case fixing a test that depended on the current behavior), and then fix find_repo to handle this case correctly. In the process, simplify and document the logic in find_repo(): - Separate the concepts of "currently checking a .git directory" and "number of iterations left before going further counts as a search" into two separate variables, in_dot_git and min_iterations. - Move the logic to handle in_dot_git and append /.git to the top of the loop. - Only search ceiling_dirs and find ceiling_offset after running out of min_iterations; since ceiling_offset only tracks the longest matching ceiling directory, if ceiling_dirs contained both the current directory and a parent directory, this change makes find_repo stop the search at the parent directory. --- src/repository.c | 42 +++++++++++++++++++++++++----------------- tests/repo/discover.c | 12 +++++++++++- tests/repo/open.c | 3 ++- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/repository.c b/src/repository.c index 8a6fef0f6..43dcfda6f 100644 --- a/src/repository.c +++ b/src/repository.c @@ -359,7 +359,8 @@ static int find_repo( git_buf path = GIT_BUF_INIT; struct stat st; dev_t initial_device = 0; - bool try_with_dot_git = ((flags & GIT_REPOSITORY_OPEN_BARE) != 0); + int min_iterations; + bool in_dot_git; int ceiling_offset; git_buf_free(repo_path); @@ -367,13 +368,27 @@ static int find_repo( if ((error = git_path_prettify(&path, start_path, NULL)) < 0) return error; - ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs); + /* in_dot_git toggles each loop: + * /a/b/c/.git, /a/b/c, /a/b/.git, /a/b, /a/.git, /a + * With GIT_REPOSITORY_OPEN_BARE, we assume we started with /a/b/c.git + * and don't append .git the first time through. + * min_iterations indicates the number of iterations left before going + * further counts as a search. */ + if (flags & GIT_REPOSITORY_OPEN_BARE) { + in_dot_git = true; + min_iterations = 1; + } else { + in_dot_git = false; + min_iterations = 2; + } - if (!try_with_dot_git && - (error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0) - return error; + while (!error && (min_iterations || !(path.ptr[ceiling_offset] == 0 || + (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))) { + if (!in_dot_git) + if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0) + break; + in_dot_git = !in_dot_git; - while (!error && !git_buf_len(repo_path)) { if (p_stat(path.ptr, &st) == 0) { /* check that we have not crossed device boundaries */ if (initial_device == 0) @@ -414,17 +429,10 @@ static int find_repo( break; } - if (try_with_dot_git) { - /* if we tried original dir with and without .git AND either hit - * directory ceiling or NO_SEARCH was requested, then be done. - */ - if (path.ptr[ceiling_offset] == '\0' || - (flags & GIT_REPOSITORY_OPEN_NO_SEARCH) != 0) - break; - /* otherwise look first for .git item */ - error = git_buf_joinpath(&path, path.ptr, DOT_GIT); - } - try_with_dot_git = !try_with_dot_git; + /* Once we've checked the directory (and .git if applicable), + * find the ceiling for a search. */ + if (min_iterations && (--min_iterations == 0)) + ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs); } if (!error && parent_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) { diff --git a/tests/repo/discover.c b/tests/repo/discover.c index 86bd7458f..358daee9f 100644 --- a/tests/repo/discover.c +++ b/tests/repo/discover.c @@ -118,12 +118,22 @@ void test_repo_discover__0(void) cl_git_fail(git_repository_discover(&found_path, ALTERNATE_MALFORMED_FOLDER3, 0, ceiling_dirs)); cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, ALTERNATE_NOT_FOUND_FOLDER, 0, ceiling_dirs)); + append_ceiling_dir(&ceiling_dirs_buf, SUB_REPOSITORY_FOLDER_SUB); + ceiling_dirs = git_buf_cstr(&ceiling_dirs_buf); + + /* this must pass as ceiling_directories cannot prevent the current + * working directory to be checked */ + ensure_repository_discover(SUB_REPOSITORY_FOLDER, ceiling_dirs, &sub_repository_path); + ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB, ceiling_dirs, &sub_repository_path); + cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB, 0, ceiling_dirs)); + cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, 0, ceiling_dirs)); + append_ceiling_dir(&ceiling_dirs_buf, SUB_REPOSITORY_FOLDER); ceiling_dirs = git_buf_cstr(&ceiling_dirs_buf); //this must pass as ceiling_directories cannot predent the current //working directory to be checked - cl_git_pass(git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER, 0, ceiling_dirs)); + ensure_repository_discover(SUB_REPOSITORY_FOLDER, ceiling_dirs, &sub_repository_path); cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB, 0, ceiling_dirs)); cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB, 0, ceiling_dirs)); cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, 0, ceiling_dirs)); diff --git a/tests/repo/open.c b/tests/repo/open.c index d3d087231..7cdd182a7 100644 --- a/tests/repo/open.c +++ b/tests/repo/open.c @@ -196,8 +196,9 @@ void test_repo_open__failures(void) &repo, "attr/sub", GIT_REPOSITORY_OPEN_NO_SEARCH, NULL)); /* fail with ceiling too low */ - cl_git_pass(git_buf_joinpath(&ceiling, ceiling.ptr, "sub")); cl_git_fail(git_repository_open_ext(&repo, "attr/sub", 0, ceiling.ptr)); + cl_git_pass(git_buf_joinpath(&ceiling, ceiling.ptr, "sub")); + cl_git_fail(git_repository_open_ext(&repo, "attr/sub/sub", 0, ceiling.ptr)); /* fail with no repo */ cl_git_pass(p_mkdir("alternate", 0777)); From 1edbfa1ffe20d9581194c0c2e3b660dd0487870f Mon Sep 17 00:00:00 2001 From: Krishna Ram Prakash R Date: Tue, 28 Jun 2016 20:19:52 +0530 Subject: [PATCH 48/67] Fixed bug while parsing INT64_MIN --- src/util.c | 6 +++--- tests/core/strtol.c | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/util.c b/src/util.c index 9e67f4347..cc4b43241 100644 --- a/src/util.c +++ b/src/util.c @@ -122,8 +122,8 @@ int git__strtol64(int64_t *result, const char *nptr, const char **endptr, int ba v = c - 'A' + 10; if (v >= base) break; - nn = n*base + v; - if (nn < n) + nn = n * base + (neg ? -v : v); + if ((!neg && nn < n) || (neg && nn > n)) ovfl = 1; n = nn; } @@ -142,7 +142,7 @@ Return: return -1; } - *result = neg ? -n : n; + *result = n; return 0; } diff --git a/tests/core/strtol.c b/tests/core/strtol.c index 8765e042b..0d3b6a5e6 100644 --- a/tests/core/strtol.c +++ b/tests/core/strtol.c @@ -33,5 +33,13 @@ void test_core_strtol__int64(void) cl_assert(i == 2147483657LL); cl_git_pass(git__strtol64(&i, " -2147483657 ", NULL, 10)); cl_assert(i == -2147483657LL); + cl_git_pass(git__strtol64(&i, " 9223372036854775807 ", NULL, 10)); + cl_assert(i == INT64_MAX); + cl_git_pass(git__strtol64(&i, " -9223372036854775808 ", NULL, 10)); + cl_assert(i == INT64_MIN); + cl_git_pass(git__strtol64(&i, " 0x7fffffffffffffff ", NULL, 16)); + cl_assert(i == INT64_MAX); + cl_git_pass(git__strtol64(&i, " -0x8000000000000000 ", NULL, 16)); + cl_assert(i == INT64_MIN); } From 49188d2b2986daf8e4a93c9d1acd177b98e304ec Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 27 Jun 2016 15:20:20 +0200 Subject: [PATCH 49/67] blame: do not decrement commit refcount in make_origin When we create a blame origin, we try to look up the blob that is to be blamed at a certain revision. When this lookup fails, e.g. because the file did not exist at that certain revision, we fail to create the blame origin and return `NULL`. The blame origin that we have just allocated is thereby free'd with `origin_decref`. The `origin_decref` function does not only decrement reference counts for the blame origin, though, but also for its commit and blob. When this is done in the error case, we will cause an uneven reference count for these objects. This may result in hard-to-debug failures at seemingly unrelated code paths, where we try to access these objects when they in fact have already been free'd. Fix the issue by refactoring `make_origin` such that we only allocate the object after the only function that may fail so that we do not have to call `origin_decref` at all. Also fix the `pass_blame` function, which indirectly calls `make_origin`, to free the commit when `make_origin` failed. --- src/blame_git.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/blame_git.c b/src/blame_git.c index 700207edb..96785c75b 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -37,25 +37,27 @@ static void origin_decref(git_blame__origin *o) static int make_origin(git_blame__origin **out, git_commit *commit, const char *path) { git_blame__origin *o; + git_object *blob; size_t path_len = strlen(path), alloc_len; int error = 0; + if ((error = git_object_lookup_bypath(&blob, (git_object*)commit, + path, GIT_OBJ_BLOB)) < 0) + return error; + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*o), path_len); GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1); o = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(o); o->commit = commit; + o->blob = (git_blob *) blob; o->refcnt = 1; strcpy(o->path, path); - if (!(error = git_object_lookup_bypath((git_object**)&o->blob, (git_object*)commit, - path, GIT_OBJ_BLOB))) { - *out = o; - } else { - origin_decref(o); - } - return error; + *out = o; + + return 0; } /* Locate an existing origin or create a new one. */ @@ -529,8 +531,16 @@ static int pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt) goto finish; porigin = find_origin(blame, p, origin); - if (!porigin) + if (!porigin) { + /* + * We only have to decrement the parent's + * reference count when no porigin has + * been created, as otherwise the commit + * is assigned to the created object. + */ + git_commit_free(p); continue; + } if (porigin->blob && origin->blob && !git_oid_cmp(git_blob_id(porigin->blob), git_blob_id(origin->blob))) { pass_whole_blame(blame, origin, porigin); From 9894c7ddeb204d0568e1173f0f9bb48580ece010 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 15 Jul 2016 13:32:23 -0400 Subject: [PATCH 50/67] remote: Handle missing config values when deleting a remote Somehow I ended up with the following in my ~/.gitconfig: [branch "master"] remote = origin merge = master rebase = true I assume something went crazy while I was running the git.git tests some time ago, and that I never noticed until now. This is not a good configuration, but it shouldn't cause problems. But it does. Specifically, if you have this in your config, and you perform the following set of actions: create a remote fetch from that remote create a branch off of the remote master branch called "master" delete the branch delete the remote The remote delete fails with the message "Could not find key 'branch.master.rebase' to delete". This is because it's iterating over the config entries (including the ones in the global config) and believes that there is a master branch which must therefore have these config keys. https://github.com/libgit2/libgit2/issues/3856 --- src/remote.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/remote.c b/src/remote.c index db7564a59..15a00dd28 100644 --- a/src/remote.c +++ b/src/remote.c @@ -2228,15 +2228,21 @@ static int remove_branch_config_related_entries( if (git_buf_printf(&buf, "branch.%.*s.merge", (int)branch_len, branch) < 0) break; - if ((error = git_config_delete_entry(config, git_buf_cstr(&buf))) < 0) - break; + if ((error = git_config_delete_entry(config, git_buf_cstr(&buf))) < 0) { + if (error != GIT_ENOTFOUND) + break; + giterr_clear(); + } git_buf_clear(&buf); if (git_buf_printf(&buf, "branch.%.*s.remote", (int)branch_len, branch) < 0) break; - if ((error = git_config_delete_entry(config, git_buf_cstr(&buf))) < 0) - break; + if ((error = git_config_delete_entry(config, git_buf_cstr(&buf))) < 0) { + if (error != GIT_ENOTFOUND) + break; + giterr_clear(); + } } if (error == GIT_ITEROVER) From d711165d03d785c1d8a8a5cf390a4dd0a6a0a107 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 22 Jul 2016 14:02:00 -0400 Subject: [PATCH 51/67] repository: don't cast to `int` for no reason And give it a default so that some compilers don't (unnecessarily) complain. --- src/repository.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/repository.c b/src/repository.c index 43dcfda6f..c19790a8c 100644 --- a/src/repository.c +++ b/src/repository.c @@ -264,7 +264,7 @@ cleanup: * the stack could remove directories name limits, but at the cost of doing * repeated malloc/frees inside the loop below, so let's not do it now. */ -static int find_ceiling_dir_offset( +static size_t find_ceiling_dir_offset( const char *path, const char *ceiling_directories) { @@ -278,7 +278,7 @@ static int find_ceiling_dir_offset( min_len = (size_t)(git_path_root(path) + 1); if (ceiling_directories == NULL || min_len == 0) - return (int)min_len; + return min_len; for (sep = ceil = ceiling_directories; *sep; ceil = sep + 1) { for (sep = ceil; *sep && *sep != GIT_PATH_LIST_SEPARATOR; sep++); @@ -305,7 +305,7 @@ static int find_ceiling_dir_offset( } } - return (int)(max_len <= min_len ? min_len : max_len); + return (max_len <= min_len ? min_len : max_len); } /* @@ -361,7 +361,7 @@ static int find_repo( dev_t initial_device = 0; int min_iterations; bool in_dot_git; - int ceiling_offset; + size_t ceiling_offset = 0; git_buf_free(repo_path); From 85addddf4c903c83d3f6851c4dfebbe5b9d10376 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Aug 2016 13:45:28 +0200 Subject: [PATCH 52/67] refspec: do not set empty rhs for fetch refspecs According to git-fetch(1), "[t]he colon can be omitted when is empty." So according to git, the refspec "refs/heads/master" is the same as the refspec "refs/heads/master:" when fetching changes. When trying to fetch from a remote with a trailing colon with libgit2, though, the fetch actually fails while it works when the trailing colon is left out. So obviously, libgit2 does _not_ treat these two refspec formats the same for fetches. The problem results from parsing refspecs, where the resulting refspec has its destination set to an empty string in the case of a trailing colon and to a `NULL` pointer in the case of no trailing colon. When passing this to our DWIM machinery, the empty string gets translated to "refs/heads/", which is simply wrong. Fix the problem by having the parsing machinery treat both cases the same for fetch refspecs. --- src/refspec.c | 6 +++-- tests/online/fetchhead.c | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/refspec.c b/src/refspec.c index debde8692..d200e5609 100644 --- a/src/refspec.c +++ b/src/refspec.c @@ -53,8 +53,10 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch) if (rhs) { size_t rlen = strlen(++rhs); - is_glob = (1 <= rlen && strchr(rhs, '*')); - refspec->dst = git__strndup(rhs, rlen); + if (rlen || !is_fetch) { + is_glob = (1 <= rlen && strchr(rhs, '*')); + refspec->dst = git__strndup(rhs, rlen); + } } llen = (rhs ? (size_t)(rhs - lhs - 1) : strlen(lhs)); diff --git a/tests/online/fetchhead.c b/tests/online/fetchhead.c index 200edacfd..9aaad253c 100644 --- a/tests/online/fetchhead.c +++ b/tests/online/fetchhead.c @@ -35,6 +35,19 @@ static void fetchhead_test_clone(void) cl_git_pass(git_clone(&g_repo, LIVE_REPO_URL, "./foo", &g_options)); } +static int count_references(void) +{ + git_strarray array; + int refs; + + cl_git_pass(git_reference_list(&array, g_repo)); + refs = array.count; + + git_strarray_free(&array); + + return refs; +} + static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fetchhead) { git_remote *remote; @@ -101,3 +114,41 @@ void test_online_fetchhead__no_merges(void) cl_git_pass(git_tag_delete(g_repo, "commit_tree")); fetchhead_test_fetch(NULL, FETCH_HEAD_NO_MERGE_DATA3); } + +void test_online_fetchhead__explicit_dst_refspec_creates_branch(void) +{ + git_reference *ref; + int refs; + + fetchhead_test_clone(); + refs = count_references(); + fetchhead_test_fetch("refs/heads/first-merge:refs/heads/explicit-refspec", FETCH_HEAD_EXPLICIT_DATA); + + cl_git_pass(git_branch_lookup(&ref, g_repo, "explicit-refspec", GIT_BRANCH_ALL)); + cl_assert_equal_i(refs + 1, count_references()); +} + +void test_online_fetchhead__empty_dst_refspec_creates_no_branch(void) +{ + git_reference *ref; + int refs; + + fetchhead_test_clone(); + refs = count_references(); + + fetchhead_test_fetch("refs/heads/first-merge", FETCH_HEAD_EXPLICIT_DATA); + cl_git_fail(git_branch_lookup(&ref, g_repo, "first-merge", GIT_BRANCH_ALL)); + + cl_assert_equal_i(refs, count_references()); +} + +void test_online_fetchhead__colon_only_dst_refspec_creates_no_branch(void) +{ + int refs; + + fetchhead_test_clone(); + refs = count_references(); + fetchhead_test_fetch("refs/heads/first-merge:", FETCH_HEAD_EXPLICIT_DATA); + + cl_assert_equal_i(refs, count_references()); +} From 1fafead53a57d8951e21907cd6bc0d9b8c7a6ffd Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 29 Jul 2016 12:59:42 -0400 Subject: [PATCH 53/67] sysdir: use the standard `init` pattern Don't try to determine when sysdirs are uninitialized. Instead, simply initialize them all at `git_libgit2_init` time and never try to reinitialize, except when consumers explicitly call `git_sysdir_set`. Looking at the buffer length is especially problematic, since there may no appropriate path for that value. (For example, the Windows-specific programdata directory has no value on non-Windows machines.) Previously we would continually trying to re-lookup these values, which could get racy if two different threads are each calling `git_sysdir_get` and trying to lookup / clear the value simultaneously. --- src/sysdir.c | 90 ++++++++++++++++++++++++---------------------------- src/sysdir.h | 5 --- 2 files changed, 42 insertions(+), 53 deletions(-) diff --git a/src/sysdir.c b/src/sysdir.c index bf53d830f..29e53e239 100644 --- a/src/sysdir.c +++ b/src/sysdir.c @@ -83,45 +83,43 @@ static int git_sysdir_guess_template_dirs(git_buf *out) #endif } -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 }; - -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, +struct git_sysdir__dir { + git_buf buf; + int (*guess)(git_buf *out); }; -static int git_sysdir__dirs_shutdown_set = 0; +static struct git_sysdir__dir git_sysdir__dirs[] = { + { GIT_BUF_INIT, git_sysdir_guess_system_dirs }, + { GIT_BUF_INIT, git_sysdir_guess_global_dirs }, + { GIT_BUF_INIT, git_sysdir_guess_xdg_dirs }, + { GIT_BUF_INIT, git_sysdir_guess_programdata_dirs }, + { GIT_BUF_INIT, git_sysdir_guess_template_dirs }, +}; + +static void git_sysdir_global_shutdown(void) +{ + size_t i; + + for (i = 0; i < ARRAY_SIZE(git_sysdir__dirs); ++i) + git_buf_free(&git_sysdir__dirs[i].buf); +} int git_sysdir_global_init(void) { - git_sysdir_t i; - const git_buf *path; + size_t i; int error = 0; - for (i = 0; !error && i < GIT_SYSDIR__MAX; i++) - error = git_sysdir_get(&path, i); + for (i = 0; !error && i < ARRAY_SIZE(git_sysdir__dirs); i++) + error = git_sysdir__dirs[i].guess(&git_sysdir__dirs[i].buf); + + git__on_shutdown(git_sysdir_global_shutdown); return error; } -void git_sysdir_global_shutdown(void) -{ - int i; - for (i = 0; i < GIT_SYSDIR__MAX; ++i) - git_buf_free(&git_sysdir__dirs[i]); - - git_sysdir__dirs_shutdown_set = 0; -} - static int git_sysdir_check_selector(git_sysdir_t which) { - if (which < GIT_SYSDIR__MAX) + if (which < ARRAY_SIZE(git_sysdir__dirs)) return 0; giterr_set(GITERR_INVALID, "config directory selector out of range"); @@ -137,18 +135,7 @@ int git_sysdir_get(const git_buf **out, git_sysdir_t which) GITERR_CHECK_ERROR(git_sysdir_check_selector(which)); - if (!git_buf_len(&git_sysdir__dirs[which])) { - /* prepare shutdown if we're going to need it */ - if (!git_sysdir__dirs_shutdown_set) { - git__on_shutdown(git_sysdir_global_shutdown); - git_sysdir__dirs_shutdown_set = 1; - } - - GITERR_CHECK_ERROR( - git_sysdir__dir_guess[which](&git_sysdir__dirs[which])); - } - - *out = &git_sysdir__dirs[which]; + *out = &git_sysdir__dirs[which].buf; return 0; } @@ -183,31 +170,38 @@ int git_sysdir_set(git_sysdir_t which, const char *search_path) if (search_path != NULL) expand_path = strstr(search_path, PATH_MAGIC); - /* init with default if not yet done and needed (ignoring error) */ - if ((!search_path || expand_path) && - !git_buf_len(&git_sysdir__dirs[which])) - git_sysdir__dir_guess[which](&git_sysdir__dirs[which]); + /* reset the default if this path has been cleared */ + if (!search_path || expand_path) + git_sysdir__dirs[which].guess(&git_sysdir__dirs[which].buf); /* if $PATH is not referenced, then just set the path */ - if (!expand_path) - return git_buf_sets(&git_sysdir__dirs[which], search_path); + if (!expand_path) { + if (search_path) + git_buf_sets(&git_sysdir__dirs[which].buf, search_path); + + goto done; + } /* otherwise set to join(before $PATH, old value, after $PATH) */ if (expand_path > search_path) git_buf_set(&merge, search_path, expand_path - search_path); - if (git_buf_len(&git_sysdir__dirs[which])) + if (git_buf_len(&git_sysdir__dirs[which].buf)) git_buf_join(&merge, GIT_PATH_LIST_SEPARATOR, - merge.ptr, git_sysdir__dirs[which].ptr); + merge.ptr, git_sysdir__dirs[which].buf.ptr); expand_path += strlen(PATH_MAGIC); if (*expand_path) git_buf_join(&merge, GIT_PATH_LIST_SEPARATOR, merge.ptr, expand_path); - git_buf_swap(&git_sysdir__dirs[which], &merge); + git_buf_swap(&git_sysdir__dirs[which].buf, &merge); git_buf_free(&merge); - return git_buf_oom(&git_sysdir__dirs[which]) ? -1 : 0; +done: + if (git_buf_oom(&git_sysdir__dirs[which].buf)) + return -1; + + return 0; } static int git_sysdir_find_in_dirlist( diff --git a/src/sysdir.h b/src/sysdir.h index 12874fc85..11878981c 100644 --- a/src/sysdir.h +++ b/src/sysdir.h @@ -103,9 +103,4 @@ extern int git_sysdir_get_str(char *out, size_t outlen, git_sysdir_t which); */ extern int git_sysdir_set(git_sysdir_t which, const char *paths); -/** - * Free the configuration file search paths. - */ -extern void git_sysdir_global_shutdown(void); - #endif /* INCLUDE_sysdir_h__ */ From b64722fd52113a0534527c7e0867d60f32783ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 5 Aug 2016 18:40:37 +0200 Subject: [PATCH 54/67] SecureTransport: handle NULL trust on success The `SSLCopyPeerTrust` call can succeed but fail to return a trust object if it can't load the certificate chain and thus cannot check the validity of a certificate. This can lead to us calling `CFRelease` on a `NULL` trust object, causing a crash. Handle this by returning ECERTIFICATE. --- src/stransport_stream.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 832f66b45..2a13fb55b 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -67,6 +67,9 @@ int stransport_connect(git_stream *stream) if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr) goto on_error; + if (!trust) + return GIT_ECERTIFICATE; + if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr) goto on_error; From 9aee7bc249ede39a7cf4d3bbbbc8eb273cfc7989 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 8 Aug 2016 13:49:17 +0200 Subject: [PATCH 55/67] stransport: make internal functions static --- src/stransport_stream.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 2a13fb55b..fc5f862a6 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -16,7 +16,7 @@ #include "socket_stream.h" #include "curl_stream.h" -int stransport_error(OSStatus ret) +static int stransport_error(OSStatus ret) { CFStringRef message; @@ -47,7 +47,7 @@ typedef struct { git_cert_x509 cert_info; } stransport_stream; -int stransport_connect(git_stream *stream) +static int stransport_connect(git_stream *stream) { stransport_stream *st = (stransport_stream *) stream; int error; @@ -93,7 +93,7 @@ on_error: return stransport_error(ret); } -int stransport_certificate(git_cert **out, git_stream *stream) +static int stransport_certificate(git_cert **out, git_stream *stream) { stransport_stream *st = (stransport_stream *) stream; SecTrustRef trust = NULL; @@ -120,7 +120,7 @@ int stransport_certificate(git_cert **out, git_stream *stream) return 0; } -int stransport_set_proxy(git_stream *stream, const char *proxy) +static int stransport_set_proxy(git_stream *stream, const char *proxy) { stransport_stream *st = (stransport_stream *) stream; @@ -150,7 +150,7 @@ static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len) return noErr; } -ssize_t stransport_write(git_stream *stream, const char *data, size_t len, int flags) +static ssize_t stransport_write(git_stream *stream, const char *data, size_t len, int flags) { stransport_stream *st = (stransport_stream *) stream; size_t data_len, processed; @@ -199,7 +199,7 @@ static OSStatus read_cb(SSLConnectionRef conn, void *data, size_t *len) return error; } -ssize_t stransport_read(git_stream *stream, void *data, size_t len) +static ssize_t stransport_read(git_stream *stream, void *data, size_t len) { stransport_stream *st = (stransport_stream *) stream; size_t processed; @@ -211,7 +211,7 @@ ssize_t stransport_read(git_stream *stream, void *data, size_t len) return processed; } -int stransport_close(git_stream *stream) +static int stransport_close(git_stream *stream) { stransport_stream *st = (stransport_stream *) stream; OSStatus ret; @@ -223,7 +223,7 @@ int stransport_close(git_stream *stream) return git_stream_close(st->io); } -void stransport_free(git_stream *stream) +static void stransport_free(git_stream *stream) { stransport_stream *st = (stransport_stream *) stream; From 8f342c6d6b4efb794d96e58c4420dc75b856af99 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 8 Aug 2016 14:47:32 +0200 Subject: [PATCH 56/67] stransport: do not use `git_stream_free` on uninitialized stransport When failing to initialize a new stransport stream, we try to release already allocated memory by calling out to `git_stream_free`, which in turn called out to the stream's `free` function pointer. As we only initialize the function pointer later on, this leads to a `NULL` pointer exception. Furthermore, plug another memory leak when failing to create the SSL context. --- src/stransport_stream.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index fc5f862a6..9d87d6029 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -259,6 +259,7 @@ int git_stransport_stream_new(git_stream **out, const char *host, const char *po st->ctx = SSLCreateContext(NULL, kSSLClientSide, kSSLStreamType); if (!st->ctx) { giterr_set(GITERR_NET, "failed to create SSL context"); + git__free(st); return -1; } @@ -268,7 +269,8 @@ int git_stransport_stream_new(git_stream **out, const char *host, const char *po (ret = SSLSetProtocolVersionMin(st->ctx, kTLSProtocol1)) != noErr || (ret = SSLSetProtocolVersionMax(st->ctx, kTLSProtocol12)) != noErr || (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) { - git_stream_free((git_stream *)st); + CFRelease(st->ctx); + git__free(st); return stransport_error(ret); } From 0dea429978cf608d77344b18a303087990b903de Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 12 Aug 2016 09:06:15 +0200 Subject: [PATCH 57/67] ignore: allow unignoring basenames in subdirectories The .gitignore file allows for patterns which unignore previous ignore patterns. When unignoring a previous pattern, there are basically three cases how this is matched when no globbing is used: 1. when a previous file has been ignored, it can be unignored by using its exact name, e.g. foo/bar !foo/bar 2. when a file in a subdirectory has been ignored, it can be unignored by using its basename, e.g. foo/bar !bar 3. when all files with a basename are ignored, a specific file can be unignored again by specifying its path in a subdirectory, e.g. bar !foo/bar The first problem in libgit2 is that we did not correctly treat the second case. While we verified that the negative pattern matches the tail of the positive one, we did not verify if it only matches the basename of the positive pattern. So e.g. we would have also negated a pattern like foo/fruz_bar !bar Furthermore, we did not check for the third case, where a basename is being unignored in a certain subdirectory again. Both issues are fixed with this commit. --- src/ignore.c | 61 +++++++++++++++++++++++++++++++------------ tests/status/ignore.c | 38 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 16 deletions(-) diff --git a/src/ignore.c b/src/ignore.c index ac2af4f58..dcbd5c1ca 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -11,35 +11,64 @@ #define GIT_IGNORE_DEFAULT_RULES ".\n..\n.git\n" /** - * A negative ignore pattern can match a positive one without - * wildcards if its pattern equals the tail of the positive - * pattern. Thus + * A negative ignore pattern can negate a positive one without + * wildcards if it is a basename only and equals the basename of + * the positive pattern. Thus * * foo/bar * !bar * - * would result in foo/bar being unignored again. + * would result in foo/bar being unignored again while + * + * moo/foo/bar + * !foo/bar + * + * would do nothing. The reverse also holds true: a positive + * basename pattern can be negated by unignoring the basename in + * subdirectories. Thus + * + * bar + * !foo/bar + * + * would result in foo/bar being unignored again. As with the + * first case, + * + * foo/bar + * !moo/foo/bar + * + * would do nothing, again. */ static int does_negate_pattern(git_attr_fnmatch *rule, git_attr_fnmatch *neg) { + git_attr_fnmatch *longer, *shorter; char *p; if ((rule->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0 && (neg->flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0) { - /* - * no chance of matching if rule is shorter than - * the negated one - */ - if (rule->length < neg->length) + + /* If lengths match we need to have an exact match */ + if (rule->length == neg->length) { + return strcmp(rule->pattern, neg->pattern) == 0; + } else if (rule->length < neg->length) { + shorter = rule; + longer = neg; + } else { + shorter = neg; + longer = rule; + } + + /* Otherwise, we need to check if the shorter + * rule is a basename only (that is, it contains + * no path separator) and, if so, if it + * matches the tail of the longer rule */ + p = longer->pattern + longer->length - shorter->length; + + if (p[-1] != '/') + return false; + if (memchr(shorter->pattern, '/', shorter->length) != NULL) return false; - /* - * shift pattern so its tail aligns with the - * negated pattern - */ - p = rule->pattern + rule->length - neg->length; - if (strcmp(p, neg->pattern) == 0) - return true; + return memcmp(p, shorter->pattern, shorter->length) == 0; } return false; diff --git a/tests/status/ignore.c b/tests/status/ignore.c index c318046da..c4878b2dd 100644 --- a/tests/status/ignore.c +++ b/tests/status/ignore.c @@ -945,6 +945,44 @@ void test_status_ignore__negative_directory_ignores(void) assert_is_ignored("padded_parent/child8/bar.txt"); } +void test_status_ignore__unignore_entry_in_ignored_dir(void) +{ + static const char *test_files[] = { + "empty_standard_repo/bar.txt", + "empty_standard_repo/parent/bar.txt", + "empty_standard_repo/parent/child/bar.txt", + "empty_standard_repo/nested/parent/child/bar.txt", + NULL + }; + + make_test_data("empty_standard_repo", test_files); + cl_git_mkfile( + "empty_standard_repo/.gitignore", + "bar.txt\n" + "!parent/child/bar.txt\n"); + + assert_is_ignored("bar.txt"); + assert_is_ignored("parent/bar.txt"); + refute_is_ignored("parent/child/bar.txt"); + assert_is_ignored("nested/parent/child/bar.txt"); +} + +void test_status_ignore__do_not_unignore_basename_prefix(void) +{ + static const char *test_files[] = { + "empty_standard_repo/foo_bar.txt", + NULL + }; + + make_test_data("empty_standard_repo", test_files); + cl_git_mkfile( + "empty_standard_repo/.gitignore", + "foo_bar.txt\n" + "!bar.txt\n"); + + assert_is_ignored("foo_bar.txt"); +} + void test_status_ignore__filename_with_cr(void) { int ignored; From 12b73ff3806d167cf2561c0b21e5354bb6f81601 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Aug 2016 11:00:05 +0200 Subject: [PATCH 58/67] transports: http: reset `connected` flag when re-connecting transport When calling `http_connect` on a subtransport whose stream is already connected, we first close the stream in case no keep-alive is in use. When doing so, we do not reset the transport's connection state, though. Usually, this will do no harm in case the subsequent connect will succeed. But when the connection fails we are left with a substransport which is tagged as connected but which has no valid stream attached. Fix the issue by resetting the subtransport's connected-state when closing its stream in `http_connect`. --- src/transports/http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/transports/http.c b/src/transports/http.c index 4bf1d91bf..f0efd956a 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -569,6 +569,7 @@ static int http_connect(http_subtransport *t) git_stream_close(t->io); git_stream_free(t->io); t->io = NULL; + t->connected = 0; } if (t->connection_data.use_ssl) { From e499b13cb0dc0c47a1ed31ed568f3a226c4bc42f Mon Sep 17 00:00:00 2001 From: Stefan Huber Date: Wed, 24 Aug 2016 01:20:39 +0200 Subject: [PATCH 59/67] git_checkout_tree options fix According to the reference the git_checkout_tree and git_checkout_head functions should accept NULL in the opts field This was broken since the opts field was dereferenced and thus lead to a crash. --- src/checkout.c | 2 +- tests/checkout/tree.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/checkout.c b/src/checkout.c index c6d91a938..1bea00da8 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -2699,7 +2699,7 @@ int git_checkout_tree( if ((error = git_repository_index(&index, repo)) < 0) return error; - if ((opts->checkout_strategy & GIT_CHECKOUT_DISABLE_PATHSPEC_MATCH)) { + if (opts && (opts->checkout_strategy & GIT_CHECKOUT_DISABLE_PATHSPEC_MATCH)) { iter_opts.pathlist.count = opts->paths.count; iter_opts.pathlist.strings = opts->paths.strings; } diff --git a/tests/checkout/tree.c b/tests/checkout/tree.c index 7df4d7ef0..4a0314a9e 100644 --- a/tests/checkout/tree.c +++ b/tests/checkout/tree.c @@ -1479,3 +1479,7 @@ void test_checkout_tree__baseline_is_empty_when_no_index(void) git_reference_free(head); } +void test_checkout_tree__nullopts(void) +{ + cl_git_pass(git_checkout_tree(g_repo, NULL, NULL)); +} From edf420f37b1be635b72717f53d81d0973613e5b3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 5 Sep 2016 13:24:07 +0200 Subject: [PATCH 60/67] cmake: add curl library path The `PKG_CHECK_MODULES` function searches a pkg-config module and then proceeds to set various variables containing information on how to link to the library. In contrast to the `FIND_PACKAGE` function, the library path set by `PKG_CHECK_MODULES` will not necessarily contain linking instructions with a complete path to the library, though. So when a library is not installed in a standard location, the linker might later fail due to being unable to locate it. While we already honor this when configuring libssh2 by adding `LIBSSH2_LIBRARY_DIRS` to the link directories, we fail to do so for libcurl, preventing us to build libgit2 on e.g. FreeBSD. Fix the issue by adding the curl library directory to the linker search path. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7baf68b5d..0f9cff575 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -286,6 +286,7 @@ ELSE () IF (CURL_FOUND) ADD_DEFINITIONS(-DGIT_CURL) INCLUDE_DIRECTORIES(${CURL_INCLUDE_DIRS}) + LINK_DIRECTORIES(${CURL_LIBRARY_DIRS}) LINK_LIBRARIES(${CURL_LIBRARIES}) LIST(APPEND LIBGIT2_PC_LIBS ${CURL_LDFLAGS}) ENDIF() From 886bd6a617281d598398be6dab6aa857a33debb5 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 3 Aug 2016 17:01:48 -0400 Subject: [PATCH 61/67] mwindow: init mwindow files in git_libgit2_init --- src/global.c | 5 +++-- src/mwindow.c | 29 +++++++++-------------------- src/mwindow.h | 3 +-- src/odb_pack.c | 3 --- 4 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/global.c b/src/global.c index e36912cbe..198bc1e80 100644 --- a/src/global.c +++ b/src/global.c @@ -59,8 +59,9 @@ static int init_common(void) if ((ret = git_hash_global_init()) == 0 && (ret = git_sysdir_global_init()) == 0 && (ret = git_filter_global_init()) == 0 && - (ret = git_transport_ssh_global_init()) == 0) - ret = git_openssl_stream_global_init(); + (ret = git_transport_ssh_global_init()) == 0 && + (ret = git_openssl_stream_global_init()) == 0) + ret = git_mwindow_global_init(); GIT_MEMORY_BARRIER; diff --git a/src/mwindow.c b/src/mwindow.c index d3e9be78b..8a5b5caee 100644 --- a/src/mwindow.c +++ b/src/mwindow.c @@ -33,20 +33,7 @@ static git_mwindow_ctl mem_ctl; /* Global list of mwindow files, to open packs once across repos */ git_strmap *git__pack_cache = NULL; -/** - * Run under mwindow lock - */ -int git_mwindow_files_init(void) -{ - if (git__pack_cache) - return 0; - - git__on_shutdown(git_mwindow_files_free); - - return git_strmap_alloc(&git__pack_cache); -} - -void git_mwindow_files_free(void) +static void git_mwindow_files_free(void) { git_strmap *tmp = git__pack_cache; @@ -54,6 +41,14 @@ void git_mwindow_files_free(void) git_strmap_free(tmp); } +int git_mwindow_global_init(void) +{ + assert(!git__pack_cache); + + git__on_shutdown(git_mwindow_files_free); + return git_strmap_alloc(&git__pack_cache); +} + int git_mwindow_get_pack(struct git_pack_file **out, const char *path) { int error; @@ -69,12 +64,6 @@ int git_mwindow_get_pack(struct git_pack_file **out, const char *path) return -1; } - if (git_mwindow_files_init() < 0) { - git_mutex_unlock(&git__mwindow_mutex); - git__free(packname); - return -1; - } - pos = git_strmap_lookup_index(git__pack_cache, packname); git__free(packname); diff --git a/src/mwindow.h b/src/mwindow.h index 63418e458..bdde9e0a4 100644 --- a/src/mwindow.h +++ b/src/mwindow.h @@ -43,8 +43,7 @@ int git_mwindow_file_register(git_mwindow_file *mwf); void git_mwindow_file_deregister(git_mwindow_file *mwf); void git_mwindow_close(git_mwindow **w_cursor); -int git_mwindow_files_init(void); -void git_mwindow_files_free(void); +extern int git_mwindow_global_init(void); struct git_pack_file; /* just declaration to avoid cyclical includes */ int git_mwindow_get_pack(struct git_pack_file **out, const char *path); diff --git a/src/odb_pack.c b/src/odb_pack.c index 5a57864ad..0764207e5 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -591,9 +591,6 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) struct pack_backend *backend = NULL; git_buf path = GIT_BUF_INIT; - if (git_mwindow_files_init() < 0) - return -1; - if (pack_backend__alloc(&backend, 8) < 0) return -1; From ed5299ac20080f7d8d2099275222380dc9a69d55 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 5 Aug 2016 20:34:19 -0400 Subject: [PATCH 62/67] odb: actually insert the empty blob in tests --- tests/index/collision.c | 60 ++++++++++++++++++++--------------------- tests/reset/hard.c | 6 ++++- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/tests/index/collision.c b/tests/index/collision.c index 19c1548e9..ad5827e0b 100644 --- a/tests/index/collision.c +++ b/tests/index/collision.c @@ -2,105 +2,103 @@ #include "git2/repository.h" #include "git2/index.h" -git_repository *repo = NULL; +static git_repository *g_repo; +static git_odb *g_odb; +static git_index *g_index; +static git_oid g_empty_id; + +void test_index_collision__initialize(void) +{ + g_repo = cl_git_sandbox_init("empty_standard_repo"); + cl_git_pass(git_repository_odb(&g_odb, g_repo)); + cl_git_pass(git_repository_index(&g_index, g_repo)); + + cl_git_pass(git_odb_write(&g_empty_id, g_odb, "", 0, GIT_OBJ_BLOB)); +} void test_index_collision__cleanup(void) { + git_index_free(g_index); + git_odb_free(g_odb); cl_git_sandbox_cleanup(); - repo = NULL; } void test_index_collision__add(void) { - git_index *index; git_index_entry entry; git_oid tree_id; git_tree *tree; - repo = cl_git_sandbox_init("empty_standard_repo"); - cl_git_pass(git_repository_index(&index, repo)); - memset(&entry, 0, sizeof(entry)); entry.ctime.seconds = 12346789; entry.mtime.seconds = 12346789; entry.mode = 0100644; entry.file_size = 0; - git_oid_fromstr(&entry.id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"); + git_oid_cpy(&entry.id, &g_empty_id); entry.path = "a/b"; - cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_add(g_index, &entry)); /* create a tree/blob collision */ entry.path = "a/b/c"; - cl_git_fail(git_index_add(index, &entry)); + cl_git_fail(git_index_add(g_index, &entry)); - cl_git_pass(git_index_write_tree(&tree_id, index)); - cl_git_pass(git_tree_lookup(&tree, repo, &tree_id)); + cl_git_pass(git_index_write_tree(&tree_id, g_index)); + cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id)); git_tree_free(tree); - git_index_free(index); } void test_index_collision__add_with_highstage_1(void) { - git_index *index; git_index_entry entry; - repo = cl_git_sandbox_init("empty_standard_repo"); - cl_git_pass(git_repository_index(&index, repo)); - memset(&entry, 0, sizeof(entry)); entry.ctime.seconds = 12346789; entry.mtime.seconds = 12346789; entry.mode = 0100644; entry.file_size = 0; - git_oid_fromstr(&entry.id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"); + git_oid_cpy(&entry.id, &g_empty_id); entry.path = "a/b"; GIT_IDXENTRY_STAGE_SET(&entry, 2); - cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_add(g_index, &entry)); /* create a blob beneath the previous tree entry */ entry.path = "a/b/c"; entry.flags = 0; - cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_add(g_index, &entry)); /* create another tree entry above the blob */ entry.path = "a/b"; GIT_IDXENTRY_STAGE_SET(&entry, 1); - cl_git_pass(git_index_add(index, &entry)); - - git_index_free(index); + cl_git_pass(git_index_add(g_index, &entry)); } void test_index_collision__add_with_highstage_2(void) { - git_index *index; git_index_entry entry; - repo = cl_git_sandbox_init("empty_standard_repo"); - cl_git_pass(git_repository_index(&index, repo)); + cl_git_pass(git_repository_index(&g_index, g_repo)); memset(&entry, 0, sizeof(entry)); entry.ctime.seconds = 12346789; entry.mtime.seconds = 12346789; entry.mode = 0100644; entry.file_size = 0; - git_oid_fromstr(&entry.id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"); + git_oid_cpy(&entry.id, &g_empty_id); entry.path = "a/b/c"; GIT_IDXENTRY_STAGE_SET(&entry, 1); - cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_add(g_index, &entry)); /* create a blob beneath the previous tree entry */ entry.path = "a/b/c"; GIT_IDXENTRY_STAGE_SET(&entry, 2); - cl_git_pass(git_index_add(index, &entry)); + cl_git_pass(git_index_add(g_index, &entry)); /* create another tree entry above the blob */ entry.path = "a/b"; GIT_IDXENTRY_STAGE_SET(&entry, 3); - cl_git_pass(git_index_add(index, &entry)); - - git_index_free(index); + cl_git_pass(git_index_add(g_index, &entry)); } diff --git a/tests/reset/hard.c b/tests/reset/hard.c index e461f8093..69ef41e50 100644 --- a/tests/reset/hard.c +++ b/tests/reset/hard.c @@ -240,14 +240,18 @@ void test_reset_hard__switch_file_to_dir(void) { git_index_entry entry = {{ 0 }}; git_index *idx; + git_odb *odb; git_object *commit; git_tree *tree; git_signature *sig; git_oid src_tree_id, tgt_tree_id; git_oid src_id, tgt_id; + cl_git_pass(git_repository_odb(&odb, repo)); + cl_git_pass(git_odb_write(&entry.id, odb, "", 0, GIT_OBJ_BLOB)); + git_odb_free(odb); + entry.mode = GIT_FILEMODE_BLOB; - cl_git_pass(git_oid_fromstr(&entry.id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391")); cl_git_pass(git_index_new(&idx)); cl_git_pass(git_signature_now(&sig, "foo", "bar")); From 5a9d850e76c0ff5bc3118f3d129e8a9b6c07c262 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 5 Aug 2016 19:30:56 -0400 Subject: [PATCH 63/67] odb: only provide the empty tree Only provide the empty tree internally, which matches git's behavior. If we provide the empty blob then any users trying to write it with libgit2 would omit it from actually landing in the odb, which appear to git proper as a broken repository (missing that object). --- src/odb.c | 9 +-------- tests/odb/emptyobjects.c | 35 ++++++++++++++++++----------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/odb.c b/src/odb.c index f396149b3..1ecfe937f 100644 --- a/src/odb.c +++ b/src/odb.c @@ -803,19 +803,12 @@ int git_odb__read_header_or_object( return 0; } -static git_oid empty_blob = {{ 0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b, - 0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91 }}; static git_oid empty_tree = {{ 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04 }}; static int hardcoded_objects(git_rawobj *raw, const git_oid *id) { - if (!git_oid_cmp(id, &empty_blob)) { - raw->type = GIT_OBJ_BLOB; - raw->len = 0; - raw->data = git__calloc(1, sizeof(uint8_t)); - return 0; - } else if (!git_oid_cmp(id, &empty_tree)) { + if (!git_oid_cmp(id, &empty_tree)) { raw->type = GIT_OBJ_TREE; raw->len = 0; raw->data = git__calloc(1, sizeof(uint8_t)); diff --git a/tests/odb/emptyobjects.c b/tests/odb/emptyobjects.c index 783d05197..61bb2b82c 100644 --- a/tests/odb/emptyobjects.c +++ b/tests/odb/emptyobjects.c @@ -2,29 +2,33 @@ #include "odb.h" #include "filebuf.h" +#define TEST_REPO_PATH "redundant.git" + git_repository *g_repo; +git_odb *g_odb; void test_odb_emptyobjects__initialize(void) { - cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git"))); -} -void test_odb_emptyobjects__cleanup(void) -{ - git_repository_free(g_repo); + g_repo = cl_git_sandbox_init(TEST_REPO_PATH); + cl_git_pass(git_repository_odb(&g_odb, g_repo)); } -void test_odb_emptyobjects__read(void) +void test_odb_emptyobjects__cleanup(void) { - git_oid id; + git_odb_free(g_odb); + cl_git_sandbox_cleanup(); +} + +void test_odb_emptyobjects__blob_notfound(void) +{ + git_oid id, written_id; git_blob *blob; cl_git_pass(git_oid_fromstr(&id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391")); - cl_git_pass(git_blob_lookup(&blob, g_repo, &id)); - cl_assert_equal_i(GIT_OBJ_BLOB, git_object_type((git_object *) blob)); - cl_assert(git_blob_rawcontent(blob)); - cl_assert_equal_s("", git_blob_rawcontent(blob)); - cl_assert_equal_i(0, git_blob_rawsize(blob)); - git_blob_free(blob); + cl_git_fail_with(GIT_ENOTFOUND, git_blob_lookup(&blob, g_repo, &id)); + + cl_git_pass(git_odb_write(&written_id, g_odb, "", 0, GIT_OBJ_BLOB)); + cl_assert(git_path_exists(TEST_REPO_PATH "/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391")); } void test_odb_emptyobjects__read_tree(void) @@ -43,15 +47,12 @@ void test_odb_emptyobjects__read_tree(void) void test_odb_emptyobjects__read_tree_odb(void) { git_oid id; - git_odb *odb; git_odb_object *tree_odb; cl_git_pass(git_oid_fromstr(&id, "4b825dc642cb6eb9a060e54bf8d69288fbee4904")); - cl_git_pass(git_repository_odb(&odb, g_repo)); - cl_git_pass(git_odb_read(&tree_odb, odb, &id)); + cl_git_pass(git_odb_read(&tree_odb, g_odb, &id)); cl_assert(git_odb_object_data(tree_odb)); cl_assert_equal_s("", git_odb_object_data(tree_odb)); cl_assert_equal_i(0, git_odb_object_size(tree_odb)); git_odb_object_free(tree_odb); - git_odb_free(odb); } From adfece0df2153234970ded1b0552c5150e7f0c53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 23 Mar 2016 16:51:52 +0100 Subject: [PATCH 64/67] array: fix search for empty arrays When the array is empty `cmp` never gets set by the comparison function. Initialize it so we return ENOTFOUND in those cases. --- src/array.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/array.h b/src/array.h index 490e6be20..78d321e82 100644 --- a/src/array.h +++ b/src/array.h @@ -96,7 +96,7 @@ GIT_INLINE(int) git_array__search( { size_t lim; unsigned char *part, *array = array_ptr, *base = array_ptr; - int cmp; + int cmp = -1; for (lim = array_len; lim != 0; lim >>= 1) { part = base + (lim >> 1) * item_size; From 1db5e0276f9641e57744508e5e81356f804a9592 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 22 Jul 2016 17:45:03 -0400 Subject: [PATCH 65/67] ci: install homebrew's zlib on mac --- script/cibuild.sh | 4 ++++ script/install-deps-osx.sh | 1 + 2 files changed, 5 insertions(+) diff --git a/script/cibuild.sh b/script/cibuild.sh index 00cde0ada..54b4f1b58 100755 --- a/script/cibuild.sh +++ b/script/cibuild.sh @@ -6,6 +6,10 @@ then exit $?; fi +if [ "$TRAVIS_OS_NAME" = "osx" ]; then + export PKG_CONFIG_PATH=$(ls -d /usr/local/Cellar/zlib/*/lib/pkgconfig | paste -s -d':' -) +fi + mkdir _build cd _build # shellcheck disable=SC2086 diff --git a/script/install-deps-osx.sh b/script/install-deps-osx.sh index 5510379d4..da0672d2b 100755 --- a/script/install-deps-osx.sh +++ b/script/install-deps-osx.sh @@ -3,4 +3,5 @@ set -x brew update +brew install homebrew/dupes/zlib brew install libssh2 From 0c7e546fa748334a3fd3413db442132b7d6b166b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sun, 24 Jul 2016 14:51:28 -0400 Subject: [PATCH 66/67] ci: install homebrew's curl on mac --- script/cibuild.sh | 2 +- script/install-deps-osx.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/script/cibuild.sh b/script/cibuild.sh index 54b4f1b58..5707f01cb 100755 --- a/script/cibuild.sh +++ b/script/cibuild.sh @@ -7,7 +7,7 @@ then fi if [ "$TRAVIS_OS_NAME" = "osx" ]; then - export PKG_CONFIG_PATH=$(ls -d /usr/local/Cellar/zlib/*/lib/pkgconfig | paste -s -d':' -) + export PKG_CONFIG_PATH=$(ls -d /usr/local/Cellar/{curl,zlib}/*/lib/pkgconfig | paste -s -d':' -) fi mkdir _build diff --git a/script/install-deps-osx.sh b/script/install-deps-osx.sh index da0672d2b..4b8393b19 100755 --- a/script/install-deps-osx.sh +++ b/script/install-deps-osx.sh @@ -4,4 +4,5 @@ set -x brew update brew install homebrew/dupes/zlib +brew install curl brew install libssh2 From 8e268168ecfdcc8efe36b58b514d1b93ea3f47f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 2 Oct 2016 13:20:28 +0200 Subject: [PATCH 67/67] Bump version to 0.24.2 --- include/git2/version.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/git2/version.h b/include/git2/version.h index 66a6623cd..93b369462 100644 --- a/include/git2/version.h +++ b/include/git2/version.h @@ -7,10 +7,10 @@ #ifndef INCLUDE_git_version_h__ #define INCLUDE_git_version_h__ -#define LIBGIT2_VERSION "0.24.0" +#define LIBGIT2_VERSION "0.24.2" #define LIBGIT2_VER_MAJOR 0 #define LIBGIT2_VER_MINOR 24 -#define LIBGIT2_VER_REVISION 0 +#define LIBGIT2_VER_REVISION 2 #define LIBGIT2_VER_PATCH 0 #define LIBGIT2_SOVERSION 24