Hi,

when creating a new repository via `create_repository()` we pass in a repository. This repository is acting as an in/out parameter: the caller expects that it will be fully configured after the call, but the function itself also uses some information from the passed-in repository to figure out how exactly we want to create it.

This interface is quite confusing, as it's not obvious at all what configuration of the repository is relevant. We have thus over a couple of patch series reduced the use of the parameter as in/out parameter. So now, the only piece of info that is still being propagated via the repo is "core.sharedRepository".

This patch series cleans up that last remaining part so that the repo becomes purely an out-parameter. To ensure that this is the case we also start to `repo_clear()` it as a first step.

Besides simplifying the interface, the intent is also to go further into the direction of unifying repository initialization in a follow-up patch series.

The series is built on top of 0f8e75abeb (Revert "Merge branch 'en/no-amend-during-conflicts'", 2026-09-23) with ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the ability to write alternates, 2026-09-10) merged into it.

Thanks!

Patrick

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

 builtin/clone.c        |  4 ++--
 builtin/init-db.c      | 15 ++-------------
 path.c                 | 13 ++++++-------
 path.h                 |  1 +
 repository.c           | 37 ++++++++++++++++++-------------------
 repository.h           |  2 +-
 setup.c                |  6 ++++++
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 78 insertions(+), 42 deletions(-)

--- base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404 change-id: 20260916-pks-create-repository-stateless-f0ca03cca689

The function `safe_create_leading_directories_1()` is being called by both `safe_create_leading_directories()` and its `_no_share()` variant. It is ultimately the exact same as the former of these functions though and is thus quite useless.

Drop the function and inline it into its callsites directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 path.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

The `safe_create_leading_directories()` family of functions modify the passed-in path so that we can obtain all the different segments of the path. This is done by overwriting path separators with a NUL byte for every component. While we ultimately restore the original string, the consequence is that the caller needs to pass a non-constant string.

While it would be trivial to modify the function to not modify the path in-place anymore, the intent of this whole mechanism is to save an allocation. It's quite dubious whether this optimization really matters in the grand scheme of things, but here we are.

In any case, we provide a `_const()` variant that handles the case where the caller only has a string constant. But we lack such a variant for the `safe_create_leading_directories_no_share()` function, and we're about to add a couple of callers that would need it.

Add this helper function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 path.c | 5 +++++
 path.h | 1 +
 2 files changed, 6 insertions(+)

When creating a new repository via git-init(1) we potentially have to create any leading directories via `safe_create_leading_directories()`. This function optionally knows to handle "core.sharedRepository" to adjust the permissions of the created directories.

The value of that setting is taken from the passed-in repository. When creating a new repository we don't want to honor it though, so we painstakingly:

  1. Save the current value of that setting.

  2. Set it to 0.

  3. Create the directory with `safe_create_leading_directories()`. This
     has the effect that `adjust_shared_perm()` will exit early and not
     adjust permissions.

  4. Restore the old value.

This is extremely awkward, but it achieves the desired effect that we ignore the configuration. There's a significantly easier way to achieve this though: we can just call the `_no_share()` variant, whose entire purpose it is to ignore "core.sharedRepository".

Refactor the code to use that variant accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/init-db.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

When initializing a new repository via git-init(1) we know to honor "core.sharedRepository" and adjust permissions of newly created files accordingly. The way we propagate that setting is quite awkward though, as we have to set it on the repository that we pass into `create_repository()` and pass it as a parameter. This is because there are two different scopes in play here:

  - We need to apply it to the repository so that creating the
    repository's directory uses the correct permissions.

  - We need to reapply it to the repository after we have created
    default files so that we know to override any configuration that we
    have read from the new repository's configuration.

The effect of this though is that the repository works as an in-out parameter, which is quite awkward.

Refactor the code so that the caller only needs to pass the value. Starting with this change, the passed-in repository can essentially be completely blank as it doesn't carry any state anymore that we'd care about in `create_repository()`.

Note that this change in theory also impacts the other caller of `create_repository()` that exists in git-clone(1). But that caller already passes `-1` as a value for this parameter, and neither does that caller modify the repository it passes. So there shouldn't be any change in behaviour here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/init-db.c | 3 ---
 setup.c           | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

When creating a repository via git-clone(1) we create leading directories with `safe_create_leading_directories()`. We have adapted git-init(1) in a preceding commit to instead use the variant of this function that doesn't honor "core.sharedRepository". In that subcommand it didn't have an effect though as we explicitly unset the value of that configuration anyway, so we never honored that config.

