Merge pull request #5064 from pks-t/pks/v0.28.2

Release v0.28.2

docs/changelog.md

+v0.28.2
+-------
+
+This is a bugfix release with the following changes:
+
+* Fix include directory ordering when using bundled dependencies.
+
+* Fix infinite loop when searching for a non-existing repository with
+  Windows-style paths including drive prefixes.
+
+* Fix paths with a trailing "/" not always being treated as
+  directories when computing ignores.
+
+* Fix false negatives when computing ignores where ignore rules
+  that are a prefix to a negative ignore rule exist.
+
+* Fix patches with CRLF line endings not being parsed correctly.
+
+* Fix segfault when parsing patches with file addition (deletion)
+  where the added (deleted) file name contains a space.
+
+* Fix assertion failure when trying to write to a non-existent
+  locked configuration file.
+
 v0.28.1
 -------
 

include/git2/version.h

@@ -7,10 +7,10 @@
 #ifndef INCLUDE_git_version_h__
 #define INCLUDE_git_version_h__
 
-#define LIBGIT2_VERSION "0.28.1"
+#define LIBGIT2_VERSION "0.28.2"
 #define LIBGIT2_VER_MAJOR 0
 #define LIBGIT2_VER_MINOR 28
-#define LIBGIT2_VER_REVISION 1
+#define LIBGIT2_VER_REVISION 2
 #define LIBGIT2_VER_PATCH 0
 
 #define LIBGIT2_SOVERSION 28

src/CMakeLists.txt

@@ -48,11 +48,23 @@ IF (ENABLE_TRACE STREQUAL "ON")
 ENDIF()
 ADD_FEATURE_INFO(tracing GIT_TRACE "tracing support")
 
+# Use `regcomp_l` if available
 CHECK_SYMBOL_EXISTS(regcomp_l "regex.h;xlocale.h" HAVE_REGCOMP_L)
 IF (HAVE_REGCOMP_L)
 	SET(GIT_USE_REGCOMP_L 1)
 ENDIF ()
 
+# Otherwise, we either want to use system's `regcomp` or our
+# bundled regcomp code, if system doesn't provide `regcomp`.
+IF(NOT HAVE_REGCOMP_L)
+	CHECK_FUNCTION_EXISTS(regcomp HAVE_REGCOMP)
+	IF(NOT HAVE_REGCOMP)
+		ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/regex" "${libgit2_BINARY_DIR}/deps/regex")
+		LIST(APPEND LIBGIT2_INCLUDES "${libgit2_SOURCE_DIR}/deps/regex")
+		LIST(APPEND LIBGIT2_OBJECTS $<TARGET_OBJECTS:regex>)
+	ENDIF()
+ENDIF()
+
 CHECK_FUNCTION_EXISTS(futimens HAVE_FUTIMENS)
 IF (HAVE_FUTIMENS)
 	SET(GIT_USE_FUTIMENS 1)
@@ -117,7 +129,7 @@ IF (WIN32 AND WINHTTP)
 	IF (MINGW)
 		ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/winhttp" "${libgit2_BINARY_DIR}/deps/winhttp")
 		LIST(APPEND LIBGIT2_LIBS winhttp)
-		LIST(APPEND LIBGIT2_SYSTEM_INCLUDES "${libgit2_SOURCE_DIR}/deps/winhttp")
+		LIST(APPEND LIBGIT2_INCLUDES "${libgit2_SOURCE_DIR}/deps/winhttp")
 	ELSE()
 		LIST(APPEND LIBGIT2_LIBS "winhttp")
 		LIST(APPEND LIBGIT2_PC_LIBS "-lwinhttp")
@@ -294,13 +306,6 @@ ELSE()
 	MESSAGE(FATAL_ERROR "Asked for unknown SHA1 backend ${SHA1_BACKEND}")
 ENDIF()
 
