Make reference lookups apply precomposeunicode

Before these changes, looking up a reference would return the
same precomposed or decomposed form of the reference name that
was used to look it up, so on MacOS which ignores the difference
between the two, a single reference could be looked up either way
and git_reference_name would return the form of the name that was
used to look it up!  This change makes lookup always return the
precomposed name if core.precomposeunicode is set regardless of
which version was used to look it up.  The reference iterator was
already returning the precomposed form from earlier work.

This also updates the CMakeLists.txt rules for enabling iconv
usage because the clar tests for this code were actually not being
activated properly with the old version.

Finally, this moves git_repository_reset_filesystem from include/
git2/repository.h to include/git2/sys/repository.h since it is not
really a function that normal library users should have to think
about very often.

CMakeLists.txt

@@ -31,6 +31,11 @@ OPTION( LIBGIT2_FILENAME	"Name of the produced binary" OFF )
 
 OPTION( ANDROID				"Build for android NDK"	 				OFF )
 
+OPTION( USE_ICONV			"Link with and use iconv library" 		OFF )
+IF(APPLE)
+	SET( USE_ICONV ON )
+ENDIF()
+
 IF(MSVC)
 	# This option is only available when building with MSVC. By default, libgit2
 	# is build using the cdecl calling convention, which is useful if you're
@@ -58,8 +63,11 @@ FUNCTION(TARGET_OS_LIBRARIES target)
 		TARGET_LINK_LIBRARIES(${target} socket nsl)
 	ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux")
 		TARGET_LINK_LIBRARIES(${target} rt)
-	ELSEIF(APPLE)
+	ENDIF()
+
+	IF(USE_ICONV)
 		TARGET_LINK_LIBRARIES(${target} iconv)
+		ADD_DEFINITIONS(-DGIT_USE_ICONV)
 	ENDIF()
 
 	IF(THREADSAFE)

include/git2/refs.h

@@ -453,7 +453,7 @@ GIT_EXTERN(int) git_reference_is_remote(git_reference *ref);
 GIT_EXTERN(int) git_reference_is_tag(git_reference *ref);
 
 typedef enum {
-	GIT_REF_FORMAT_NORMAL = 0,
+	GIT_REF_FORMAT_NORMAL = 0u,
 
 	/**
 	 * Control whether one-level refnames are accepted
@@ -461,7 +461,7 @@ typedef enum {
 	 * components). Those are expected to be written only using
 	 * uppercase letters and underscore (FETCH_HEAD, ...)
 	 */
-	GIT_REF_FORMAT_ALLOW_ONELEVEL = (1 << 0),
+	GIT_REF_FORMAT_ALLOW_ONELEVEL = (1u << 0),
 
 	/**
 	 * Interpret the provided name as a reference pattern for a
@@ -470,14 +470,14 @@ typedef enum {
 	 * in place of a one full pathname component
 	 * (e.g., foo/<star>/bar but not foo/bar<star>).
 	 */
-	GIT_REF_FORMAT_REFSPEC_PATTERN = (1 << 1),
+	GIT_REF_FORMAT_REFSPEC_PATTERN = (1u << 1),
 
 	/**
 	 * Interpret the name as part of a refspec in shorthand form
 	 * so the `ONELEVEL` naming rules aren't enforced and 'master'
 	 * becomes a valid name.
 	 */
-	GIT_REF_FORMAT_REFSPEC_SHORTHAND = (1 << 2),
+	GIT_REF_FORMAT_REFSPEC_SHORTHAND = (1u << 2),
 } git_reference_normalize_t;
 
 /**

include/git2/repository.h

@@ -288,25 +288,6 @@ GIT_EXTERN(int) git_repository_init_ext(
 	git_repository_init_options *opts);
 
 /**
- * Update the filesystem config settings for an open repository
- *
- * When a repository is initialized, config values are set based on the
- * properties of the filesystem that the repository is on, such as
- * "core.ignorecase", "core.filemode", "core.symlinks", etc.  If the
- * repository is moved to a new filesystem, these properties may no
- * longer be correct and API calls may not behave as expected.  This
- * call reruns the phase of repository initialization that sets those
- * properties to compensate for the current filesystem of the repo.
- *
- * @param repo A repository object
- * @param recurse_submodules Should submodules be reset recursively
- * @returrn 0 on success, < 0 on error
- */
-GIT_EXTERN(int) git_repository_reset_filesystem(
-	git_repository *repo,
-	int recurse_submodules);
-
-/**
  * Retrieve and resolve the reference pointed at by HEAD.
  *
  * The returned `git_reference` will be owned by caller and

include/git2/sys/repository.h

@@ -27,7 +27,6 @@ GIT_BEGIN_DECL
  */
 GIT_EXTERN(int) git_repository_new(git_repository **out);
 
