Merge branch 'ps/setup-enforce-repo-passed-to-create-repository-has-no-state'

The repository initialization sequence has been refactored to treat
the repository object passed to create_repository() purely as an
out-parameter.

* ps/setup-enforce-repo-passed-to-create-repository-has-no-state:
  setup: enforce that passed-in repo does not carry relevant state
  repository: adapt `repo_clear()` to fully reset the repository
  builtin/clone: don't apply "core.sharedRepository" to leading dirs
  builtin/init: move handling of "core.sharedRepository" into "setup.c"
  builtin/init: refactor messy creation of leading directories
  path: introduce `safe_create_leading_directories_no_share_const()`
  path: drop useless `safe_create_leading_directories_1()`

builtin/clone.c

@@ -1134,7 +1134,7 @@ int cmd_clone(int argc,
 	sigchain_push_common(remove_junk_on_signal);
 
 	if (!option_bare) {
-		if (safe_create_leading_directories_const(the_repository, work_tree) < 0)
+		if (safe_create_leading_directories_no_share_const(work_tree) < 0)
 			die_errno(_("could not create leading directories of '%s'"),
 				  work_tree);
 		if (dest_exists)
@@ -1154,7 +1154,7 @@ int cmd_clone(int argc,
 			junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
 		junk_git_dir = git_dir;
 	}
-	if (safe_create_leading_directories_const(the_repository, git_dir) < 0)
+	if (safe_create_leading_directories_no_share_const(git_dir) < 0)
 		die(_("could not create leading directories of '%s'"), git_dir);
 
 	if (0 <= option_verbosity) {

builtin/init-db.c

@@ -131,15 +131,7 @@ int cmd_init_db(int argc,
 	retry:
 		if (chdir(argv[0]) < 0) {
 			if (!mkdir_tried) {
-				int saved;
-				/*
-				 * At this point we haven't read any configuration,
-				 * and we know shared_repository should always be 0;
-				 * but just in case we play safe.
-				 */
-				saved = repo_settings_get_shared_repository(the_repository);
-				repo_settings_set_shared_repository(the_repository, 0);
-				switch (safe_create_leading_directories_const(the_repository, argv[0])) {
+				switch (safe_create_leading_directories_no_share_const(argv[0])) {
 				case SCLD_OK:
 				case SCLD_PERMS:
 					break;
@@ -150,7 +142,7 @@ int cmd_init_db(int argc,
 					die_errno(_("cannot mkdir %s"), argv[0]);
 					break;
 				}
-				repo_settings_set_shared_repository(the_repository, saved);
+
 				if (mkdir(argv[0], 0777) < 0)
 					die_errno(_("cannot mkdir %s"), argv[0]);
 				mkdir_tried = 1;
@@ -173,9 +165,6 @@ int cmd_init_db(int argc,
 			die(_("unknown hash algorithm '%s'"), object_format);
 	}
 
-	if (init_shared_repository != -1)
-		repo_settings_set_shared_repository(the_repository, init_shared_repository);
-
 	/*
 	 * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR
 	 * without --bare.  Catch the error early.

path.c

@@ -829,8 +829,8 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path)
 	return adjust_shared_perm(repo, path);
 }
 
-static enum scld_error safe_create_leading_directories_1(struct repository *repo,
-							 char *path)
+enum scld_error safe_create_leading_directories(struct repository *repo,
+						char *path)
 {
 	char *next_component = path + offset_1st_component(path);
 	enum scld_error ret = SCLD_OK;
@@ -884,15 +884,14 @@ static enum scld_error safe_create_leading_directories_1(struct repository *repo
 	return ret;
 }
 
-enum scld_error safe_create_leading_directories(struct repository *repo,
-						char *path)
-{
-	return safe_create_leading_directories_1(repo, path);
-}
-
 enum scld_error safe_create_leading_directories_no_share(char *path)
 {
-	return safe_create_leading_directories_1(NULL, path);
+	return safe_create_leading_directories(NULL, path);
+}
+
+enum scld_error safe_create_leading_directories_no_share_const(const char *path)
+{
+	return safe_create_leading_directories_const(NULL, path);
 }
 
 enum scld_error safe_create_leading_directories_const(struct repository *repo,

path.h

@@ -235,14 +235,11 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path);
  * race, callers might want to try invoking the function again when it
  * returns SCLD_VANISHED.
  *
- * safe_create_leading_directories() temporarily changes path while it
- * is working but restores it before returning.
- * safe_create_leading_directories_const() doesn't modify path, even
- * temporarily. Both these variants adjust the permissions of the
- * created directories to honor core.sharedRepository, so they are best
- * suited for files inside the git dir. For working tree files, use
- * safe_create_leading_directories_no_share() instead, as it ignores
- * the core.sharedRepository setting.
+ * The default variants honor "core.sharedRepository" and temporarily modify
+ * `path`. Note that this configuration should be honored for all files in the
+ * git directory. The `no_share()` variants ignore "core.sharedRepository",
+ * and should be used for working tree files. The `const()` variants do not
+ * modify `path`.
  */
 enum scld_error {
 	SCLD_OK = 0,
@@ -255,6 +252,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p
 enum scld_error safe_create_leading_directories_const(struct repository *repo,
 						      const char *path);
 enum scld_error safe_create_leading_directories_no_share(char *path);
+enum scld_error safe_create_leading_directories_no_share_const(const char *path);
 
 /*
  * Create a file, potentially creating its leading directories in case they

repository.c

@@ -375,60 +375,57 @@ void repo_clear(struct repository *repo)
 	struct hashmap_iter iter;
 	struct strmap_entry *e;
 
-	FREE_AND_NULL(repo->gitdir);
-	FREE_AND_NULL(repo->commondir);
-	FREE_AND_NULL(repo->prefix);
-	FREE_AND_NULL(repo->graft_file);
-	FREE_AND_NULL(repo->index_file);
-	FREE_AND_NULL(repo->worktree);
-	FREE_AND_NULL(repo->submodule_prefix);
-	FREE_AND_NULL(repo->ref_storage_payload);
+	free(repo->gitdir);
+	free(repo->commondir);
+	free(repo->prefix);
+	free(repo->graft_file);
+	free(repo->index_file);
+	free(repo->worktree);
+	free(repo->submodule_prefix);
+	free(repo->ref_storage_payload);
 
 	odb_free(repo->objects);
-	repo->objects = NULL;
 
 	if (repo->parsed_objects)
 		parsed_object_pool_clear(repo->parsed_objects);
-	FREE_AND_NULL(repo->parsed_objects);
+	free(repo->parsed_objects);
 
 	repo_settings_clear(repo);
 	repo_config_values_clear(&repo->config_values_private_);
 
 	if (repo->config) {
 		git_configset_clear(repo->config);
-		FREE_AND_NULL(repo->config);
+		free(repo->config);
 	}
 
-	if (repo->submodule_cache) {
+	if (repo->submodule_cache)
 		submodule_cache_free(repo->submodule_cache);
-		repo->submodule_cache = NULL;
-	}
 
 	if (repo->index) {
 		discard_index(repo->index);
-		FREE_AND_NULL(repo->index);
+		free(repo->index);
 	}
 
 	if (repo->hook_config_cache) {
 		hook_cache_clear(repo->hook_config_cache);
-		FREE_AND_NULL(repo->hook_config_cache);
+		free(repo->hook_config_cache);
 	}
 	strmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */
 	string_list_clear(&repo->disabled_events, 0);
 
 	if (repo->promisor_remote_config) {
 		promisor_remote_clear(repo->promisor_remote_config);
-		FREE_AND_NULL(repo->promisor_remote_config);
+		free(repo->promisor_remote_config);
 	}
 
 	if (repo->remote_state) {
 		remote_state_clear(repo->remote_state);
-		FREE_AND_NULL(repo->remote_state);
+		free(repo->remote_state);
 	}
 
 	if (repo->refs_private) {
 		ref_store_release(repo->refs_private);
-		FREE_AND_NULL(repo->refs_private);
+		free(repo->refs_private);
 	}
 
 	strmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)
@@ -440,6 +437,8 @@ void repo_clear(struct repository *repo)
 	strmap_clear(&repo->worktree_ref_stores, 1);
 
 	repo_clear_path_cache(&repo->cached_paths);
+
+	memset(repo, 0, sizeof(*repo));
 }
 
 int repo_read_index(struct repository *repo)

repository.h

@@ -259,6 +259,7 @@ void repo_set_ref_storage_format(struct repository *repo,
 void initialize_repository(struct repository *repo);
 RESULT_MUST_BE_USED
 int repo_init(struct repository *r, const char *gitdir, const char *worktree);
+void repo_clear(struct repository *repo);
 
 /*
  * Initialize the repository 'subrepo' as the submodule at the given path. If
@@ -274,7 +275,6 @@ int repo_submodule_init(struct repository *subrepo,
 			struct repository *superproject,
 			const char *path,
 			const struct object_id *treeish_name);
-void repo_clear(struct repository *repo);
 
 /*
  * Populates the repository's index from its index_file, an index struct will

setup.c

@@ -2927,6 +2927,9 @@ void create_repository(struct repository *repo,
 	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
 	struct strbuf err = STRBUF_INIT;
 
+	repo_clear(repo);
+	initialize_repository(repo);
+
 	if (real_git_dir) {
 		struct stat st;
 
@@ -2965,6 +2968,9 @@ void create_repository(struct repository *repo,
 	 */
 	repo_config(repo, git_default_core_config, NULL);
 
+	if (init_shared_repository != -1)
+		repo_settings_set_shared_repository(repo, init_shared_repository);
+
 	safe_create_dir(repo, git_dir, 0);
 
 	if (!reinit_ok)

t/t1301-shared-repo.sh

@@ -210,4 +210,46 @@ test_expect_success POSIXPERM 'template can set core.sharedrepository' '
 	test_cmp expect actual
 '
 
+test_expect_success POSIXPERM 'init does not apply core.sharedRepository to leading directories' '
+	test_config_global core.sharedRepository 0666 &&
+	umask 0077 &&
+	test_when_finished "rm -rf dst" &&
+	git init --bare dst/with/leading/dirs &&
+	cat >expect <<-\EOF &&
+	drwx------
+	drwx------
+	drwx------
+	drwxrwxrwx
+	EOF
+	{
+		test_modebits dst &&
+		test_modebits dst/with &&
+		test_modebits dst/with/leading &&
+		test_modebits dst/with/leading/dirs
+	} >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success POSIXPERM 'clone does not apply core.sharedRepository to leading directories' '
+	test_config_global core.sharedRepository 0666 &&
+	umask 0077 &&
+	test_when_finished "rm -rf source dst" &&
+	git init source &&
+	test_commit -C source initial &&
+	git clone --bare source dst/with/leading/dirs &&
+	cat >expect <<-\EOF &&
+	drwx------
+	drwx------
+	drwx------
+	drwxrwxrwx
+	EOF
+	{
+		test_modebits dst &&
+		test_modebits dst/with &&
+		test_modebits dst/with/leading &&
+		test_modebits dst/with/leading/dirs
+	} >actual &&
+	test_cmp expect actual
+'
+
 test_done