-# Include POSIX regex when it is required
-IF(WIN32 OR AMIGA OR CMAKE_SYSTEM_NAME MATCHES "(Solaris|SunOS)")
-	ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/regex" "${libgit2_BINARY_DIR}/deps/regex")
-	LIST(APPEND LIBGIT2_SYSTEM_INCLUDES "${libgit2_SOURCE_DIR}/deps/regex")
-	LIST(APPEND LIBGIT2_OBJECTS $<TARGET_OBJECTS:regex>)
-ENDIF()
-
 # Optional external dependency: http-parser
 FIND_PACKAGE(HTTP_Parser)
 IF (USE_EXT_HTTP_PARSER AND HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUAL 2)
@@ -311,7 +316,7 @@ IF (USE_EXT_HTTP_PARSER AND HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUA
 ELSE()
 	MESSAGE(STATUS "http-parser version 2 was not found or disabled; using bundled 3rd-party sources.")
 	ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/http-parser" "${libgit2_BINARY_DIR}/deps/http-parser")
-	LIST(APPEND LIBGIT2_SYSTEM_INCLUDES "${libgit2_SOURCE_DIR}/deps/http-parser")
+	LIST(APPEND LIBGIT2_INCLUDES "${libgit2_SOURCE_DIR}/deps/http-parser")
 	LIST(APPEND LIBGIT2_OBJECTS "$<TARGET_OBJECTS:http-parser>")
 	ADD_FEATURE_INFO(http-parser ON "http-parser support (bundled)")
 ENDIF()
@@ -335,7 +340,7 @@ IF(NOT USE_BUNDLED_ZLIB)
 ENDIF()
 IF(USE_BUNDLED_ZLIB OR NOT ZLIB_FOUND)
 	ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/zlib" "${libgit2_BINARY_DIR}/deps/zlib")
-	LIST(APPEND LIBGIT2_SYSTEM_INCLUDES "${libgit2_SOURCE_DIR}/deps/zlib")
+	LIST(APPEND LIBGIT2_INCLUDES "${libgit2_SOURCE_DIR}/deps/zlib")
 	LIST(APPEND LIBGIT2_OBJECTS $<TARGET_OBJECTS:zlib>)
 	ADD_FEATURE_INFO(zlib ON "using bundled zlib")
 ENDIF()

src/attr_file.c

@@ -429,18 +429,6 @@ bool git_attr_fnmatch__match(
 		return (p_fnmatch(match->pattern, relpath, flags) != FNM_NOMATCH);
 	}
 
-	/* if path is a directory prefix of a negated pattern, then match */
-	if ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) && path->is_dir) {
-		size_t pathlen = strlen(relpath);
-		bool prefixed = (pathlen <= match->length) &&
-			((match->flags & GIT_ATTR_FNMATCH_ICASE) ?
-			!strncasecmp(match->pattern, relpath, pathlen) :
-			!strncmp(match->pattern, relpath, pathlen));
-
-		if (prefixed && git_path_at_end_of_segment(&match->pattern[pathlen]))
-			return true;
-	}
-
 	return (p_fnmatch(match->pattern, filename, flags) != FNM_NOMATCH);
 }
 

src/config_file.c

