Merge pull request #783 from nulltoken/topic/reinit-coverage

Repo reinit fix + enhanced test coverage

src/repository.c

@@ -602,14 +602,10 @@ void git_repository_set_index(git_repository *repo, git_index *index)
 	GIT_REFCOUNT_INC(index);
 }
 
-static int check_repositoryformatversion(git_repository *repo)
+static int check_repositoryformatversion(git_config *config)
 {
-	git_config *config;
 	int version;
 
-	if (git_repository_config__weakptr(&config, repo) < 0)
-		return -1;
-
 	if (git_config_get_int32(&version, config, "core.repositoryformatversion") < 0)
 		return -1;
 
@@ -623,26 +619,6 @@ static int check_repositoryformatversion(git_repository *repo)
 	return 0;
 }
 
-static int repo_init_reinit(git_repository **repo_out, const char *repository_path, int is_bare)
-{
-	git_repository *repo = NULL;
-
-	GIT_UNUSED(is_bare);
-
-	if (git_repository_open(&repo, repository_path) < 0)
-		return -1;
-
-	if (check_repositoryformatversion(repo) < 0) {
-		git_repository_free(repo);
-		return -1;
-	}
-
-	/* TODO: reinitialize the templates */
-
-	*repo_out = repo;
-	return 0;
-}
-
 static int repo_init_createhead(const char *git_dir)
 {
 	git_buf ref_path = GIT_BUF_INIT;
@@ -717,6 +693,12 @@ static int repo_init_config(const char *git_dir, bool is_bare, bool is_reinit)
 		return -1;
 	}
 
+	if (is_reinit && check_repositoryformatversion(config) < 0) {
+		git_buf_free(&cfg_path);
+		git_config_free(config);
+		return -1;
+	}
+
 	SET_REPO_CONFIG(bool, "core.bare", is_bare);
 	SET_REPO_CONFIG(int32, "core.repositoryformatversion", GIT_REPO_VERSION);
 	SET_REPO_CONFIG(bool, "core.filemode", is_chmod_supported(git_buf_cstr(&cfg_path)));
@@ -850,21 +832,18 @@ int git_repository_init(git_repository **repo_out, const char *path, unsigned is
 	is_reinit = git_path_isdir(repository_path.ptr) && valid_repository_path(&repository_path);
 
 	if (is_reinit) {
-		if (repo_init_reinit(repo_out, repository_path.ptr, is_bare) < 0)
-			goto cleanup;
+		/* TODO: reinitialize the templates */
 
-		result = repo_init_config(repository_path.ptr, is_bare, is_reinit);
-		goto cleanup;
-	}
+		if (repo_init_config(repository_path.ptr, is_bare, is_reinit) < 0)
+			goto cleanup;
 
-	if (repo_init_structure(repository_path.ptr, is_bare) < 0 ||
+	} else if (repo_init_structure(repository_path.ptr, is_bare) < 0 ||
 		repo_init_config(repository_path.ptr, is_bare, is_reinit) < 0 || 
-		repo_init_createhead(repository_path.ptr) < 0 ||
-		git_repository_open(repo_out, repository_path.ptr) < 0) {
+		repo_init_createhead(repository_path.ptr) < 0) {
 		goto cleanup;
 	}
 
-	result = 0;
+	result = git_repository_open(repo_out, repository_path.ptr);
 
 cleanup:
 	git_buf_free(&repository_path);

tests-clar/repo/init.c

@@ -170,10 +170,21 @@ static void assert_config_entry_on_init_bytype(const char *config_key, int expec
 {
 	git_config *config;
 	int current_value;
+	git_buf repo_path = GIT_BUF_INIT;
 
 	cl_set_cleanup(&cleanup_repository, "config_entry");
-	
-	cl_git_pass(git_repository_init(&_repo, "config_entry/test.git", is_bare));
+
+	cl_git_pass(git_buf_puts(&repo_path, "config_entry/test."));
+
+	if (!is_bare)
+		cl_git_pass(git_buf_puts(&repo_path, "non."));
+
+	cl_git_pass(git_buf_puts(&repo_path, "bare.git"));
+
+	cl_git_pass(git_repository_init(&_repo, git_buf_cstr(&repo_path), is_bare));
+
+	git_buf_free(&repo_path);
+
 	git_repository_config(&config, _repo);
 
 	if (expected_value >= 0) {
@@ -223,7 +234,7 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void)
 	int current_value;
 
 	/* Init a new repo */
-	test_repo_init__detect_ignorecase();
+	cl_git_pass(git_repository_init(&_repo, "not.overwrite.git", 1));
 
 	/* Change the "core.ignorecase" config value to something unlikely */
 	git_repository_config(&config, _repo);
@@ -232,7 +243,7 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void)
 	git_repository_free(_repo);
 
 	/* Reinit the repository */
-	cl_git_pass(git_repository_init(&_repo, "config_entry/test.git", 1));
+	cl_git_pass(git_repository_init(&_repo, "not.overwrite.git", 1));
 	git_repository_config(&config, _repo);
 
 	/* Ensure the "core.ignorecase" config value hasn't been updated */
@@ -242,6 +253,37 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void)
 	git_config_free(config);
 }
 
+void test_repo_init__reinit_overwrites_filemode(void)
+{
+    git_config *config;
+    int expected, current_value;
+
+#ifdef GIT_WIN32
+    expected = false;
+#else
+    expected = true;
+#endif
+
+    /* Init a new repo */
+    cl_git_pass(git_repository_init(&_repo, "overwrite.git", 1));
+
+    /* Change the "core.filemode" config value to something unlikely */
+    git_repository_config(&config, _repo);
+    git_config_set_bool(config, "core.filemode", !expected);
+    git_config_free(config);
+    git_repository_free(_repo);
+
+    /* Reinit the repository */
+    cl_git_pass(git_repository_init(&_repo, "overwrite.git", 1));
+    git_repository_config(&config, _repo);
+
+    /* Ensure the "core.filemode" config value has been reset */
+    cl_git_pass(git_config_get_bool(&current_value, config, "core.filemode"));
+    cl_assert_equal_i(expected, current_value);
+
+    git_config_free(config);
+}
+
 void test_repo_init__sets_logAllRefUpdates_according_to_type_of_repository(void)
 {
 	assert_config_entry_on_init_bytype("core.logallrefupdates", GIT_ENOTFOUND, true);