In git-clone(1) it's a bit of a different thing though: while the repository isn't initialized at the point in time where we call the function, we didn't explicitly unset the value. Consequently we _do_ honor the configuration here, but when it's configured in global- or system-level scope.

This divergence doesn't seem to be intentional -- I cannot think of any good reason why git-init(1) and git-clone(1) should have divergent behaviour here.

Adapt git-clone(1) to work the same as git-init(1) by also using the `no_share()` variants to create leading directories. Add tests for both commands.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c        |  4 ++--
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

The function `repo_clear()` can be used to clear a repository's state. The way it's written though it's quite easy for it to accidentally leak some state because we don't make sure to clear the whole structure.

Refactor the function to set the whole repository to all-zeroes to avoid any kind of leaking state. While at it, make it a bit more robust when called on an already-blank repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 repository.c | 37 ++++++++++++++++++-------------------
 repository.h |  2 +-
 2 files changed, 19 insertions(+), 20 deletions(-)

In the preceding patches we have refactored `create_repository()` so that the passed-in repository is not used anymore to propagate any kind of state. This was done so that the parameter doesn't act like an in-out parameter, but only as an out parameter that we initialize with the state of the newly created repository.

We don't enforce though that the repository _cannot_ be used to propagate state anymore, which makes it quite easy for state to sneak in at a later point again.

Ideally, we'd do that by having the function create a newly allocated repository instead of taking a repository as input. But unfortunately, that does not work because we end up calling `repo_config_values()` when we create the "files" ref database, and that function requires that the passed-in repository is `the_repository`.

Instead, call `repo_clear()` at the beginning of the function, which gives us a clean slate.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 3 +++
 1 file changed, 3 insertions(+)

On 9/24/26 14:49, Patrick Steinhardt wrote:

nit: All other variants are mentioned in the documentation blurb just above the declarations. Would it also be worth mentioning this new one there?

On 9/24/26 14:49, Patrick Steinhardt wrote:

Even though this patch does not aim to do so, we lost a bunch of 'the_repository' references with this change which is nice.

The patch also looks good to me.

