{"thread":{"id":"66382","subject":"[PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state","startedAt":"2026-09-24T09:19:37Z","lastAt":"2026-09-28T15:59:14Z","messageCount":32,"participants":["Patrick Steinhardt","Kaartic Sivaraam","Karthik Nayak","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"553153","messageId":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","threadId":"66382","inReplyTo":null,"subject":"[PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:18Z","receivedAt":"2026-09-24T09:19:37Z","isPatch":true,"body":"Hi,\n\nwhen creating a new repository via `create_repository()` we pass in a\nrepository. This repository is acting as an in/out parameter: the caller\nexpects that it will be fully configured after the call, but the\nfunction itself also uses some information from the passed-in repository\nto figure out how exactly we want to create it.\n\nThis interface is quite confusing, as it's not obvious at all what\nconfiguration of the repository is relevant. We have thus over a couple\nof patch series reduced the use of the parameter as in/out parameter. So\nnow, the only piece of info that is still being propagated via the repo\nis \"core.sharedRepository\".\n\nThis patch series cleans up that last remaining part so that the repo\nbecomes purely an out-parameter. To ensure that this is the case we also\nstart to `repo_clear()` it as a first step.\n\nBesides simplifying the interface, the intent is also to go further into\nthe direction of unifying repository initialization in a follow-up patch\nseries.\n\nThe series is built on top of 0f8e75abeb (Revert \"Merge branch\n'en/no-amend-during-conflicts'\", 2026-09-23) with\nps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the\nability to write alternates, 2026-09-10) merged into it.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n path: drop useless `safe_create_leading_directories_1()`\n path: introduce `safe_create_leading_directories_no_share_const()`\n builtin/init: refactor messy creation of leading directories\n builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n repository: adapt `repo_clear()` to fully reset the repository\n setup: enforce that passed-in repo does not carry relevant state\n\n builtin/clone.c | 4 ++--\n builtin/init-db.c | 15 ++-------------\n path.c | 13 ++++++-------\n path.h | 1 +\n repository.c | 37 ++++++++++++++++++-------------------\n repository.h | 2 +-\n setup.c | 6 ++++++\n t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 78 insertions(+), 42 deletions(-)\n\n\n---\nbase-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404\nchange-id: 20260916-pks-create-repository-stateless-f0ca03cca689\n\n"},{"id":"553154","messageId":"20260924-pks-create-repository-stateless-v1-1-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 1/7] path: drop useless `safe_create_leading_directories_1()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:19Z","receivedAt":"2026-09-24T09:19:38Z","isPatch":true,"body":"The function `safe_create_leading_directories_1()` is being called by\nboth `safe_create_leading_directories()` and its `_no_share()` variant.\nIt is ultimately the exact same as the former of these functions though\nand is thus quite useless.\n\nDrop the function and inline it into its callsites directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n path.c | 12 +++---------\n 1 file changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex c3a709a928..69b06c9464 100644\n--- a/path.c\n+++ b/path.c\n@@ -829,8 +829,8 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path)\n \treturn adjust_shared_perm(repo, path);\n }\n \n-static enum scld_error safe_create_leading_directories_1(struct repository *repo,\n-\t\t\t\t\t\t\t char *path)\n+enum scld_error safe_create_leading_directories(struct repository *repo,\n+\t\t\t\t\t\tchar *path)\n {\n \tchar *next_component = path + offset_1st_component(path);\n \tenum scld_error ret = SCLD_OK;\n@@ -884,15 +884,9 @@ static enum scld_error safe_create_leading_directories_1(struct repository *repo\n \treturn ret;\n }\n \n-enum scld_error safe_create_leading_directories(struct repository *repo,\n-\t\t\t\t\t\tchar *path)\n-{\n-\treturn safe_create_leading_directories_1(repo, path);\n-}\n-\n enum scld_error safe_create_leading_directories_no_share(char *path)\n {\n-\treturn safe_create_leading_directories_1(NULL, path);\n+\treturn safe_create_leading_directories(NULL, path);\n }\n \n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553155","messageId":"20260924-pks-create-repository-stateless-v1-2-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:20Z","receivedAt":"2026-09-24T09:19:41Z","isPatch":true,"body":"The `safe_create_leading_directories()` family of functions modify the\npassed-in path so that we can obtain all the different segments of the\npath. This is done by overwriting path separators with a NUL byte for\nevery component. While we ultimately restore the original string, the\nconsequence is that the caller needs to pass a non-constant string.\n\nWhile it would be trivial to modify the function to not modify the path\nin-place anymore, the intent of this whole mechanism is to save an\nallocation. It's quite dubious whether this optimization really matters\nin the grand scheme of things, but here we are.\n\nIn any case, we provide a `_const()` variant that handles the case where\nthe caller only has a string constant. But we lack such a variant for\nthe `safe_create_leading_directories_no_share()` function, and we're\nabout to add a couple of callers that would need it.\n\nAdd this helper function.\n\nSigned-off-by: Patrick Steinhardt \n---\n path.c | 5 +++++\n path.h | 1 +\n 2 files changed, 6 insertions(+)\n\ndiff --git a/path.c b/path.c\nindex 69b06c9464..f8f5a9dd28 100644\n--- a/path.c\n+++ b/path.c\n@@ -889,6 +889,11 @@ enum scld_error safe_create_leading_directories_no_share(char *path)\n \treturn safe_create_leading_directories(NULL, path);\n }\n \n+enum scld_error safe_create_leading_directories_no_share_const(const char *path)\n+{\n+\treturn safe_create_leading_directories_const(NULL, path);\n+}\n+\n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n \t\t\t\t\t\t const char *path)\n {\ndiff --git a/path.h b/path.h\nindex 7e7408dd05..e2d62c4978 100644\n--- a/path.h\n+++ b/path.h\n@@ -254,6 +254,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p\n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n \t\t\t\t\t\t const char *path);\n enum scld_error safe_create_leading_directories_no_share(char *path);\n+enum scld_error safe_create_leading_directories_no_share_const(const char *path);\n \n /*\n * Create a file, potentially creating its leading directories in case they\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553156","messageId":"20260924-pks-create-repository-stateless-v1-3-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 3/7] builtin/init: refactor messy creation of leading directories","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:21Z","receivedAt":"2026-09-24T09:19:43Z","isPatch":true,"body":"When creating a new repository via git-init(1) we potentially have to\ncreate any leading directories via `safe_create_leading_directories()`.\nThis function optionally knows to handle \"core.sharedRepository\" to\nadjust the permissions of the created directories.\n\nThe value of that setting is taken from the passed-in repository. When\ncreating a new repository we don't want to honor it though, so we\npainstakingly:\n\n 1. Save the current value of that setting.\n\n 2. Set it to 0.\n\n 3. Create the directory with `safe_create_leading_directories()`. This\n has the effect that `adjust_shared_perm()` will exit early and not\n adjust permissions.\n\n 4. Restore the old value.\n\nThis is extremely awkward, but it achieves the desired effect that we\nignore the configuration. There's a significantly easier way to achieve\nthis though: we can just call the `_no_share()` variant, whose entire\npurpose it is to ignore \"core.sharedRepository\".\n\nRefactor the code to use that variant accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/init-db.c | 12 ++----------\n 1 file changed, 2 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 5c22eae2f3..e45268f1ff 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -131,15 +131,7 @@ int cmd_init_db(int argc,\n \tretry:\n \t\tif (chdir(argv[0]) < 0) {\n \t\t\tif (!mkdir_tried) {\n-\t\t\t\tint saved;\n-\t\t\t\t/*\n-\t\t\t\t * At this point we haven't read any configuration,\n-\t\t\t\t * and we know shared_repository should always be 0;\n-\t\t\t\t * but just in case we play safe.\n-\t\t\t\t */\n-\t\t\t\tsaved = repo_settings_get_shared_repository(the_repository);\n-\t\t\t\trepo_settings_set_shared_repository(the_repository, 0);\n-\t\t\t\tswitch (safe_create_leading_directories_const(the_repository, argv[0])) {\n+\t\t\t\tswitch (safe_create_leading_directories_no_share_const(argv[0])) {\n \t\t\t\tcase SCLD_OK:\n \t\t\t\tcase SCLD_PERMS:\n \t\t\t\t\tbreak;\n@@ -150,7 +142,7 @@ int cmd_init_db(int argc,\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\trepo_settings_set_shared_repository(the_repository, saved);\n+\n \t\t\t\tif (mkdir(argv[0], 0777) < 0)\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\tmkdir_tried = 1;\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553157","messageId":"20260924-pks-create-repository-stateless-v1-4-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 4/7] builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:22Z","receivedAt":"2026-09-24T09:19:45Z","isPatch":true,"body":"When initializing a new repository via git-init(1) we know to honor\n\"core.sharedRepository\" and adjust permissions of newly created files\naccordingly. The way we propagate that setting is quite awkward though,\nas we have to set it on the repository that we pass into\n`create_repository()` and pass it as a parameter. This is because there\nare two different scopes in play here:\n\n - We need to apply it to the repository so that creating the\n repository's directory uses the correct permissions.\n\n - We need to reapply it to the repository after we have created\n default files so that we know to override any configuration that we\n have read from the new repository's configuration.\n\nThe effect of this though is that the repository works as an in-out\nparameter, which is quite awkward.\n\nRefactor the code so that the caller only needs to pass the value.\nStarting with this change, the passed-in repository can essentially be\ncompletely blank as it doesn't carry any state anymore that we'd care\nabout in `create_repository()`.\n\nNote that this change in theory also impacts the other caller of\n`create_repository()` that exists in git-clone(1). But that caller\nalready passes `-1` as a value for this parameter, and neither does that\ncaller modify the repository it passes. So there shouldn't be any change\nin behaviour here.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/init-db.c | 3 ---\n setup.c | 3 +++\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex e45268f1ff..34215bbf18 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -171,9 +171,6 @@ int cmd_init_db(int argc,\n \t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_format);\n \t}\n \n-\tif (init_shared_repository != -1)\n-\t\trepo_settings_set_shared_repository(the_repository, init_shared_repository);\n-\n \t/*\n \t * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR\n \t * without --bare. Catch the error early.\ndiff --git a/setup.c b/setup.c\nindex f335111d1e..0d0a4abbe6 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2896,6 +2896,9 @@ void create_repository(struct repository *repo,\n \t */\n \trepo_config(repo, git_default_core_config, NULL);\n \n+\tif (init_shared_repository != -1)\n+\t\trepo_settings_set_shared_repository(repo, init_shared_repository);\n+\n \tsafe_create_dir(repo, git_dir, 0);\n \n \tif (!reinit_ok)\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553158","messageId":"20260924-pks-create-repository-stateless-v1-5-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 5/7] builtin/clone: don't apply \"core.sharedRepository\" to leading dirs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:23Z","receivedAt":"2026-09-24T09:19:48Z","isPatch":true,"body":"When creating a repository via git-clone(1) we create leading\ndirectories with `safe_create_leading_directories()`. We have adapted\ngit-init(1) in a preceding commit to instead use the variant of\nthis function that doesn't honor \"core.sharedRepository\". In that\nsubcommand it didn't have an effect though as we explicitly unset the\nvalue of that configuration anyway, so we never honored that config.\n\nIn git-clone(1) it's a bit of a different thing though: while the\nrepository isn't initialized at the point in time where we call the\nfunction, we didn't explicitly unset the value. Consequently we _do_\nhonor the configuration here, but when it's configured in global- or\nsystem-level scope.\n\nThis divergence doesn't seem to be intentional -- I cannot think of any\ngood reason why git-init(1) and git-clone(1) should have divergent\nbehaviour here.\n\nAdapt git-clone(1) to work the same as git-init(1) by also using the\n`no_share()` variants to create leading directories. Add tests for both\ncommands.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/clone.c | 4 ++--\n t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b14264c33a..e72f8aa325 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1133,7 +1133,7 @@ int cmd_clone(int argc,\n \tsigchain_push_common(remove_junk_on_signal);\n \n \tif (!option_bare) {\n-\t\tif (safe_create_leading_directories_const(the_repository, work_tree) < 0)\n+\t\tif (safe_create_leading_directories_no_share_const(work_tree) < 0)\n \t\t\tdie_errno(_(\"could not create leading directories of '%s'\"),\n \t\t\t\t work_tree);\n \t\tif (dest_exists)\n@@ -1153,7 +1153,7 @@ int cmd_clone(int argc,\n \t\t\tjunk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n \t\tjunk_git_dir = git_dir;\n \t}\n-\tif (safe_create_leading_directories_const(the_repository, git_dir) < 0)\n+\tif (safe_create_leading_directories_no_share_const(git_dir) < 0)\n \t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n \n \tif (0 <= option_verbosity) {\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 0e0d07a1a1..3bc4bdb038 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -210,4 +210,46 @@ test_expect_success POSIXPERM 'template can set core.sharedrepository' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success POSIXPERM 'init does not apply core.sharedRepository to leading directories' '\n+\ttest_config_global core.sharedRepository 0666 &&\n+\tumask 0077 &&\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit init --bare dst/with/leading/dirs &&\n+\tcat >expect <<-\\EOF &&\n+\tdrwx------\n+\tdrwx------\n+\tdrwx------\n+\tdrwxrwxrwx\n+\tEOF\n+\t{\n+\t\ttest_modebits dst &&\n+\t\ttest_modebits dst/with &&\n+\t\ttest_modebits dst/with/leading &&\n+\t\ttest_modebits dst/with/leading/dirs\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success POSIXPERM 'clone does not apply core.sharedRepository to leading directories' '\n+\ttest_config_global core.sharedRepository 0666 &&\n+\tumask 0077 &&\n+\ttest_when_finished \"rm -rf source dst\" &&\n+\tgit init source &&\n+\ttest_commit -C source initial &&\n+\tgit clone --bare source dst/with/leading/dirs &&\n+\tcat >expect <<-\\EOF &&\n+\tdrwx------\n+\tdrwx------\n+\tdrwx------\n+\tdrwxrwxrwx\n+\tEOF\n+\t{\n+\t\ttest_modebits dst &&\n+\t\ttest_modebits dst/with &&\n+\t\ttest_modebits dst/with/leading &&\n+\t\ttest_modebits dst/with/leading/dirs\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553159","messageId":"20260924-pks-create-repository-stateless-v1-6-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:24Z","receivedAt":"2026-09-24T09:19:50Z","isPatch":true,"body":"The function `repo_clear()` can be used to clear a repository's state.\nThe way it's written though it's quite easy for it to accidentally leak\nsome state because we don't make sure to clear the whole structure.\n\nRefactor the function to set the whole repository to all-zeroes to avoid\nany kind of leaking state. While at it, make it a bit more robust when\ncalled on an already-blank repository.\n\nSigned-off-by: Patrick Steinhardt \n---\n repository.c | 37 ++++++++++++++++++-------------------\n repository.h | 2 +-\n 2 files changed, 19 insertions(+), 20 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex b857e1c580..e67ff00550 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -374,60 +374,57 @@ void repo_clear(struct repository *repo)\n \tstruct hashmap_iter iter;\n \tstruct strmap_entry *e;\n \n-\tFREE_AND_NULL(repo->gitdir);\n-\tFREE_AND_NULL(repo->commondir);\n-\tFREE_AND_NULL(repo->prefix);\n-\tFREE_AND_NULL(repo->graft_file);\n-\tFREE_AND_NULL(repo->index_file);\n-\tFREE_AND_NULL(repo->worktree);\n-\tFREE_AND_NULL(repo->submodule_prefix);\n-\tFREE_AND_NULL(repo->ref_storage_payload);\n+\tfree(repo->gitdir);\n+\tfree(repo->commondir);\n+\tfree(repo->prefix);\n+\tfree(repo->graft_file);\n+\tfree(repo->index_file);\n+\tfree(repo->worktree);\n+\tfree(repo->submodule_prefix);\n+\tfree(repo->ref_storage_payload);\n \n \todb_free(repo->objects);\n-\trepo->objects = NULL;\n \n \tif (repo->parsed_objects)\n \t\tparsed_object_pool_clear(repo->parsed_objects);\n-\tFREE_AND_NULL(repo->parsed_objects);\n+\tfree(repo->parsed_objects);\n \n \trepo_settings_clear(repo);\n \trepo_config_values_clear(&repo->config_values_private_);\n \n \tif (repo->config) {\n \t\tgit_configset_clear(repo->config);\n-\t\tFREE_AND_NULL(repo->config);\n+\t\tfree(repo->config);\n \t}\n \n-\tif (repo->submodule_cache) {\n+\tif (repo->submodule_cache)\n \t\tsubmodule_cache_free(repo->submodule_cache);\n-\t\trepo->submodule_cache = NULL;\n-\t}\n \n \tif (repo->index) {\n \t\tdiscard_index(repo->index);\n-\t\tFREE_AND_NULL(repo->index);\n+\t\tfree(repo->index);\n \t}\n \n \tif (repo->hook_config_cache) {\n \t\thook_cache_clear(repo->hook_config_cache);\n-\t\tFREE_AND_NULL(repo->hook_config_cache);\n+\t\tfree(repo->hook_config_cache);\n \t}\n \tstrmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */\n \tstring_list_clear(&repo->disabled_events, 0);\n \n \tif (repo->promisor_remote_config) {\n \t\tpromisor_remote_clear(repo->promisor_remote_config);\n-\t\tFREE_AND_NULL(repo->promisor_remote_config);\n+\t\tfree(repo->promisor_remote_config);\n \t}\n \n \tif (repo->remote_state) {\n \t\tremote_state_clear(repo->remote_state);\n-\t\tFREE_AND_NULL(repo->remote_state);\n+\t\tfree(repo->remote_state);\n \t}\n \n \tif (repo->refs_private) {\n \t\tref_store_release(repo->refs_private);\n-\t\tFREE_AND_NULL(repo->refs_private);\n+\t\tfree(repo->refs_private);\n \t}\n \n \tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n@@ -439,6 +436,8 @@ void repo_clear(struct repository *repo)\n \tstrmap_clear(&repo->worktree_ref_stores, 1);\n \n \trepo_clear_path_cache(&repo->cached_paths);\n+\n+\tmemset(repo, 0, sizeof(*repo));\n }\n \n int repo_read_index(struct repository *repo)\ndiff --git a/repository.h b/repository.h\nindex 11f5c2ed10..2a348012e8 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo,\n void initialize_repository(struct repository *repo);\n RESULT_MUST_BE_USED\n int repo_init(struct repository *r, const char *gitdir, const char *worktree);\n+void repo_clear(struct repository *repo);\n \n /*\n * Initialize the repository 'subrepo' as the submodule at the given path. If\n@@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo,\n \t\t\tstruct repository *superproject,\n \t\t\tconst char *path,\n \t\t\tconst struct object_id *treeish_name);\n-void repo_clear(struct repository *repo);\n \n /*\n * Populates the repository's index from its index_file, an index struct will\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553160","messageId":"20260924-pks-create-repository-stateless-v1-7-11499557cf31@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH 7/7] setup: enforce that passed-in repo does not carry relevant state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-24T09:19:25Z","receivedAt":"2026-09-24T09:19:53Z","isPatch":true,"body":"In the preceding patches we have refactored `create_repository()` so\nthat the passed-in repository is not used anymore to propagate any kind\nof state. This was done so that the parameter doesn't act like an in-out\nparameter, but only as an out parameter that we initialize with the\nstate of the newly created repository.\n\nWe don't enforce though that the repository _cannot_ be used to\npropagate state anymore, which makes it quite easy for state to sneak in\nat a later point again.\n\nIdeally, we'd do that by having the function create a newly allocated\nrepository instead of taking a repository as input. But unfortunately,\nthat does not work because we end up calling `repo_config_values()` when\nwe create the \"files\" ref database, and that function requires that the\npassed-in repository is `the_repository`.\n\nInstead, call `repo_clear()` at the beginning of the function, which\ngives us a clean slate.\n\nSigned-off-by: Patrick Steinhardt \n---\n setup.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/setup.c b/setup.c\nindex 0d0a4abbe6..fa39219d6a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2858,6 +2858,9 @@ void create_repository(struct repository *repo,\n \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n \n+\trepo_clear(repo);\n+\tinitialize_repository(repo);\n+\n \tif (real_git_dir) {\n \t\tstruct stat st;\n \n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553318","messageId":"4770b19f-9a8d-4a4c-8cc6-745aa2868b94@gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-2-11499557cf31@pks.im","subject":"Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T19:49:42Z","receivedAt":"2026-09-25T19:49:47Z","isPatch":true,"body":"On 9/24/26 14:49, Patrick Steinhardt wrote:\n> \n> diff --git a/path.h b/path.h\n> index 7e7408dd05..e2d62c4978 100644\n> --- a/path.h\n> +++ b/path.h\n> @@ -254,6 +254,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p\n> enum scld_error safe_create_leading_directories_const(struct repository *repo,\n> \t\t\t\t\t\t const char *path);\n> enum scld_error safe_create_leading_directories_no_share(char *path);\n> +enum scld_error safe_create_leading_directories_no_share_const(const char *path);\n> \n\nnit: All other variants are mentioned in the documentation blurb just \nabove the declarations. Would it also be worth mentioning this new one \nthere?\n\n-- \nSivaraam\n\n"},{"id":"553319","messageId":"63798ed5-3fea-4938-8b0b-910d5f1c7bf0@gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-3-11499557cf31@pks.im","subject":"Re: [PATCH 3/7] builtin/init: refactor messy creation of leading directories","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T20:21:54Z","receivedAt":"2026-09-25T20:21:59Z","isPatch":true,"body":"On 9/24/26 14:49, Patrick Steinhardt wrote:\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index 5c22eae2f3..e45268f1ff 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -131,15 +131,7 @@ int cmd_init_db(int argc,\n> \tretry:\n> \t\tif (chdir(argv[0]) < 0) {\n> \t\t\tif (!mkdir_tried) {\n> -\t\t\t\tint saved;\n> -\t\t\t\t/*\n> -\t\t\t\t * At this point we haven't read any configuration,\n> -\t\t\t\t * and we know shared_repository should always be 0;\n> -\t\t\t\t * but just in case we play safe.\n> -\t\t\t\t */\n> -\t\t\t\tsaved = repo_settings_get_shared_repository(the_repository);\n> -\t\t\t\trepo_settings_set_shared_repository(the_repository, 0);\n> -\t\t\t\tswitch (safe_create_leading_directories_const(the_repository, argv[0])) {\n> +\t\t\t\tswitch (safe_create_leading_directories_no_share_const(argv[0])) {\n> \t\t\t\tcase SCLD_OK:\n> \t\t\t\tcase SCLD_PERMS:\n> \t\t\t\t\tbreak;\n> @@ -150,7 +142,7 @@ int cmd_init_db(int argc,\n> \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n> \t\t\t\t\tbreak;\n> \t\t\t\t}\n> -\t\t\t\trepo_settings_set_shared_repository(the_repository, saved);\n> +\n\nEven though this patch does not aim to do so, we lost a bunch of \n'the_repository' references with this change which is nice.\n\nThe patch also looks good to me.\n\n-- \nSivaraam\n\n"},{"id":"553329","messageId":"4e26b51a-f811-4548-97e0-6570680efa13@gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-4-11499557cf31@pks.im","subject":"Re: [PATCH 4/7] builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T20:54:15Z","receivedAt":"2026-09-25T20:54:20Z","isPatch":true,"body":"On 9/24/26 14:49, Patrick Steinhardt wrote:\n >\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index e45268f1ff..34215bbf18 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -171,9 +171,6 @@ int cmd_init_db(int argc,\n> \t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_format);\n> \t}\n> \n> -\tif (init_shared_repository != -1)\n> -\t\trepo_settings_set_shared_repository(the_repository, init_shared_repository);\n> -\n > \t/*\n > \t * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR\n > \t * without --bare. Catch the error early.\n\nI was wondering if there'll be any function that between here and the \ncreate_repository call that may use the_repository. I could not find any \nfrom my reading of the code, though. So, this looks good to me.\n\n-\nSivaraam\n\n"},{"id":"553332","messageId":"d783ea21-12db-4bea-82f3-a784c428b712@gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"Re: [PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T21:08:53Z","receivedAt":"2026-09-25T21:08:58Z","isPatch":true,"body":"On 9/24/26 14:49, Patrick Steinhardt wrote:\n> \n> when creating a new repository via `create_repository()` we pass in a\n> repository. This repository is acting as an in/out parameter: the caller\n> expects that it will be fully configured after the call, but the\n> function itself also uses some information from the passed-in repository\n> to figure out how exactly we want to create it.\n> \n> This interface is quite confusing, as it's not obvious at all what\n> configuration of the repository is relevant. We have thus over a couple\n> of patch series reduced the use of the parameter as in/out parameter. So\n> now, the only piece of info that is still being propagated via the repo\n> is \"core.sharedRepository\".\n> \n> This patch series cleans up that last remaining part so that the repo\n> becomes purely an out-parameter. To ensure that this is the case we also\n> start to `repo_clear()` it as a first step.\n> \n> Besides simplifying the interface, the intent is also to go further into\n> the direction of unifying repository initialization in a follow-up patch\n> series.\n> \n\nThe patches seem to be well-split and the changes look good. It was a \nnice read.\n\nOverall, this series seems to look good to me. Thank you for making \ncreate_repository not rely on state from the repo given to it!\n\n-- \nSivaraam\n\n"},{"id":"553411","messageId":"aroULK79T-UkwkoM@pks.im","threadId":"66382","inReplyTo":"4770b19f-9a8d-4a4c-8cc6-745aa2868b94@gmail.com","subject":"Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T07:15:56Z","receivedAt":"2026-09-28T07:16:02Z","isPatch":true,"body":"On Sat, Sep 26, 2026 at 01:19:42AM +0530, Kaartic Sivaraam wrote:\n> On 9/24/26 14:49, Patrick Steinhardt wrote:\n> > \n> > diff --git a/path.h b/path.h\n> > index 7e7408dd05..e2d62c4978 100644\n> > --- a/path.h\n> > +++ b/path.h\n> > @@ -254,6 +254,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p\n> > enum scld_error safe_create_leading_directories_const(struct repository *repo,\n> > \t\t\t\t\t\t const char *path);\n> > enum scld_error safe_create_leading_directories_no_share(char *path);\n> > +enum scld_error safe_create_leading_directories_no_share_const(const char *path);\n> > \n> \n> nit: All other variants are mentioned in the documentation blurb just above\n> the declarations. Would it also be worth mentioning this new one there?\n\nThat's fair. I find the comment to be somewhat unwieldy overall. How\nabout this diff?\n\ndiff --git a/path.h b/path.h\nindex 7e7408dd05..922bd6e377 100644\n--- a/path.h\n+++ b/path.h\n@@ -234,14 +234,11 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path);\n * race, callers might want to try invoking the function again when it\n * returns SCLD_VANISHED.\n *\n- * safe_create_leading_directories() temporarily changes path while it\n- * is working but restores it before returning.\n- * safe_create_leading_directories_const() doesn't modify path, even\n- * temporarily. Both these variants adjust the permissions of the\n- * created directories to honor core.sharedRepository, so they are best\n- * suited for files inside the git dir. For working tree files, use\n- * safe_create_leading_directories_no_share() instead, as it ignores\n- * the core.sharedRepository setting.\n+ * The default variants honor \"core.sharedRepository\" and temporarily modify\n+ * `path`. Note that this configuration should be honored for all files in the\n+ * git directory. The `no_share()` variants ignore \"core.sharedRepository\",\n+ * and should be used for working tree files. The `const()` variants do not\n+ * modify `path`.\n */\n enum scld_error {\n SCLD_OK = 0,\n\nPatrick\n"},{"id":"553422","messageId":"CAOLa=ZSxnDLBK+ayCWd3PJ6+3NcYzWCUFdkcnvx8qDBcaeODYQ@mail.gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-1-11499557cf31@pks.im","subject":"Re: [PATCH 1/7] path: drop useless `safe_create_leading_directories_1()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-28T09:01:40Z","receivedAt":"2026-09-28T09:01:43Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> The function `safe_create_leading_directories_1()` is being called by\n> both `safe_create_leading_directories()` and its `_no_share()` variant.\n> It is ultimately the exact same as the former of these functions though\n> and is thus quite useless.\n>\n> Drop the function and inline it into its callsites directly.\n>\n\nNice, always happy to see '_1()' functions go away or be renamed.\n\n[snip]\n"},{"id":"553423","messageId":"CAOLa=ZRwgSZuKHKPnXXz2Voc6o6WYZpVEC+dXk_LfeRZc1R+Cw@mail.gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-4-11499557cf31@pks.im","subject":"Re: [PATCH 4/7] builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-28T09:13:31Z","receivedAt":"2026-09-28T09:13:34Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> When initializing a new repository via git-init(1) we know to honor\n> \"core.sharedRepository\" and adjust permissions of newly created files\n> accordingly. The way we propagate that setting is quite awkward though,\n> as we have to set it on the repository that we pass into\n> `create_repository()` and pass it as a parameter. This is because there\n> are two different scopes in play here:\n>\n> - We need to apply it to the repository so that creating the\n> repository's directory uses the correct permissions.\n>\n> - We need to reapply it to the repository after we have created\n> default files so that we know to override any configuration that we\n> have read from the new repository's configuration.\n>\n> The effect of this though is that the repository works as an in-out\n> parameter, which is quite awkward.\n>\n> Refactor the code so that the caller only needs to pass the value.\n> Starting with this change, the passed-in repository can essentially be\n> completely blank as it doesn't carry any state anymore that we'd care\n> about in `create_repository()`.\n>\n> Note that this change in theory also impacts the other caller of\n> `create_repository()` that exists in git-clone(1). But that caller\n> already passes `-1` as a value for this parameter, and neither does that\n> caller modify the repository it passes. So there shouldn't be any change\n> in behaviour here.\n>\n\nSo this works, becaus we already pass in the `init_shared_repository`\nvalue to `create_repository()`. Which is currently used while creating\nthe default files, now we also extend it to set the adequate permissions\non the repository too.\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/init-db.c | 3 ---\n> setup.c | 3 +++\n> 2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index e45268f1ff..34215bbf18 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -171,9 +171,6 @@ int cmd_init_db(int argc,\n> \t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_format);\n> \t}\n>\n> -\tif (init_shared_repository != -1)\n> -\t\trepo_settings_set_shared_repository(the_repository, init_shared_repository);\n> -\n> \t/*\n> \t * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR\n> \t * without --bare. Catch the error early.\n> diff --git a/setup.c b/setup.c\n> index f335111d1e..0d0a4abbe6 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -2896,6 +2896,9 @@ void create_repository(struct repository *repo,\n> \t */\n> \trepo_config(repo, git_default_core_config, NULL);\n>\n> +\tif (init_shared_repository != -1)\n> +\t\trepo_settings_set_shared_repository(repo, init_shared_repository);\n> +\n> \tsafe_create_dir(repo, git_dir, 0);\n>\n> \tif (!reinit_ok)\n>\n> --\n> 2.56.0.rc2.329.gd58861e689.dirty\n\nWith that context, this patch makes sense.\n"},{"id":"553424","messageId":"CAOLa=ZQ_+Ofya1q01fpZjd_wDn=tk8YxbQWNxhFHya47hFRp-Q@mail.gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-6-11499557cf31@pks.im","subject":"Re: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-28T09:18:52Z","receivedAt":"2026-09-28T09:18:56Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> The function `repo_clear()` can be used to clear a repository's state.\n> The way it's written though it's quite easy for it to accidentally leak\n> some state because we don't make sure to clear the whole structure.\n>\n> Refactor the function to set the whole repository to all-zeroes to avoid\n> any kind of leaking state. While at it, make it a bit more robust when\n> called on an already-blank repository.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> repository.c | 37 ++++++++++++++++++-------------------\n> repository.h | 2 +-\n> 2 files changed, 19 insertions(+), 20 deletions(-)\n>\n> diff --git a/repository.c b/repository.c\n> index b857e1c580..e67ff00550 100644\n> --- a/repository.c\n> +++ b/repository.c\n> @@ -374,60 +374,57 @@ void repo_clear(struct repository *repo)\n> \tstruct hashmap_iter iter;\n> \tstruct strmap_entry *e;\n>\n> -\tFREE_AND_NULL(repo->gitdir);\n> -\tFREE_AND_NULL(repo->commondir);\n> -\tFREE_AND_NULL(repo->prefix);\n> -\tFREE_AND_NULL(repo->graft_file);\n> -\tFREE_AND_NULL(repo->index_file);\n> -\tFREE_AND_NULL(repo->worktree);\n> -\tFREE_AND_NULL(repo->submodule_prefix);\n> -\tFREE_AND_NULL(repo->ref_storage_payload);\n> +\tfree(repo->gitdir);\n> +\tfree(repo->commondir);\n> +\tfree(repo->prefix);\n> +\tfree(repo->graft_file);\n> +\tfree(repo->index_file);\n> +\tfree(repo->worktree);\n> +\tfree(repo->submodule_prefix);\n> +\tfree(repo->ref_storage_payload);\n>\n> \todb_free(repo->objects);\n> -\trepo->objects = NULL;\n>\n> \tif (repo->parsed_objects)\n> \t\tparsed_object_pool_clear(repo->parsed_objects);\n> -\tFREE_AND_NULL(repo->parsed_objects);\n> +\tfree(repo->parsed_objects);\n>\n> \trepo_settings_clear(repo);\n> \trepo_config_values_clear(&repo->config_values_private_);\n>\n> \tif (repo->config) {\n> \t\tgit_configset_clear(repo->config);\n> -\t\tFREE_AND_NULL(repo->config);\n> +\t\tfree(repo->config);\n> \t}\n>\n> -\tif (repo->submodule_cache) {\n> +\tif (repo->submodule_cache)\n> \t\tsubmodule_cache_free(repo->submodule_cache);\n> -\t\trepo->submodule_cache = NULL;\n> -\t}\n>\n> \tif (repo->index) {\n> \t\tdiscard_index(repo->index);\n> -\t\tFREE_AND_NULL(repo->index);\n> +\t\tfree(repo->index);\n> \t}\n>\n> \tif (repo->hook_config_cache) {\n> \t\thook_cache_clear(repo->hook_config_cache);\n> -\t\tFREE_AND_NULL(repo->hook_config_cache);\n> +\t\tfree(repo->hook_config_cache);\n> \t}\n> \tstrmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */\n> \tstring_list_clear(&repo->disabled_events, 0);\n>\n> \tif (repo->promisor_remote_config) {\n> \t\tpromisor_remote_clear(repo->promisor_remote_config);\n> -\t\tFREE_AND_NULL(repo->promisor_remote_config);\n> +\t\tfree(repo->promisor_remote_config);\n> \t}\n>\n> \tif (repo->remote_state) {\n> \t\tremote_state_clear(repo->remote_state);\n> -\t\tFREE_AND_NULL(repo->remote_state);\n> +\t\tfree(repo->remote_state);\n> \t}\n>\n> \tif (repo->refs_private) {\n> \t\tref_store_release(repo->refs_private);\n> -\t\tFREE_AND_NULL(repo->refs_private);\n> +\t\tfree(repo->refs_private);\n> \t}\n>\n> \tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n> @@ -439,6 +436,8 @@ void repo_clear(struct repository *repo)\n> \tstrmap_clear(&repo->worktree_ref_stores, 1);\n>\n> \trepo_clear_path_cache(&repo->cached_paths);\n> +\n> +\tmemset(repo, 0, sizeof(*repo));\n\nThe reason we swap `FREE_AND_NULL()` with `free()` is because we anyways\nset everything to 0. Okay.\n\nOr was this referring to the 'already blank' repository? Since\nFREE_AND_NULL() can already handle NULL values.\n\n> }\n>\n> int repo_read_index(struct repository *repo)\n> diff --git a/repository.h b/repository.h\n> index 11f5c2ed10..2a348012e8 100644\n> --- a/repository.h\n> +++ b/repository.h\n> @@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo,\n> void initialize_repository(struct repository *repo);\n> RESULT_MUST_BE_USED\n> int repo_init(struct repository *r, const char *gitdir, const char *worktree);\n> +void repo_clear(struct repository *repo);\n>\n> /*\n> * Initialize the repository 'subrepo' as the submodule at the given path. If\n> @@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo,\n> \t\t\tstruct repository *superproject,\n> \t\t\tconst char *path,\n> \t\t\tconst struct object_id *treeish_name);\n> -void repo_clear(struct repository *repo);\n>\n\nThis is a purely cosmetic move to bring it closer to `repo_init()`,\nright? I think it makes sense.\n\n> /*\n> * Populates the repository's index from its index_file, an index struct will\n>\n> --\n> 2.56.0.rc2.329.gd58861e689.dirty\n"},{"id":"553425","messageId":"CAOLa=ZSX0e25wK5qQwznXN9rVM+WHn8631pkTEN9Zm-BrXfEsg@mail.gmail.com","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"Re: [PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-28T09:20:30Z","receivedAt":"2026-09-28T09:20:32Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> when creating a new repository via `create_repository()` we pass in a\n> repository. This repository is acting as an in/out parameter: the caller\n> expects that it will be fully configured after the call, but the\n> function itself also uses some information from the passed-in repository\n> to figure out how exactly we want to create it.\n>\n> This interface is quite confusing, as it's not obvious at all what\n> configuration of the repository is relevant. We have thus over a couple\n> of patch series reduced the use of the parameter as in/out parameter. So\n> now, the only piece of info that is still being propagated via the repo\n> is \"core.sharedRepository\".\n>\n> This patch series cleans up that last remaining part so that the repo\n> becomes purely an out-parameter. To ensure that this is the case we also\n> start to `repo_clear()` it as a first step.\n>\n> Besides simplifying the interface, the intent is also to go further into\n> the direction of unifying repository initialization in a follow-up patch\n> series.\n>\n> The series is built on top of 0f8e75abeb (Revert \"Merge branch\n> 'en/no-amend-during-conflicts'\", 2026-09-23) with\n> ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the\n> ability to write alternates, 2026-09-10) merged into it.\n>\n> Thanks!\n>\n> Patrick\n>\n\nThe series was a good read and I didn't see anything that needed\nchanges. Thanks\n\n> ---\n> Patrick Steinhardt (7):\n> path: drop useless `safe_create_leading_directories_1()`\n> path: introduce `safe_create_leading_directories_no_share_const()`\n> builtin/init: refactor messy creation of leading directories\n> builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n> builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n> repository: adapt `repo_clear()` to fully reset the repository\n> setup: enforce that passed-in repo does not carry relevant state\n>\n> builtin/clone.c | 4 ++--\n> builtin/init-db.c | 15 ++-------------\n> path.c | 13 ++++++-------\n> path.h | 1 +\n> repository.c | 37 ++++++++++++++++++-------------------\n> repository.h | 2 +-\n> setup.c | 6 ++++++\n> t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n> 8 files changed, 78 insertions(+), 42 deletions(-)\n>\n>\n> ---\n> base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404\n> change-id: 20260916-pks-create-repository-stateless-f0ca03cca689\n"},{"id":"553426","messageId":"557ab7b2-a1b6-4064-997c-d0dc50126df6@gmail.com","threadId":"66382","inReplyTo":"aroULK79T-UkwkoM@pks.im","subject":"Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-28T09:21:08Z","receivedAt":"2026-09-28T09:21:11Z","isPatch":true,"body":"On 9/28/26 12:45, Patrick Steinhardt wrote:\n> On Sat, Sep 26, 2026 at 01:19:42AM +0530, Kaartic Sivaraam wrote:\n>>\n>> nit: All other variants are mentioned in the documentation blurb just above\n>> the declarations. Would it also be worth mentioning this new one there?\n> \n> That's fair. I find the comment to be somewhat unwieldy overall. How\n> about this diff?\n> \n> diff --git a/path.h b/path.h\n> index 7e7408dd05..922bd6e377 100644\n> --- a/path.h\n> +++ b/path.h\n> @@ -234,14 +234,11 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path);\n> * race, callers might want to try invoking the function again when it\n> * returns SCLD_VANISHED.\n> *\n> - * safe_create_leading_directories() temporarily changes path while it\n> - * is working but restores it before returning.\n> - * safe_create_leading_directories_const() doesn't modify path, even\n> - * temporarily. Both these variants adjust the permissions of the\n> - * created directories to honor core.sharedRepository, so they are best\n> - * suited for files inside the git dir. For working tree files, use\n> - * safe_create_leading_directories_no_share() instead, as it ignores\n> - * the core.sharedRepository setting.\n> + * The default variants honor \"core.sharedRepository\" and temporarily modify\n> + * `path`. Note that this configuration should be honored for all files in the\n> + * git directory. The `no_share()` variants ignore \"core.sharedRepository\",\n> + * and should be used for working tree files. The `const()` variants do not\n> + * modify `path`.\n> */\n\nReads much better to me. Thanks.\n\n-- \nSivaraam\n\n"},{"id":"553429","messageId":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im","subject":"[PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:01Z","receivedAt":"2026-09-28T09:51:50Z","isPatch":true,"body":"Hi,\n\nwhen creating a new repository via `create_repository()` we pass in a\nrepository. This repository is acting as an in/out parameter: the caller\nexpects that it will be fully configured after the call, but the\nfunction itself also uses some information from the passed-in repository\nto figure out how exactly we want to create it.\n\nThis interface is quite confusing, as it's not obvious at all what\nconfiguration of the repository is relevant. We have thus over a couple\nof patch series reduced the use of the parameter as in/out parameter. So\nnow, the only piece of info that is still being propagated via the repo\nis \"core.sharedRepository\".\n\nThis patch series cleans up that last remaining part so that the repo\nbecomes purely an out-parameter. To ensure that this is the case we also\nstart to `repo_clear()` it as a first step.\n\nBesides simplifying the interface, the intent is also to go further into\nthe direction of unifying repository initialization in a follow-up patch\nseries.\n\nThe series is built on top of 0f8e75abeb (Revert \"Merge branch\n'en/no-amend-during-conflicts'\", 2026-09-23) with\nps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the\nability to write alternates, 2026-09-10) merged into it.\n\nChanges in v2:\n - Adapt documentation of `safe_create_leading_directories()`.\n - Better explain change to fully clear repos.\n - Link to v1: https://patch.msgid.link/20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n path: drop useless `safe_create_leading_directories_1()`\n path: introduce `safe_create_leading_directories_no_share_const()`\n builtin/init: refactor messy creation of leading directories\n builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n repository: adapt `repo_clear()` to fully reset the repository\n setup: enforce that passed-in repo does not carry relevant state\n\n builtin/clone.c | 4 ++--\n builtin/init-db.c | 15 ++-------------\n path.c | 13 ++++++-------\n path.h | 14 ++++++--------\n repository.c | 37 ++++++++++++++++++-------------------\n repository.h | 2 +-\n setup.c | 6 ++++++\n t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 83 insertions(+), 50 deletions(-)\n\nRange-diff versus v1:\n\n1: 6ec41760de = 1: 0f9513d5c6 path: drop useless `safe_create_leading_directories_1()`\n2: 85ac056b80 ! 2: 3294cdb6b6 path: introduce `safe_create_leading_directories_no_share_const()`\n @@ path.c: enum scld_error safe_create_leading_directories_no_share(char *path)\n {\n \n ## path.h ##\n +@@ path.h: int safe_create_dir_in_gitdir(struct repository *repo, const char *path);\n + * race, callers might want to try invoking the function again when it\n + * returns SCLD_VANISHED.\n + *\n +- * safe_create_leading_directories() temporarily changes path while it\n +- * is working but restores it before returning.\n +- * safe_create_leading_directories_const() doesn't modify path, even\n +- * temporarily. Both these variants adjust the permissions of the\n +- * created directories to honor core.sharedRepository, so they are best\n +- * suited for files inside the git dir. For working tree files, use\n +- * safe_create_leading_directories_no_share() instead, as it ignores\n +- * the core.sharedRepository setting.\n ++ * The default variants honor \"core.sharedRepository\" and temporarily modify\n ++ * `path`. Note that this configuration should be honored for all files in the\n ++ * git directory. The `no_share()` variants ignore \"core.sharedRepository\",\n ++ * and should be used for working tree files. The `const()` variants do not\n ++ * modify `path`.\n + */\n + enum scld_error {\n + \tSCLD_OK = 0,\n @@ path.h: enum scld_error safe_create_leading_directories(struct repository *repo, char *p\n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n \t\t\t\t\t\t const char *path);\n3: 3a7c197f1b = 3: 8dd89f144a builtin/init: refactor messy creation of leading directories\n4: c3ced666bd = 4: f37db1b17d builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n5: 25918a4ff6 = 5: db76d32f2c builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n6: a742852675 ! 6: 19388a188c repository: adapt `repo_clear()` to fully reset the repository\n @@ Commit message\n some state because we don't make sure to clear the whole structure.\n \n Refactor the function to set the whole repository to all-zeroes to avoid\n - any kind of leaking state. While at it, make it a bit more robust when\n - called on an already-blank repository.\n + any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead\n + use free(3p) to avoid zeroing out the data twice.\n \n Signed-off-by: Patrick Steinhardt \n \n7: 760058e9c5 = 7: 0d4819f005 setup: enforce that passed-in repo does not carry relevant state\n\n---\nbase-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404\nchange-id: 20260916-pks-create-repository-stateless-f0ca03cca689\n\n"},{"id":"553430","messageId":"20260928-pks-create-repository-stateless-v2-1-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 1/7] path: drop useless `safe_create_leading_directories_1()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:02Z","receivedAt":"2026-09-28T09:51:50Z","isPatch":true,"body":"The function `safe_create_leading_directories_1()` is being called by\nboth `safe_create_leading_directories()` and its `_no_share()` variant.\nIt is ultimately the exact same as the former of these functions though\nand is thus quite useless.\n\nDrop the function and inline it into its callsites directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n path.c | 12 +++---------\n 1 file changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex c3a709a928..69b06c9464 100644\n--- a/path.c\n+++ b/path.c\n@@ -829,8 +829,8 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path)\n \treturn adjust_shared_perm(repo, path);\n }\n \n-static enum scld_error safe_create_leading_directories_1(struct repository *repo,\n-\t\t\t\t\t\t\t char *path)\n+enum scld_error safe_create_leading_directories(struct repository *repo,\n+\t\t\t\t\t\tchar *path)\n {\n \tchar *next_component = path + offset_1st_component(path);\n \tenum scld_error ret = SCLD_OK;\n@@ -884,15 +884,9 @@ static enum scld_error safe_create_leading_directories_1(struct repository *repo\n \treturn ret;\n }\n \n-enum scld_error safe_create_leading_directories(struct repository *repo,\n-\t\t\t\t\t\tchar *path)\n-{\n-\treturn safe_create_leading_directories_1(repo, path);\n-}\n-\n enum scld_error safe_create_leading_directories_no_share(char *path)\n {\n-\treturn safe_create_leading_directories_1(NULL, path);\n+\treturn safe_create_leading_directories(NULL, path);\n }\n \n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553431","messageId":"20260928-pks-create-repository-stateless-v2-2-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 2/7] path: introduce `safe_create_leading_directories_no_share_const()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:03Z","receivedAt":"2026-09-28T09:51:52Z","isPatch":true,"body":"The `safe_create_leading_directories()` family of functions modify the\npassed-in path so that we can obtain all the different segments of the\npath. This is done by overwriting path separators with a NUL byte for\nevery component. While we ultimately restore the original string, the\nconsequence is that the caller needs to pass a non-constant string.\n\nWhile it would be trivial to modify the function to not modify the path\nin-place anymore, the intent of this whole mechanism is to save an\nallocation. It's quite dubious whether this optimization really matters\nin the grand scheme of things, but here we are.\n\nIn any case, we provide a `_const()` variant that handles the case where\nthe caller only has a string constant. But we lack such a variant for\nthe `safe_create_leading_directories_no_share()` function, and we're\nabout to add a couple of callers that would need it.\n\nAdd this helper function.\n\nSigned-off-by: Patrick Steinhardt \n---\n path.c | 5 +++++\n path.h | 14 ++++++--------\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 69b06c9464..f8f5a9dd28 100644\n--- a/path.c\n+++ b/path.c\n@@ -889,6 +889,11 @@ enum scld_error safe_create_leading_directories_no_share(char *path)\n \treturn safe_create_leading_directories(NULL, path);\n }\n \n+enum scld_error safe_create_leading_directories_no_share_const(const char *path)\n+{\n+\treturn safe_create_leading_directories_const(NULL, path);\n+}\n+\n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n \t\t\t\t\t\t const char *path)\n {\ndiff --git a/path.h b/path.h\nindex 7e7408dd05..922bd6e377 100644\n--- a/path.h\n+++ b/path.h\n@@ -234,14 +234,11 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path);\n * race, callers might want to try invoking the function again when it\n * returns SCLD_VANISHED.\n *\n- * safe_create_leading_directories() temporarily changes path while it\n- * is working but restores it before returning.\n- * safe_create_leading_directories_const() doesn't modify path, even\n- * temporarily. Both these variants adjust the permissions of the\n- * created directories to honor core.sharedRepository, so they are best\n- * suited for files inside the git dir. For working tree files, use\n- * safe_create_leading_directories_no_share() instead, as it ignores\n- * the core.sharedRepository setting.\n+ * The default variants honor \"core.sharedRepository\" and temporarily modify\n+ * `path`. Note that this configuration should be honored for all files in the\n+ * git directory. The `no_share()` variants ignore \"core.sharedRepository\",\n+ * and should be used for working tree files. The `const()` variants do not\n+ * modify `path`.\n */\n enum scld_error {\n \tSCLD_OK = 0,\n@@ -254,6 +251,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p\n enum scld_error safe_create_leading_directories_const(struct repository *repo,\n \t\t\t\t\t\t const char *path);\n enum scld_error safe_create_leading_directories_no_share(char *path);\n+enum scld_error safe_create_leading_directories_no_share_const(const char *path);\n \n /*\n * Create a file, potentially creating its leading directories in case they\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553432","messageId":"20260928-pks-create-repository-stateless-v2-3-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 3/7] builtin/init: refactor messy creation of leading directories","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:04Z","receivedAt":"2026-09-28T09:51:55Z","isPatch":true,"body":"When creating a new repository via git-init(1) we potentially have to\ncreate any leading directories via `safe_create_leading_directories()`.\nThis function optionally knows to handle \"core.sharedRepository\" to\nadjust the permissions of the created directories.\n\nThe value of that setting is taken from the passed-in repository. When\ncreating a new repository we don't want to honor it though, so we\npainstakingly:\n\n 1. Save the current value of that setting.\n\n 2. Set it to 0.\n\n 3. Create the directory with `safe_create_leading_directories()`. This\n has the effect that `adjust_shared_perm()` will exit early and not\n adjust permissions.\n\n 4. Restore the old value.\n\nThis is extremely awkward, but it achieves the desired effect that we\nignore the configuration. There's a significantly easier way to achieve\nthis though: we can just call the `_no_share()` variant, whose entire\npurpose it is to ignore \"core.sharedRepository\".\n\nRefactor the code to use that variant accordingly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/init-db.c | 12 ++----------\n 1 file changed, 2 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 5c22eae2f3..e45268f1ff 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -131,15 +131,7 @@ int cmd_init_db(int argc,\n \tretry:\n \t\tif (chdir(argv[0]) < 0) {\n \t\t\tif (!mkdir_tried) {\n-\t\t\t\tint saved;\n-\t\t\t\t/*\n-\t\t\t\t * At this point we haven't read any configuration,\n-\t\t\t\t * and we know shared_repository should always be 0;\n-\t\t\t\t * but just in case we play safe.\n-\t\t\t\t */\n-\t\t\t\tsaved = repo_settings_get_shared_repository(the_repository);\n-\t\t\t\trepo_settings_set_shared_repository(the_repository, 0);\n-\t\t\t\tswitch (safe_create_leading_directories_const(the_repository, argv[0])) {\n+\t\t\t\tswitch (safe_create_leading_directories_no_share_const(argv[0])) {\n \t\t\t\tcase SCLD_OK:\n \t\t\t\tcase SCLD_PERMS:\n \t\t\t\t\tbreak;\n@@ -150,7 +142,7 @@ int cmd_init_db(int argc,\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\trepo_settings_set_shared_repository(the_repository, saved);\n+\n \t\t\t\tif (mkdir(argv[0], 0777) < 0)\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\tmkdir_tried = 1;\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553433","messageId":"20260928-pks-create-repository-stateless-v2-4-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 4/7] builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:05Z","receivedAt":"2026-09-28T09:51:57Z","isPatch":true,"body":"When initializing a new repository via git-init(1) we know to honor\n\"core.sharedRepository\" and adjust permissions of newly created files\naccordingly. The way we propagate that setting is quite awkward though,\nas we have to set it on the repository that we pass into\n`create_repository()` and pass it as a parameter. This is because there\nare two different scopes in play here:\n\n - We need to apply it to the repository so that creating the\n repository's directory uses the correct permissions.\n\n - We need to reapply it to the repository after we have created\n default files so that we know to override any configuration that we\n have read from the new repository's configuration.\n\nThe effect of this though is that the repository works as an in-out\nparameter, which is quite awkward.\n\nRefactor the code so that the caller only needs to pass the value.\nStarting with this change, the passed-in repository can essentially be\ncompletely blank as it doesn't carry any state anymore that we'd care\nabout in `create_repository()`.\n\nNote that this change in theory also impacts the other caller of\n`create_repository()` that exists in git-clone(1). But that caller\nalready passes `-1` as a value for this parameter, and neither does that\ncaller modify the repository it passes. So there shouldn't be any change\nin behaviour here.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/init-db.c | 3 ---\n setup.c | 3 +++\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex e45268f1ff..34215bbf18 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -171,9 +171,6 @@ int cmd_init_db(int argc,\n \t\t\tdie(_(\"unknown ref storage format '%s'\"), ref_format);\n \t}\n \n-\tif (init_shared_repository != -1)\n-\t\trepo_settings_set_shared_repository(the_repository, init_shared_repository);\n-\n \t/*\n \t * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR\n \t * without --bare. Catch the error early.\ndiff --git a/setup.c b/setup.c\nindex f335111d1e..0d0a4abbe6 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2896,6 +2896,9 @@ void create_repository(struct repository *repo,\n \t */\n \trepo_config(repo, git_default_core_config, NULL);\n \n+\tif (init_shared_repository != -1)\n+\t\trepo_settings_set_shared_repository(repo, init_shared_repository);\n+\n \tsafe_create_dir(repo, git_dir, 0);\n \n \tif (!reinit_ok)\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553434","messageId":"20260928-pks-create-repository-stateless-v2-5-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 5/7] builtin/clone: don't apply \"core.sharedRepository\" to leading dirs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:06Z","receivedAt":"2026-09-28T09:52:00Z","isPatch":true,"body":"When creating a repository via git-clone(1) we create leading\ndirectories with `safe_create_leading_directories()`. We have adapted\ngit-init(1) in a preceding commit to instead use the variant of\nthis function that doesn't honor \"core.sharedRepository\". In that\nsubcommand it didn't have an effect though as we explicitly unset the\nvalue of that configuration anyway, so we never honored that config.\n\nIn git-clone(1) it's a bit of a different thing though: while the\nrepository isn't initialized at the point in time where we call the\nfunction, we didn't explicitly unset the value. Consequently we _do_\nhonor the configuration here, but when it's configured in global- or\nsystem-level scope.\n\nThis divergence doesn't seem to be intentional -- I cannot think of any\ngood reason why git-init(1) and git-clone(1) should have divergent\nbehaviour here.\n\nAdapt git-clone(1) to work the same as git-init(1) by also using the\n`no_share()` variants to create leading directories. Add tests for both\ncommands.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/clone.c | 4 ++--\n t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b14264c33a..e72f8aa325 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1133,7 +1133,7 @@ int cmd_clone(int argc,\n \tsigchain_push_common(remove_junk_on_signal);\n \n \tif (!option_bare) {\n-\t\tif (safe_create_leading_directories_const(the_repository, work_tree) < 0)\n+\t\tif (safe_create_leading_directories_no_share_const(work_tree) < 0)\n \t\t\tdie_errno(_(\"could not create leading directories of '%s'\"),\n \t\t\t\t work_tree);\n \t\tif (dest_exists)\n@@ -1153,7 +1153,7 @@ int cmd_clone(int argc,\n \t\t\tjunk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n \t\tjunk_git_dir = git_dir;\n \t}\n-\tif (safe_create_leading_directories_const(the_repository, git_dir) < 0)\n+\tif (safe_create_leading_directories_no_share_const(git_dir) < 0)\n \t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n \n \tif (0 <= option_verbosity) {\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 0e0d07a1a1..3bc4bdb038 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -210,4 +210,46 @@ test_expect_success POSIXPERM 'template can set core.sharedrepository' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success POSIXPERM 'init does not apply core.sharedRepository to leading directories' '\n+\ttest_config_global core.sharedRepository 0666 &&\n+\tumask 0077 &&\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit init --bare dst/with/leading/dirs &&\n+\tcat >expect <<-\\EOF &&\n+\tdrwx------\n+\tdrwx------\n+\tdrwx------\n+\tdrwxrwxrwx\n+\tEOF\n+\t{\n+\t\ttest_modebits dst &&\n+\t\ttest_modebits dst/with &&\n+\t\ttest_modebits dst/with/leading &&\n+\t\ttest_modebits dst/with/leading/dirs\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success POSIXPERM 'clone does not apply core.sharedRepository to leading directories' '\n+\ttest_config_global core.sharedRepository 0666 &&\n+\tumask 0077 &&\n+\ttest_when_finished \"rm -rf source dst\" &&\n+\tgit init source &&\n+\ttest_commit -C source initial &&\n+\tgit clone --bare source dst/with/leading/dirs &&\n+\tcat >expect <<-\\EOF &&\n+\tdrwx------\n+\tdrwx------\n+\tdrwx------\n+\tdrwxrwxrwx\n+\tEOF\n+\t{\n+\t\ttest_modebits dst &&\n+\t\ttest_modebits dst/with &&\n+\t\ttest_modebits dst/with/leading &&\n+\t\ttest_modebits dst/with/leading/dirs\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553435","messageId":"20260928-pks-create-repository-stateless-v2-6-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 6/7] repository: adapt `repo_clear()` to fully reset the repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:07Z","receivedAt":"2026-09-28T09:52:03Z","isPatch":true,"body":"The function `repo_clear()` can be used to clear a repository's state.\nThe way it's written though it's quite easy for it to accidentally leak\nsome state because we don't make sure to clear the whole structure.\n\nRefactor the function to set the whole repository to all-zeroes to avoid\nany kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead\nuse free(3p) to avoid zeroing out the data twice.\n\nSigned-off-by: Patrick Steinhardt \n---\n repository.c | 37 ++++++++++++++++++-------------------\n repository.h | 2 +-\n 2 files changed, 19 insertions(+), 20 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex b857e1c580..e67ff00550 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -374,60 +374,57 @@ void repo_clear(struct repository *repo)\n \tstruct hashmap_iter iter;\n \tstruct strmap_entry *e;\n \n-\tFREE_AND_NULL(repo->gitdir);\n-\tFREE_AND_NULL(repo->commondir);\n-\tFREE_AND_NULL(repo->prefix);\n-\tFREE_AND_NULL(repo->graft_file);\n-\tFREE_AND_NULL(repo->index_file);\n-\tFREE_AND_NULL(repo->worktree);\n-\tFREE_AND_NULL(repo->submodule_prefix);\n-\tFREE_AND_NULL(repo->ref_storage_payload);\n+\tfree(repo->gitdir);\n+\tfree(repo->commondir);\n+\tfree(repo->prefix);\n+\tfree(repo->graft_file);\n+\tfree(repo->index_file);\n+\tfree(repo->worktree);\n+\tfree(repo->submodule_prefix);\n+\tfree(repo->ref_storage_payload);\n \n \todb_free(repo->objects);\n-\trepo->objects = NULL;\n \n \tif (repo->parsed_objects)\n \t\tparsed_object_pool_clear(repo->parsed_objects);\n-\tFREE_AND_NULL(repo->parsed_objects);\n+\tfree(repo->parsed_objects);\n \n \trepo_settings_clear(repo);\n \trepo_config_values_clear(&repo->config_values_private_);\n \n \tif (repo->config) {\n \t\tgit_configset_clear(repo->config);\n-\t\tFREE_AND_NULL(repo->config);\n+\t\tfree(repo->config);\n \t}\n \n-\tif (repo->submodule_cache) {\n+\tif (repo->submodule_cache)\n \t\tsubmodule_cache_free(repo->submodule_cache);\n-\t\trepo->submodule_cache = NULL;\n-\t}\n \n \tif (repo->index) {\n \t\tdiscard_index(repo->index);\n-\t\tFREE_AND_NULL(repo->index);\n+\t\tfree(repo->index);\n \t}\n \n \tif (repo->hook_config_cache) {\n \t\thook_cache_clear(repo->hook_config_cache);\n-\t\tFREE_AND_NULL(repo->hook_config_cache);\n+\t\tfree(repo->hook_config_cache);\n \t}\n \tstrmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */\n \tstring_list_clear(&repo->disabled_events, 0);\n \n \tif (repo->promisor_remote_config) {\n \t\tpromisor_remote_clear(repo->promisor_remote_config);\n-\t\tFREE_AND_NULL(repo->promisor_remote_config);\n+\t\tfree(repo->promisor_remote_config);\n \t}\n \n \tif (repo->remote_state) {\n \t\tremote_state_clear(repo->remote_state);\n-\t\tFREE_AND_NULL(repo->remote_state);\n+\t\tfree(repo->remote_state);\n \t}\n \n \tif (repo->refs_private) {\n \t\tref_store_release(repo->refs_private);\n-\t\tFREE_AND_NULL(repo->refs_private);\n+\t\tfree(repo->refs_private);\n \t}\n \n \tstrmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)\n@@ -439,6 +436,8 @@ void repo_clear(struct repository *repo)\n \tstrmap_clear(&repo->worktree_ref_stores, 1);\n \n \trepo_clear_path_cache(&repo->cached_paths);\n+\n+\tmemset(repo, 0, sizeof(*repo));\n }\n \n int repo_read_index(struct repository *repo)\ndiff --git a/repository.h b/repository.h\nindex 11f5c2ed10..2a348012e8 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo,\n void initialize_repository(struct repository *repo);\n RESULT_MUST_BE_USED\n int repo_init(struct repository *r, const char *gitdir, const char *worktree);\n+void repo_clear(struct repository *repo);\n \n /*\n * Initialize the repository 'subrepo' as the submodule at the given path. If\n@@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo,\n \t\t\tstruct repository *superproject,\n \t\t\tconst char *path,\n \t\t\tconst struct object_id *treeish_name);\n-void repo_clear(struct repository *repo);\n \n /*\n * Populates the repository's index from its index_file, an index struct will\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553436","messageId":"20260928-pks-create-repository-stateless-v2-7-a03612f703fa@pks.im","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"[PATCH v2 7/7] setup: enforce that passed-in repo does not carry relevant state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:51:08Z","receivedAt":"2026-09-28T09:52:05Z","isPatch":true,"body":"In the preceding patches we have refactored `create_repository()` so\nthat the passed-in repository is not used anymore to propagate any kind\nof state. This was done so that the parameter doesn't act like an in-out\nparameter, but only as an out parameter that we initialize with the\nstate of the newly created repository.\n\nWe don't enforce though that the repository _cannot_ be used to\npropagate state anymore, which makes it quite easy for state to sneak in\nat a later point again.\n\nIdeally, we'd do that by having the function create a newly allocated\nrepository instead of taking a repository as input. But unfortunately,\nthat does not work because we end up calling `repo_config_values()` when\nwe create the \"files\" ref database, and that function requires that the\npassed-in repository is `the_repository`.\n\nInstead, call `repo_clear()` at the beginning of the function, which\ngives us a clean slate.\n\nSigned-off-by: Patrick Steinhardt \n---\n setup.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/setup.c b/setup.c\nindex 0d0a4abbe6..fa39219d6a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2858,6 +2858,9 @@ void create_repository(struct repository *repo,\n \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n \n+\trepo_clear(repo);\n+\tinitialize_repository(repo);\n+\n \tif (real_git_dir) {\n \t\tstruct stat st;\n \n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553437","messageId":"aro4xkSiJDWknI1W@pks.im","threadId":"66382","inReplyTo":"CAOLa=ZQ_+Ofya1q01fpZjd_wDn=tk8YxbQWNxhFHya47hFRp-Q@mail.gmail.com","subject":"Re: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T09:52:06Z","receivedAt":"2026-09-28T09:52:10Z","isPatch":true,"body":"On Mon, Sep 28, 2026 at 09:18:52AM +0000, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> > diff --git a/repository.c b/repository.c\n> > index b857e1c580..e67ff00550 100644\n> > --- a/repository.c\n> > +++ b/repository.c\n> > @@ -439,6 +436,8 @@ void repo_clear(struct repository *repo)\n> > \tstrmap_clear(&repo->worktree_ref_stores, 1);\n> >\n> > \trepo_clear_path_cache(&repo->cached_paths);\n> > +\n> > +\tmemset(repo, 0, sizeof(*repo));\n> \n> The reason we swap `FREE_AND_NULL()` with `free()` is because we anyways\n> set everything to 0. Okay.\n> \n> Or was this referring to the 'already blank' repository? Since\n> FREE_AND_NULL() can already handle NULL values.\n\nYeah, the only reason I swap to plain free(3p) calls is because it's\nredundant now with the final call to memset(3p). I think the part about\nalready-blank repositories is not accurate anymore, but it used to be at\none point. Let me reword it.\n\n> > diff --git a/repository.h b/repository.h\n> > index 11f5c2ed10..2a348012e8 100644\n> > --- a/repository.h\n> > +++ b/repository.h\n> > @@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo,\n> > void initialize_repository(struct repository *repo);\n> > RESULT_MUST_BE_USED\n> > int repo_init(struct repository *r, const char *gitdir, const char *worktree);\n> > +void repo_clear(struct repository *repo);\n> >\n> > /*\n> > * Initialize the repository 'subrepo' as the submodule at the given path. If\n> > @@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo,\n> > \t\t\tstruct repository *superproject,\n> > \t\t\tconst char *path,\n> > \t\t\tconst struct object_id *treeish_name);\n> > -void repo_clear(struct repository *repo);\n> >\n> \n> This is a purely cosmetic move to bring it closer to `repo_init()`,\n> right? I think it makes sense.\n\nYes, it is.\n\nPatrick\n"},{"id":"553453","messageId":"b0ec2ef9-7aef-4f7d-b31b-7141b39c2d24@gmail.com","threadId":"66382","inReplyTo":"20260928-pks-create-repository-stateless-v2-0-a03612f703fa@pks.im","subject":"Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-28T12:15:00Z","receivedAt":"2026-09-28T12:15:04Z","isPatch":true,"body":"On 9/28/26 15:21, Patrick Steinhardt wrote:\n> \n> [... snip ...]\n >\n> 3: 3a7c197f1b = 3: 8dd89f144a builtin/init: refactor messy creation of leading directories\n> 4: c3ced666bd = 4: f37db1b17d builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n> 5: 25918a4ff6 = 5: db76d32f2c builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n> 6: a742852675 ! 6: 19388a188c repository: adapt `repo_clear()` to fully reset the repository\n> @@ Commit message\n> some state because we don't make sure to clear the whole structure.\n> \n> Refactor the function to set the whole repository to all-zeroes to avoid\n> - any kind of leaking state. While at it, make it a bit more robust when\n> - called on an already-blank repository.\n> + any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead\n> + use free(3p) to avoid zeroing out the data twice.\n>\n\ns/free(3p)/free/\nRest of the inter-diff looks neat.\n\n-- \nSivaraam\n\n"},{"id":"553454","messageId":"886d145f-ac38-4079-8a96-f09904fc3b10@gmail.com","threadId":"66382","inReplyTo":"b0ec2ef9-7aef-4f7d-b31b-7141b39c2d24@gmail.com","subject":"Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-28T12:19:20Z","receivedAt":"2026-09-28T12:19:25Z","isPatch":true,"body":"On 9/28/26 17:45, Kaartic Sivaraam wrote:\n> On 9/28/26 15:21, Patrick Steinhardt wrote:\n>>\n>> [... snip ...]\n> >\n>> 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation \n>> of leading directories\n>> 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of \n>> \"core.sharedRepository\" into \"setup.c\"\n>> 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply \n>> \"core.sharedRepository\" to leading dirs\n>> 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to \n>> fully reset the repository\n>>      @@ Commit message\n>>           some state because we don't make sure to clear the whole \n>> structure.\n>>           Refactor the function to set the whole repository to all- \n>> zeroes to avoid\n>>      -    any kind of leaking state. While at it, make it a bit more \n>> robust when\n>>      -    called on an already-blank repository.\n>>      +    any kind of leaking state. Replace calls of \n>> `FREE_AND_NULL()` to instead\n>>      +    use free(3p) to avoid zeroing out the data twice.\n>>\n> \n> s/free(3p)/free/\n\nOops. I meant s/free(3p)/free(3)/\n\n >\n> Rest of the inter-diff looks neat.\n> \n\n-- \nSivaraam\n"},{"id":"553458","messageId":"arpiCEYBV-IzVTK3@pks.im","threadId":"66382","inReplyTo":"b0ec2ef9-7aef-4f7d-b31b-7141b39c2d24@gmail.com","subject":"Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T12:48:08Z","receivedAt":"2026-09-28T12:48:13Z","isPatch":true,"body":"On Mon, Sep 28, 2026 at 05:45:00PM +0530, Kaartic Sivaraam wrote:\n> On 9/28/26 15:21, Patrick Steinhardt wrote:\n> > \n> > [... snip ...]\n> >\n> > 3: 3a7c197f1b = 3: 8dd89f144a builtin/init: refactor messy creation of leading directories\n> > 4: c3ced666bd = 4: f37db1b17d builtin/init: move handling of \"core.sharedRepository\" into \"setup.c\"\n> > 5: 25918a4ff6 = 5: db76d32f2c builtin/clone: don't apply \"core.sharedRepository\" to leading dirs\n> > 6: a742852675 ! 6: 19388a188c repository: adapt `repo_clear()` to fully reset the repository\n> > @@ Commit message\n> > some state because we don't make sure to clear the whole structure.\n> > Refactor the function to set the whole repository to all-zeroes to avoid\n> > - any kind of leaking state. While at it, make it a bit more robust when\n> > - called on an already-blank repository.\n> > + any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead\n> > + use free(3p) to avoid zeroing out the data twice.\n> > \n> \n> s/free(3p)/free/\n> Rest of the inter-diff looks neat.\n\nThe \"(3p)\" is intentional, as we use that to refer to man pages. In this\ncase, it's free as specified in the POSIX programmer's manual.\n\nThanks!\n\nPatrick\n"},{"id":"553459","messageId":"arpiK4chGRDnHXrW@pks.im","threadId":"66382","inReplyTo":"886d145f-ac38-4079-8a96-f09904fc3b10@gmail.com","subject":"Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-28T12:48:43Z","receivedAt":"2026-09-28T12:48:48Z","isPatch":true,"body":"On Mon, Sep 28, 2026 at 05:49:20PM +0530, Kaartic Sivaraam wrote:\n> On 9/28/26 17:45, Kaartic Sivaraam wrote:\n> > On 9/28/26 15:21, Patrick Steinhardt wrote:\n> > > \n> > > [... snip ...]\n> > >\n> > > 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy\n> > > creation of leading directories\n> > > 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of\n> > > \"core.sharedRepository\" into \"setup.c\"\n> > > 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply\n> > > \"core.sharedRepository\" to leading dirs\n> > > 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to\n> > > fully reset the repository\n> > >      @@ Commit message\n> > >           some state because we don't make sure to clear the whole\n> > > structure.\n> > >           Refactor the function to set the whole repository to all-\n> > > zeroes to avoid\n> > >      -    any kind of leaking state. While at it, make it a bit more\n> > > robust when\n> > >      -    called on an already-blank repository.\n> > >      +    any kind of leaking state. Replace calls of\n> > > `FREE_AND_NULL()` to instead\n> > >      +    use free(3p) to avoid zeroing out the data twice.\n> > > \n> > \n> > s/free(3p)/free/\n> \n> Oops. I meant s/free(3p)/free(3)/\n\nAh. 3p is correct though and refers to the POSIX man pages.\n\nPatrick\n"},{"id":"553502","messageId":"xmqqtsn9mkog.fsf@gitster.g","threadId":"66382","inReplyTo":"arpiK4chGRDnHXrW@pks.im","subject":"Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-28T15:59:11Z","receivedAt":"2026-09-28T15:59:14Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n>> Oops. I meant s/free(3p)/free(3)/\n>\n> Ah. 3p is correct though and refers to the POSIX man pages.\n\nIf used in a context where you really care about posix specified\nbehaviour and are interferred by differences among generic C\nlibrary's free() implementations, free(3p) may be the right way to\nspell it out concisely.\n\nEverywhere else, like in this patch where you do not care about the\ndistinction, the extra 'p' is merely a noise, I would have to say.\n\nIf you are writing a wrapper that _depends_ on your platform free()\nbeing strictly posix compliant, then you might write something like\n\n #ifdef WE_HAVE_POSIX_FREE\n #define safe_free(x) free(x)\n #else\n static void safe_free(void *x)\n {\n ...\n }\n #endif\n\nand your commit log message may say \"We use free(3p) where\navailable, but otherwise emulate it via platform free() with some\nsafety knob\".\n\n"}]}