-
 /**
  * Reset all the internal state in a repository.
  *
@@ -42,6 +41,25 @@ GIT_EXTERN(int) git_repository_new(git_repository **out);
 GIT_EXTERN(void) git_repository__cleanup(git_repository *repo);
 
 /**
+ * Update the filesystem config settings for an open repository
+ *
+ * When a repository is initialized, config values are set based on the
+ * properties of the filesystem that the repository is on, such as
+ * "core.ignorecase", "core.filemode", "core.symlinks", etc.  If the
+ * repository is moved to a new filesystem, these properties may no
+ * longer be correct and API calls may not behave as expected.  This
+ * call reruns the phase of repository initialization that sets those
+ * properties to compensate for the current filesystem of the repo.
+ *
+ * @param repo A repository object
+ * @param recurse_submodules Should submodules be reset recursively
+ * @returrn 0 on success, < 0 on error
+ */
+GIT_EXTERN(int) git_repository_reset_filesystem(
+	git_repository *repo,
+	int recurse_submodules);
+
+/**
  * Set the configuration file for this repository
  *
  * This configuration file will be used for all configuration

src/path.h

@@ -365,11 +365,6 @@ extern int git_path_set_error(
 /* check if non-ascii characters are present in filename */
 extern bool git_path_has_non_ascii(const char *path, size_t pathlen);
 
-/* only enable iconv on Mac for now */
-#ifdef __APPLE__
-#define GIT_USE_ICONV 1
-#endif
-
 #define GIT_PATH_REPO_ENCODING "UTF-8"
 
 #ifdef __APPLE__

src/refs.c

@@ -138,6 +138,22 @@ int git_reference_name_to_id(
 	return 0;
 }
 