On 9/24/26 14:49, Patrick Steinhardt wrote:
 >

 >   	/*
 >   	 * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR
 >   	 * without --bare.  Catch the error early.

I was wondering if there'll be any function that between here and the create_repository call that may use the_repository. I could not find any from my reading of the code, though. So, this looks good to me.

- Sivaraam

On 9/24/26 14:49, Patrick Steinhardt wrote:

The patches seem to be well-split and the changes look good. It was a nice read.

Overall, this series seems to look good to me. Thank you for making create_repository not rely on state from the repo given to it!

On Sat, Sep 26, 2026 at 01:19:42AM +0530, Kaartic Sivaraam wrote:

That's fair. I find the comment to be somewhat unwieldy overall. How about this diff?

Patrick Steinhardt <ps@pks.im> writes:

Nice, always happy to see '_1()' functions go away or be renamed.

[snip]

Patrick Steinhardt <ps@pks.im> writes:

So this works, becaus we already pass in the `init_shared_repository` value to `create_repository()`. Which is currently used while creating the default files, now we also extend it to set the adequate permissions on the repository too.

With that context, this patch makes sense.

Patrick Steinhardt <ps@pks.im> writes:

The reason we swap `FREE_AND_NULL()` with `free()` is because we anyways set everything to 0. Okay.

Or was this referring to the 'already blank' repository? Since FREE_AND_NULL() can already handle NULL values.

This is a purely cosmetic move to bring it closer to `repo_init()`, right? I think it makes sense.

Patrick Steinhardt <ps@pks.im> writes:

The series was a good read and I didn't see anything that needed changes. Thanks

On 9/28/26 12:45, Patrick Steinhardt wrote:

Reads much better to me. Thanks.

Hi,

when creating a new repository via `create_repository()` we pass in a repository. This repository is acting as an in/out parameter: the caller expects that it will be fully configured after the call, but the function itself also uses some information from the passed-in repository to figure out how exactly we want to create it.

This interface is quite confusing, as it's not obvious at all what configuration of the repository is relevant. We have thus over a couple of patch series reduced the use of the parameter as in/out parameter. So now, the only piece of info that is still being propagated via the repo is "core.sharedRepository".

This patch series cleans up that last remaining part so that the repo becomes purely an out-parameter. To ensure that this is the case we also start to `repo_clear()` it as a first step.

Besides simplifying the interface, the intent is also to go further into the direction of unifying repository initialization in a follow-up patch series.

The series is built on top of 0f8e75abeb (Revert "Merge branch 'en/no-amend-during-conflicts'", 2026-09-23) with ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the ability to write alternates, 2026-09-10) merged into it.

Changes in v2:
  - Adapt documentation of `safe_create_leading_directories()`.
  - Better explain change to fully clear repos.
  - Link to v1: https://patch.msgid.link/20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im

Thanks!

Patrick

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

 builtin/clone.c        |  4 ++--
 builtin/init-db.c      | 15 ++-------------
 path.c                 | 13 ++++++-------
 path.h                 | 14 ++++++--------
 repository.c           | 37 ++++++++++++++++++-------------------
 repository.h           |  2 +-
 setup.c                |  6 ++++++
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 83 insertions(+), 50 deletions(-)

Range-diff versus v1:

1:  6ec41760de = 1:  0f9513d5c6 path: drop useless `safe_create_leading_directories_1()`
2:  85ac056b80 ! 2:  3294cdb6b6 path: introduce `safe_create_leading_directories_no_share_const()`
    @@ path.c: enum scld_error safe_create_leading_directories_no_share(char *path)
      {
     
      ## path.h ##
    +@@ path.h: 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,
     @@ path.h: 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);
3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation of leading directories
4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of "core.sharedRepository" into "setup.c"
5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply "core.sharedRepository" to leading dirs
6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to fully reset the repository
    @@ Commit message
         some state because we don't make sure to clear the whole structure.
     
         Refactor the function to set the whole repository to all-zeroes to avoid
    -    any kind of leaking state. While at it, make it a bit more robust when
    -    called on an already-blank repository.
    +    any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead
    +    use free(3p) to avoid zeroing out the data twice.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
7:  760058e9c5 = 7:  0d4819f005 setup: enforce that passed-in repo does not carry relevant state

--- base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404 change-id: 20260916-pks-create-repository-stateless-f0ca03cca689

The function `safe_create_leading_directories_1()` is being called by both `safe_create_leading_directories()` and its `_no_share()` variant. It is ultimately the exact same as the former of these functions though and is thus quite useless.

Drop the function and inline it into its callsites directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 path.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

The `safe_create_leading_directories()` family of functions modify the passed-in path so that we can obtain all the different segments of the path. This is done by overwriting path separators with a NUL byte for every component. While we ultimately restore the original string, the consequence is that the caller needs to pass a non-constant string.

While it would be trivial to modify the function to not modify the path in-place anymore, the intent of this whole mechanism is to save an allocation. It's quite dubious whether this optimization really matters in the grand scheme of things, but here we are.

In any case, we provide a `_const()` variant that handles the case where the caller only has a string constant. But we lack such a variant for the `safe_create_leading_directories_no_share()` function, and we're about to add a couple of callers that would need it.

Add this helper function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 path.c |  5 +++++
 path.h | 14 ++++++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

When creating a new repository via git-init(1) we potentially have to create any leading directories via `safe_create_leading_directories()`. This function optionally knows to handle "core.sharedRepository" to adjust the permissions of the created directories.

The value of that setting is taken from the passed-in repository. When creating a new repository we don't want to honor it though, so we painstakingly:

  1. Save the current value of that setting.

  2. Set it to 0.

  3. Create the directory with `safe_create_leading_directories()`. This
     has the effect that `adjust_shared_perm()` will exit early and not
     adjust permissions.

  4. Restore the old value.

This is extremely awkward, but it achieves the desired effect that we ignore the configuration. There's a significantly easier way to achieve this though: we can just call the `_no_share()` variant, whose entire purpose it is to ignore "core.sharedRepository".

Refactor the code to use that variant accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/init-db.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

When initializing a new repository via git-init(1) we know to honor "core.sharedRepository" and adjust permissions of newly created files accordingly. The way we propagate that setting is quite awkward though, as we have to set it on the repository that we pass into `create_repository()` and pass it as a parameter. This is because there are two different scopes in play here:

  - We need to apply it to the repository so that creating the
    repository's directory uses the correct permissions.

  - We need to reapply it to the repository after we have created
    default files so that we know to override any configuration that we
    have read from the new repository's configuration.

The effect of this though is that the repository works as an in-out parameter, which is quite awkward.

Refactor the code so that the caller only needs to pass the value. Starting with this change, the passed-in repository can essentially be completely blank as it doesn't carry any state anymore that we'd care about in `create_repository()`.

Note that this change in theory also impacts the other caller of `create_repository()` that exists in git-clone(1). But that caller already passes `-1` as a value for this parameter, and neither does that caller modify the repository it passes. So there shouldn't be any change in behaviour here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/init-db.c | 3 ---
 setup.c           | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

When creating a repository via git-clone(1) we create leading directories with `safe_create_leading_directories()`. We have adapted git-init(1) in a preceding commit to instead use the variant of this function that doesn't honor "core.sharedRepository". In that subcommand it didn't have an effect though as we explicitly unset the value of that configuration anyway, so we never honored that config.

In git-clone(1) it's a bit of a different thing though: while the repository isn't initialized at the point in time where we call the function, we didn't explicitly unset the value. Consequently we _do_ honor the configuration here, but when it's configured in global- or system-level scope.

This divergence doesn't seem to be intentional -- I cannot think of any good reason why git-init(1) and git-clone(1) should have divergent behaviour here.

Adapt git-clone(1) to work the same as git-init(1) by also using the `no_share()` variants to create leading directories. Add tests for both commands.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/clone.c        |  4 ++--
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

The function `repo_clear()` can be used to clear a repository's state. The way it's written though it's quite easy for it to accidentally leak some state because we don't make sure to clear the whole structure.

Refactor the function to set the whole repository to all-zeroes to avoid any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead use free(3p) to avoid zeroing out the data twice.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 repository.c | 37 ++++++++++++++++++-------------------
 repository.h |  2 +-
 2 files changed, 19 insertions(+), 20 deletions(-)

In the preceding patches we have refactored `create_repository()` so that the passed-in repository is not used anymore to propagate any kind of state. This was done so that the parameter doesn't act like an in-out parameter, but only as an out parameter that we initialize with the state of the newly created repository.

We don't enforce though that the repository _cannot_ be used to propagate state anymore, which makes it quite easy for state to sneak in at a later point again.

Ideally, we'd do that by having the function create a newly allocated repository instead of taking a repository as input. But unfortunately, that does not work because we end up calling `repo_config_values()` when we create the "files" ref database, and that function requires that the passed-in repository is `the_repository`.

Instead, call `repo_clear()` at the beginning of the function, which gives us a clean slate.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 setup.c | 3 +++
 1 file changed, 3 insertions(+)

On Mon, Sep 28, 2026 at 09:18:52AM +0000, Karthik Nayak wrote:

Yeah, the only reason I swap to plain free(3p) calls is because it's redundant now with the final call to memset(3p). I think the part about already-blank repositories is not accurate anymore, but it used to be at one point. Let me reword it.

Yes, it is.

Patrick

On 9/28/26 15:21, Patrick Steinhardt wrote:

 >

s/free(3p)/free/ Rest of the inter-diff looks neat.

On 9/28/26 17:45, Kaartic Sivaraam wrote:

Oops. I meant s/free(3p)/free(3)/

 >

On Mon, Sep 28, 2026 at 05:45:00PM +0530, Kaartic Sivaraam wrote:

The "(3p)" is intentional, as we use that to refer to man pages. In this case, it's free as specified in the POSIX programmer's manual.

Thanks!

Patrick

On Mon, Sep 28, 2026 at 05:49:20PM +0530, Kaartic Sivaraam wrote:

Ah. 3p is correct though and refers to the POSIX man pages.

Patrick

Patrick Steinhardt <ps@pks.im> writes:

If used in a context where you really care about posix specified behaviour and are interferred by differences among generic C library's free() implementations, free(3p) may be the right way to spell it out concisely.

Everywhere else, like in this patch where you do not care about the distinction, the extra 'p' is merely a noise, I would have to say.

If you are writing a wrapper that _depends_ on your platform free() being strictly posix compliant, then you might write something like

        #ifdef WE_HAVE_POSIX_FREE
        #define safe_free(x) free(x)
        #else
        static void safe_free(void *x)
        {
                ...
        }
        #endif

and your commit log message may say "We use free(3p) where available, but otherwise emulate it via platform free() with some safety knob".