@@ -678,6 +678,7 @@ static int parse_include(git_config_parser *reader,
 		return result;
 
 	include = git_array_alloc(reader->file->includes);
+	GIT_ERROR_CHECK_ALLOC(include);
 	memset(include, 0, sizeof(*include));
 	git_array_init(include->includes);
 	include->path = git_buf_detach(&path);
@@ -1132,7 +1133,7 @@ static int config_write(diskfile_backend *cfg, const char *orig_key, const char
 	reader.file = &cfg->file;
 
 	if (cfg->locked) {
-		result = git_buf_puts(&contents, git_buf_cstr(&cfg->locked_content));
+		result = git_buf_puts(&contents, git_buf_cstr(&cfg->locked_content) == NULL ? "" : git_buf_cstr(&cfg->locked_content));
 	} else {
 		/* Lock the file */
 		if ((result = git_filebuf_open(

src/fileops.c

@@ -489,10 +489,13 @@ int git_futils_mkdir(
 
 		assert(len);
 
-		/* we've walked all the given path's parents and it's either relative
-		 * or rooted.  either way, give up and make the entire path.
+		/*
+		 * We've walked all the given path's parents and it's either relative
+		 * (the parent is simply '.') or rooted (the length is less than or
+		 * equal to length of the root path).  The path may be less than the
+		 * root path length on Windows, where `C:` == `C:/`.
 		 */
-		if ((len == 1 && parent_path.ptr[0] == '.') || len == root_len+1) {
+		if ((len == 1 && parent_path.ptr[0] == '.') || len <= root_len) {
 			relative = make_path.ptr;
 			break;
 		}

src/ignore.c

@@ -534,7 +534,9 @@ int git_ignore_path_is_ignored(
 	memset(&path, 0, sizeof(path));
 	memset(&ignores, 0, sizeof(ignores));
 
-	if (git_repository_is_bare(repo))
+	if (!git__suffixcmp(pathname, "/"))
+		dir_flag = GIT_DIR_FLAG_TRUE;
+	else if (git_repository_is_bare(repo))
 		dir_flag = GIT_DIR_FLAG_FALSE;
 
 	if ((error = git_attr_path__init(&path, pathname, workdir, dir_flag)) < 0 ||

src/odb.c

@@ -443,8 +443,12 @@ int git_odb_new(git_odb **out)
 	git_odb *db = git__calloc(1, sizeof(*db));
 	GIT_ERROR_CHECK_ALLOC(db);
 
-	if (git_cache_init(&db->own_cache) < 0 ||
-		git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) {
+	if (git_cache_init(&db->own_cache) < 0) {
+		git__free(db);
+		return -1;
+	}
+	if (git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) {
+		git_cache_free(&db->own_cache);
 		git__free(db);
 		return -1;
 	}
@@ -1124,6 +1128,7 @@ static int odb_otype_fast(git_object_t *type_p, git_odb *db, const git_oid *id)
 
 	if ((object = git_cache_get_raw(odb_cache(db), id)) != NULL) {
 		*type_p = object->cached.type;
+		git_odb_object_free(object);
 		return 0;
 	}
 

src/patch_parse.c

@@ -328,7 +328,8 @@ static int parse_header_start(git_patch_parsed *patch, git_patch_parse_ctx *ctx)
 	 * proceeed here. We then hope for the "---" and "+++" lines to fix that
 	 * for us.
 	 */
-	if (!git_parse_ctx_contains(&ctx->parse_ctx, "\n", 1)) {
+	if (!git_parse_ctx_contains(&ctx->parse_ctx, "\n", 1) &&
+	    !git_parse_ctx_contains(&ctx->parse_ctx, "\r\n", 2)) {
 		git_parse_advance_chars(&ctx->parse_ctx, ctx->parse_ctx.line_len - 1);
 
 		git__free(patch->header_old_path);
@@ -921,21 +922,15 @@ static int check_filenames(git_patch_parsed *patch)
 		return git_parse_err("missing old path");
 
 	/* Ensure (non-renamed) paths match */
-	if (check_header_names(
-			patch->header_old_path, patch->old_path, "old", added) < 0 ||
-		check_header_names(
-			patch->header_new_path, patch->new_path, "new", deleted) < 0)
+	if (check_header_names(patch->header_old_path, patch->old_path, "old", added) < 0 ||
+	    check_header_names(patch->header_new_path, patch->new_path, "new", deleted) < 0)
 		return -1;
 
-	prefixed_old = (!added && patch->old_path) ? patch->old_path :
-		patch->header_old_path;
-	prefixed_new = (!deleted && patch->new_path) ? patch->new_path :
-		patch->header_new_path;
+	prefixed_old = (!added && patch->old_path) ? patch->old_path : patch->header_old_path;
+	prefixed_new = (!deleted && patch->new_path) ? patch->new_path : patch->header_new_path;
 
-	if (check_prefix(
-			&patch->old_prefix, &old_prefixlen, patch, prefixed_old) < 0 ||
-		check_prefix(
-			&patch->new_prefix, &new_prefixlen, patch, prefixed_new) < 0)
+	if ((prefixed_old && check_prefix(&patch->old_prefix, &old_prefixlen, patch, prefixed_old) < 0) ||
+	    (prefixed_new && check_prefix(&patch->new_prefix, &new_prefixlen, patch, prefixed_new) < 0))
 		return -1;
 
 	/* Prefer the rename filenames as they are unambiguous and unprefixed */

src/win32/posix_w32.c

@@ -33,6 +33,10 @@
 # define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x02
 #endif
 
+#ifndef SYMBOLIC_LINK_FLAG_DIRECTORY
+# define SYMBOLIC_LINK_FLAG_DIRECTORY 0x01
+#endif
+
 /* Allowable mode bits on Win32.  Using mode bits that are not supported on
  * Win32 (eg S_IRWXU) is generally ignored, but Wine warns loudly about it
  * so we simply remove them.
@@ -397,13 +401,18 @@ int p_readlink(const char *path, char *buf, size_t bufsiz)
 int p_symlink(const char *target, const char *path)
 {
 	git_win32_path target_w, path_w;
+	DWORD dwFlags;
 
 	if (git_win32_path_from_utf8(path_w, path) < 0 ||
 		git__utf8_to_16(target_w, MAX_PATH, target) < 0)
 		return -1;
 
-	if (!CreateSymbolicLinkW(path_w, target_w,
-	    SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE))
+	dwFlags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
+
+	if (GetFileAttributesW(target_w) & FILE_ATTRIBUTE_DIRECTORY)
+		dwFlags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
+
+	if (!CreateSymbolicLinkW(path_w, target_w, dwFlags))
 		return -1;
 
 	return 0;

tests/attr/ignore.c

@@ -372,3 +372,44 @@ void test_attr_ignore__case_sensitive_unignore_does_nothing(void)
 
 	assert_is_ignored(true, "case/file");
 }
+
+void test_attr_ignore__ignored_subdirfiles_with_subdir_rule(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"dir/*\n"
+		"!dir/sub1/sub2/**\n");
+
+	assert_is_ignored(true, "dir/a.test");
+	assert_is_ignored(true, "dir/sub1/a.test");
+	assert_is_ignored(true, "dir/sub1/sub2");
+}
+
+void test_attr_ignore__ignored_subdirfiles_with_negations(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"dir/*\n"
+		"!dir/a.test\n");
+
+	assert_is_ignored(false, "dir/a.test");
+	assert_is_ignored(true, "dir/b.test");
+	assert_is_ignored(true, "dir/sub1/c.test");
+}
+
+void test_attr_ignore__negative_directory_rules_only_match_directories(void)
+{
+	cl_git_rewritefile(
+		"attr/.gitignore",
+		"*\n"
+		"!/**/\n"
+		"!*.keep\n"
+		"!.gitignore\n"
+	);
+
+	assert_is_ignored(true, "src");
+	assert_is_ignored(true, "src/A");
+	assert_is_ignored(false, "src/");
+	assert_is_ignored(false, "src/A.keep");
+	assert_is_ignored(false, ".gitignore");
+}

tests/config/global.c

@@ -43,6 +43,36 @@ void test_config_global__open_global(void)
 	git_config_free(cfg);
 }
 
+void test_config_global__lock_missing_global_config(void)
+{
+	git_config *cfg;
+	git_config_entry *entry;
+	git_transaction *transaction;
+
+	p_unlink("home/.gitconfig"); /* No global config */
+
+	cl_git_pass(git_config_open_default(&cfg));
+	cl_git_pass(git_config_lock(&transaction, cfg));
+	cl_git_pass(git_config_set_string(cfg, "assertion.fail", "boom"));
+	cl_git_pass(git_transaction_commit(transaction));
+	git_transaction_free(transaction);
+
+	/* cfg is updated */
+	cl_git_pass(git_config_get_entry(&entry, cfg, "assertion.fail"));
+	cl_assert_equal_s("boom", entry->value);
+
+	git_config_entry_free(entry);
+	git_config_free(cfg);
+
+	/* We can reread the new value from the global config */
+	cl_git_pass(git_config_open_default(&cfg));
+	cl_git_pass(git_config_get_entry(&entry, cfg, "assertion.fail"));
+	cl_assert_equal_s("boom", entry->value);
+
+	git_config_entry_free(entry);
+	git_config_free(cfg);
+}
+
 void test_config_global__open_xdg(void)
 {
 	git_config *cfg, *xdg, *selected;

tests/diff/parse.c

@@ -359,3 +359,40 @@ void test_diff_parse__lineinfo(void)
 	git_patch_free(patch);
 	git_diff_free(diff);
 }
+
+
+void test_diff_parse__new_file_with_space(void)
+{
+	const char *content = PATCH_ORIGINAL_NEW_FILE_WITH_SPACE;
+	git_patch *patch;
+	git_diff *diff;
+
+	cl_git_pass(git_diff_from_buffer(&diff, content, strlen(content)));
+	cl_git_pass(git_patch_from_diff((git_patch **) &patch, diff, 0));
+
+	cl_assert_equal_p(patch->diff_opts.old_prefix, NULL);
+	cl_assert_equal_p(patch->delta->old_file.path, NULL);
+	cl_assert_equal_s(patch->diff_opts.new_prefix, "b/");
+	cl_assert_equal_s(patch->delta->new_file.path, "sp ace.txt");
+
+	git_patch_free(patch);
+	git_diff_free(diff);
+}
+
+void test_diff_parse__crlf(void)
+{
+	const char *text = PATCH_CRLF;
+	git_diff *diff;
+	git_patch *patch;
+	const git_diff_delta *delta;
+
+	cl_git_pass(git_diff_from_buffer(&diff, text, strlen(text)));
+	cl_git_pass(git_patch_from_diff(&patch, diff, 0));
+	delta = git_patch_get_delta(patch);
+
+	cl_assert_equal_s(delta->old_file.path, "test-file");
+	cl_assert_equal_s(delta->new_file.path, "test-file");
+
+	git_patch_free(patch);
+	git_diff_free(diff);
+}

tests/odb/mixed.c

@@ -263,3 +263,24 @@ void test_odb_mixed__expand_ids(void)
 	git__free(ids);
 }
 
+void test_odb_mixed__expand_ids_cached(void)
+{
+	git_odb_expand_id *ids;
+	size_t i, num;
+
+	/* test looking for the actual (correct) types after accessing the object */
+
+	setup_prefix_query(&ids, &num);
+
+	for (i = 0; i < num; i++) {
+		git_odb_object *obj;
+		if (ids[i].type == GIT_OBJECT_ANY)
+			continue;
+		cl_git_pass(git_odb_read_prefix(&obj, _odb, &ids[i].id, ids[i].length));
+		git_odb_object_free(obj);
+	}
+
+	cl_git_pass(git_odb_expand_ids(_odb, ids, num));
+	assert_found_objects(ids);
+	git__free(ids);
+}

tests/patch/patch_common.h

@@ -841,3 +841,21 @@
 	"diff --git a/binary.bin b/binary.bin\n" \
 	"index 27184d9..7c94f9e 100644\n" \
 	"Binary files a/binary.bin and b/binary.bin differ\n"
+
+#define PATCH_ORIGINAL_NEW_FILE_WITH_SPACE \
+	"diff --git a/sp ace.txt b/sp ace.txt\n" \
+	"new file mode 100644\n" \
+	"index 000000000..789819226\n" \
+	"--- /dev/null\n" \
+	"+++ b/sp ace.txt\n" \
+	"@@ -0,0 +1 @@\n" \
+	"+a\n"
+
+#define PATCH_CRLF \
+	"diff --git a/test-file b/test-file\r\n" \
+	"new file mode 100644\r\n" \
+	"index 0000000..af431f2 100644\r\n" \
+	"--- /dev/null\r\n" \
+	"+++ b/test-file\r\n" \
+	"@@ -0,0 +1 @@\r\n" \
+	"+a contents\r\n"

tests/repo/init.c

@@ -877,3 +877,15 @@ void test_repo_init__at_filesystem_root(void)
 	git_buf_dispose(&root);
 	git_repository_free(repo);
 }
+
+void test_repo_init__nonexistent_paths(void)
+{
+	git_repository *repo;
+
+#ifdef GIT_WIN32
+	cl_git_fail(git_repository_init(&repo, "Q:/non/existent/path", 0));
+	cl_git_fail(git_repository_init(&repo, "Q:\\non\\existent\\path", 0));
+#else
+	cl_git_fail(git_repository_init(&repo, "/non/existent/path", 0));
+#endif
+}