+static int reference_normalize_for_repo(
+	char *out,
+	size_t out_size,
+	git_repository *repo,
+	const char *name)
+{
+	int precompose;
+	unsigned int flags = GIT_REF_FORMAT_ALLOW_ONELEVEL;
+
+	if (!git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) &&
+		precompose)
+		flags |= GIT_REF_FORMAT__PRECOMPOSE_UNICODE;
+
+	return git_reference_normalize_name(out, out_size, name, flags);
+}
+
 int git_reference_lookup_resolved(
 	git_reference **ref_out,
 	git_repository *repo,
@@ -159,13 +175,13 @@ int git_reference_lookup_resolved(
 	else if (max_nesting < 0)
 		max_nesting = DEFAULT_NESTING_LEVEL;
 
-	strncpy(scan_name, name, GIT_REFNAME_MAX);
 	scan_type = GIT_REF_SYMBOLIC;
 
-	if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0)
-		return -1;
+	if ((error = reference_normalize_for_repo(
+			scan_name, sizeof(scan_name), repo, name)) < 0)
+		return error;
 
-	if ((error = git_reference__normalize_name_lax(scan_name, GIT_REFNAME_MAX, name)) < 0)
+	if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0)
 		return error;
 
 	for (nesting = max_nesting;
@@ -173,7 +189,7 @@ int git_reference_lookup_resolved(
 		 nesting--)
 	{
 		if (nesting != max_nesting) {
-			strncpy(scan_name, ref->target.symbolic, GIT_REFNAME_MAX);
+			strncpy(scan_name, ref->target.symbolic, sizeof(scan_name));
 			git_reference_free(ref);
 		}
 
@@ -711,17 +727,18 @@ static bool is_all_caps_and_underscore(const char *name, size_t len)
 	return true;
 }
 
+/* Inspired from https://github.com/git/git/blob/f06d47e7e0d9db709ee204ed13a8a7486149f494/refs.c#L36-100 */
 int git_reference__normalize_name(
 	git_buf *buf,
 	const char *name,
 	unsigned int flags)
 {
-	// Inspired from https://github.com/git/git/blob/f06d47e7e0d9db709ee204ed13a8a7486149f494/refs.c#L36-100
-
 	char *current;
 	int segment_len, segments_count = 0, error = GIT_EINVALIDSPEC;
 	unsigned int process_flags;
 	bool normalize = (buf != NULL);
+	git_path_iconv_t ic = GIT_PATH_ICONV_INIT;
+
 	assert(name);
 
 	process_flags = flags;
@@ -733,6 +750,13 @@ int git_reference__normalize_name(
 	if (normalize)
 		git_buf_clear(buf);
 
+	if ((flags & GIT_REF_FORMAT__PRECOMPOSE_UNICODE) != 0) {
+		size_t namelen = strlen(current);
+		if ((error = git_path_iconv_init_precompose(&ic)) < 0 ||
+			(error = git_path_iconv(&ic, &current, &namelen)) < 0)
+			goto cleanup;
+	}
+
 	while (true) {
 		segment_len = ensure_segment_validity(current);
 		if (segment_len < 0) {
@@ -809,6 +833,8 @@ cleanup:
 	if (error && normalize)
 		git_buf_free(buf);
 
+	git_path_iconv_clear(&ic);
+
 	return error;
 }
 
@@ -1027,24 +1053,19 @@ cleanup:
 	return error;
 }
 
-int git_reference__is_valid_name(
-	const char *refname,
-	unsigned int flags)
+int git_reference__is_valid_name(const char *refname, unsigned int flags)
 {
-	int error;
-
-	error = git_reference__normalize_name(NULL, refname, flags) == 0;
+	if (git_reference__normalize_name(NULL, refname, flags) < 0) {
 		giterr_clear();
+		return false;
+	}
 
-	return error;
+	return true;
 }
 
-int git_reference_is_valid_name(
-	const char *refname)
+int git_reference_is_valid_name(const char *refname)
 {
-	return git_reference__is_valid_name(
-		refname,
-		GIT_REF_FORMAT_ALLOW_ONELEVEL);
+	return git_reference__is_valid_name(refname, GIT_REF_FORMAT_ALLOW_ONELEVEL);
 }
 
 const char *git_reference_shorthand(git_reference *ref)

src/refs.h

@@ -46,6 +46,8 @@
 #define GIT_STASH_FILE "stash"
 #define GIT_REFS_STASH_FILE GIT_REFS_DIR GIT_STASH_FILE
 
+#define GIT_REF_FORMAT__PRECOMPOSE_UNICODE	(1u << 16)
+
 #define GIT_REFNAME_MAX 1024
 
 struct git_reference {

tests-clar/clar_libgit2.c

 #include "clar_libgit2.h"
 #include "posix.h"
 #include "path.h"
+#include "git2/sys/repository.h"
 
 void cl_git_report_failure(
 	int error, const char *file, int line, const char *fncall)

tests-clar/refs/unicode.c

@@ -4,17 +4,13 @@ static git_repository *repo;
 
 void test_refs_unicode__initialize(void)
 {
-	cl_fixture_sandbox("testrepo.git");
-
-	cl_git_pass(git_repository_open(&repo, "testrepo.git"));
+	repo = cl_git_sandbox_init("testrepo.git");
 }
 
 void test_refs_unicode__cleanup(void)
 {
-	git_repository_free(repo);
+	cl_git_sandbox_cleanup();
 	repo = NULL;
-
-	cl_fixture_cleanup("testrepo.git");
 }
 
 void test_refs_unicode__create_and_lookup(void)
@@ -23,13 +19,12 @@ void test_refs_unicode__create_and_lookup(void)
 	git_repository *repo2;
 
 	const char *REFNAME = "refs/heads/" "\303\205" "ngstr" "\303\266" "m";
-	const char *REFNAME_DECOMPOSED =
-		"refs/heads/" "A" "\314\212" "ngstro" "\314\210" "m";
 	const char *master = "refs/heads/master";
 
 	/* Create the reference */
 	cl_git_pass(git_reference_lookup(&ref0, repo, master));
-	cl_git_pass(git_reference_create(&ref1, repo, REFNAME, git_reference_target(ref0), 0));
+	cl_git_pass(git_reference_create(
+		&ref1, repo, REFNAME, git_reference_target(ref0), 0));
 	cl_assert_equal_s(REFNAME, git_reference_name(ref1));
 	git_reference_free(ref0);
 
@@ -45,6 +40,8 @@ void test_refs_unicode__create_and_lookup(void)
 #if GIT_USE_ICONV
 	/* Lookup reference by decomposed unicode name */
 
+#define REFNAME_DECOMPOSED "refs/heads/" "A" "\314\212" "ngstro" "\314\210" "m"
+
 	cl_git_pass(git_reference_lookup(&ref2, repo2, REFNAME_DECOMPOSED));
 	cl_assert_equal_i(
 		0, git_oid_cmp(git_reference_target(ref1), git_reference_target(ref2)));