Thread (32 messages) 32 messages, 4 authors, 13d ago

[PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

flat view
COOLING13d

From: Patrick Steinhardt <hidden>
Date: 2026-09-28 09:51:50

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 (local)

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 [off-list ref]
     
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help