* [PATCH 0/5] tempfile: stop using the_repository
@ 2026-07-14 17:59 René Scharfe
2026-07-14 17:59 ` [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() René Scharfe
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw)
To: git
create_tempfile_mode() and create_tempfile() use the_repository
internally to call adjust_shared_perm(). Expose that dependency and
push it out to their callers.
Patch 5 is a bonus; it converts lockfile users that already work with
other repositories.
tempfile: add repo_create_tempfile{,_mode}()
refs/packed: use repo_create_tempfile()
lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}()
tempfile: stop using the_repository
use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos
apply.c | 10 ++++++----
builtin/difftool.c | 2 +-
builtin/gc.c | 2 +-
builtin/history.c | 2 +-
builtin/sparse-checkout.c | 3 ++-
bundle.c | 4 ++--
commit-graph.c | 9 +++++----
config.c | 4 ++--
lockfile.c | 30 ++++++++++++++++++++++--------
lockfile.h | 31 +++++++++++++++++++++++++++++++
loose.c | 6 ++++--
midx-write.c | 7 ++++---
odb/source-files.c | 3 ++-
refs/files-backend.c | 10 ++++++----
refs/packed-backend.c | 9 ++++-----
refs/packed-backend.h | 2 +-
repack-midx.c | 3 ++-
repository.c | 2 +-
rerere.c | 6 +++---
tempfile.c | 7 +++----
tempfile.h | 10 +++++++---
21 files changed, 110 insertions(+), 52 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe @ 2026-07-14 17:59 ` René Scharfe 2026-07-15 9:52 ` Patrick Steinhardt 2026-07-14 17:59 ` [PATCH 2/5] refs/packed: use repo_create_tempfile() René Scharfe ` (5 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw) To: git Add variants of create_tempfile_mode() that handle arbitrary repositories. Signed-off-by: René Scharfe <l.s.r@web.de> --- tempfile.c | 8 +++++++- tempfile.h | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tempfile.c b/tempfile.c index f0fdf58279..3132eb4371 100644 --- a/tempfile.c +++ b/tempfile.c @@ -135,6 +135,12 @@ static void deactivate_tempfile(struct tempfile *tempfile) /* Make sure errno contains a meaningful value on error */ struct tempfile *create_tempfile_mode(const char *path, int mode) +{ + return repo_create_tempfile_mode(the_repository, path, mode); +} + +struct tempfile *repo_create_tempfile_mode(struct repository *r, + const char *path, int mode) { struct tempfile *tempfile = new_tempfile(); @@ -150,7 +156,7 @@ struct tempfile *create_tempfile_mode(const char *path, int mode) return NULL; } activate_tempfile(tempfile); - if (adjust_shared_perm(the_repository, tempfile->filename.buf)) { + if (adjust_shared_perm(r, tempfile->filename.buf)) { int save_errno = errno; error("cannot fix permission bits on %s", tempfile->filename.buf); delete_tempfile(&tempfile); diff --git a/tempfile.h b/tempfile.h index 2227a095fd..2d17e4dad3 100644 --- a/tempfile.h +++ b/tempfile.h @@ -4,6 +4,8 @@ #include "list.h" #include "strbuf.h" +struct repository; + /* * Handle temporary files. * @@ -94,11 +96,20 @@ struct tempfile { */ struct tempfile *create_tempfile_mode(const char *path, int mode); +struct tempfile *repo_create_tempfile_mode(struct repository *r, + const char *path, int mode); + static inline struct tempfile *create_tempfile(const char *path) { return create_tempfile_mode(path, 0666); } +static inline struct tempfile *repo_create_tempfile(struct repository *r, + const char *path) +{ + return repo_create_tempfile_mode(r, path, 0666); +} + /* * Register an existing file as a tempfile, meaning that it will be * deleted when the program exits. The tempfile is considered closed, -- 2.55.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() 2026-07-14 17:59 ` [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() René Scharfe @ 2026-07-15 9:52 ` Patrick Steinhardt 2026-07-15 11:25 ` René Scharfe 0 siblings, 1 reply; 14+ messages in thread From: Patrick Steinhardt @ 2026-07-15 9:52 UTC (permalink / raw) To: René Scharfe; +Cc: git On Tue, Jul 14, 2026 at 07:59:52PM +0200, René Scharfe wrote: > Add variants of create_tempfile_mode() that handle arbitrary > repositories. One thing I was wondering is whether it really makes sense to pass in a full repository. All we require it for is `adjust_shared_perm()`, and it feels quite extreme to require a full-blown repository. An alternative would be to let callers pass in the setting by themselves, but that would likely lead to lots of duplicated code. So maybe this is a good first step, and we could eventually create another API where users can pass in the configuration instead of a repository if we ever gain callers that don't have a repository available. > diff --git a/tempfile.c b/tempfile.c > index f0fdf58279..3132eb4371 100644 > --- a/tempfile.c > +++ b/tempfile.c > @@ -135,6 +135,12 @@ static void deactivate_tempfile(struct tempfile *tempfile) > > /* Make sure errno contains a meaningful value on error */ > struct tempfile *create_tempfile_mode(const char *path, int mode) > +{ > + return repo_create_tempfile_mode(the_repository, path, mode); > +} Nit: We could've easily created this as an inline function in "tempfile.h". But I expect that we'll get mostly rid of this function in subssubsequent patches, so it probably doesn't matter too much. Patrick ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() 2026-07-15 9:52 ` Patrick Steinhardt @ 2026-07-15 11:25 ` René Scharfe 0 siblings, 0 replies; 14+ messages in thread From: René Scharfe @ 2026-07-15 11:25 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 7/15/26 11:52 AM, Patrick Steinhardt wrote: > On Tue, Jul 14, 2026 at 07:59:52PM +0200, René Scharfe wrote: >> Add variants of create_tempfile_mode() that handle arbitrary >> repositories. > > One thing I was wondering is whether it really makes sense to pass in a > full repository. All we require it for is `adjust_shared_perm()`, and it > feels quite extreme to require a full-blown repository. > > An alternative would be to let callers pass in the setting by > themselves, but that would likely lead to lots of duplicated code. So > maybe this is a good first step, and we could eventually create another > API where users can pass in the configuration instead of a repository if > we ever gain callers that don't have a repository available. Had the same thought. I think it's because create_tempfile() sounds quite generic, but is actually for creating temporary files within a repository, not just anywhere or just for the duration of the creating process, so shared access matters (if enabled). I didn't find a case where a caller would not have at least the_repository to pass in, so while a repo-less adjust_shared_perm() or create_tempfile() might seem cleaner, we probably won't need it in practice. We'll find out.. René ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] refs/packed: use repo_create_tempfile() 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe 2026-07-14 17:59 ` [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() René Scharfe @ 2026-07-14 17:59 ` René Scharfe 2026-07-14 17:59 ` [PATCH 3/5] lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}() René Scharfe ` (4 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw) To: git Apply the config setting core.sharedRepository from the ref store base repository at hand instead of from the_repository. Signed-off-by: René Scharfe <l.s.r@web.de> --- refs/packed-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 499cb55dfa..7e65d9580e 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1393,7 +1393,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re packed_refs_path = get_locked_file_path(&refs->lock); strbuf_addf(&sb, "%s.new", packed_refs_path); free(packed_refs_path); - refs->tempfile = create_tempfile(sb.buf); + refs->tempfile = repo_create_tempfile(refs->base.repo, sb.buf); if (!refs->tempfile) { strbuf_addf(err, "unable to create file %s: %s", sb.buf, strerror(errno)); -- 2.55.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}() 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe 2026-07-14 17:59 ` [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() René Scharfe 2026-07-14 17:59 ` [PATCH 2/5] refs/packed: use repo_create_tempfile() René Scharfe @ 2026-07-14 17:59 ` René Scharfe 2026-07-14 17:59 ` [PATCH 4/5] tempfile: stop using the_repository René Scharfe ` (3 subsequent siblings) 6 siblings, 0 replies; 14+ messages in thread From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw) To: git Add variants of hold_lock_file_for_update_timeout_mode() that handle arbitrary repositories. Signed-off-by: René Scharfe <l.s.r@web.de> --- lockfile.c | 30 ++++++++++++++++++++++-------- lockfile.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lockfile.c b/lockfile.c index 7add2f136a..100f603771 100644 --- a/lockfile.c +++ b/lockfile.c @@ -2,11 +2,14 @@ * Copyright (c) 2005, Junio C Hamano */ +#define USE_THE_REPOSITORY_VARIABLE + #include "git-compat-util.h" #include "abspath.h" #include "gettext.h" #include "lockfile.h" #include "parse.h" +#include "repository.h" #include "strbuf.h" #include "wrapper.h" @@ -162,8 +165,8 @@ static int read_lock_pid(const char *pid_path, uintmax_t *pid_out) } /* Make sure errno contains a meaningful value on error */ -static int lock_file(struct lock_file *lk, const char *path, int flags, - int mode) +static int lock_file(struct repository *r, struct lock_file *lk, + const char *path, int flags, int mode) { struct strbuf base_path = STRBUF_INIT; struct strbuf lock_path = STRBUF_INIT; @@ -176,7 +179,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags, get_lock_path(&lock_path, base_path.buf); get_pid_path(&pid_path, base_path.buf); - lk->tempfile = create_tempfile_mode(lock_path.buf, mode); + lk->tempfile = repo_create_tempfile_mode(r, lock_path.buf, mode); if (lk->tempfile) lk->pid_tempfile = create_lock_pid_file(pid_path.buf, mode); @@ -200,8 +203,9 @@ static int lock_file(struct lock_file *lk, const char *path, int flags, * timeout_ms milliseconds. If timeout_ms is 0, try locking the file * exactly once. If timeout_ms is -1, try indefinitely. */ -static int lock_file_timeout(struct lock_file *lk, const char *path, - int flags, long timeout_ms, int mode) +static int lock_file_timeout(struct repository *r, struct lock_file *lk, + const char *path, int flags, long timeout_ms, + int mode) { int n = 1; int multiplier = 1; @@ -209,7 +213,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, static int random_initialized = 0; if (timeout_ms == 0) - return lock_file(lk, path, flags, mode); + return lock_file(r, lk, path, flags, mode); if (!random_initialized) { srand((unsigned int)getpid()); @@ -223,7 +227,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, long backoff_ms, wait_ms; int fd; - fd = lock_file(lk, path, flags, mode); + fd = lock_file(r, lk, path, flags, mode); if (fd >= 0) return fd; /* success */ @@ -308,7 +312,17 @@ int hold_lock_file_for_update_timeout_mode(struct lock_file *lk, const char *path, int flags, long timeout_ms, int mode) { - int fd = lock_file_timeout(lk, path, flags, timeout_ms, mode); + return repo_hold_lock_file_for_update_timeout_mode(the_repository, + lk, path, flags, + timeout_ms, mode); +} + +int repo_hold_lock_file_for_update_timeout_mode(struct repository *r, + struct lock_file *lk, + const char *path, int flags, + long timeout_ms, int mode) +{ + int fd = lock_file_timeout(r, lk, path, flags, timeout_ms, mode); if (fd < 0) { if (flags & LOCK_DIE_ON_ERROR) unable_to_lock_die(path, errno); diff --git a/lockfile.h b/lockfile.h index e7233f28de..1667612674 100644 --- a/lockfile.h +++ b/lockfile.h @@ -189,6 +189,11 @@ int hold_lock_file_for_update_timeout_mode( struct lock_file *lk, const char *path, int flags, long timeout_ms, int mode); +int repo_hold_lock_file_for_update_timeout_mode(struct repository *r, + struct lock_file *lk, + const char *path, int flags, + long timeout_ms, int mode); + static inline int hold_lock_file_for_update_timeout( struct lock_file *lk, const char *path, int flags, long timeout_ms) @@ -197,6 +202,16 @@ static inline int hold_lock_file_for_update_timeout( timeout_ms, 0666); } +static inline int repo_hold_lock_file_for_update_timeout(struct repository *r, + struct lock_file *lk, + const char *path, + int flags, + long timeout_ms) +{ + return repo_hold_lock_file_for_update_timeout_mode(r, lk, path, flags, + timeout_ms, 0666); +} + /* * Attempt to create a lockfile for the file at `path` and return a * file descriptor for writing to it, or -1 on error. The flags @@ -208,6 +223,13 @@ static inline int hold_lock_file_for_update( return hold_lock_file_for_update_timeout(lk, path, flags, 0); } +static inline int repo_hold_lock_file_for_update(struct repository *r, + struct lock_file *lk, + const char *path, int flags) +{ + return repo_hold_lock_file_for_update_timeout(r, lk, path, flags, 0); +} + static inline int hold_lock_file_for_update_mode( struct lock_file *lk, const char *path, int flags, int mode) @@ -215,6 +237,15 @@ static inline int hold_lock_file_for_update_mode( return hold_lock_file_for_update_timeout_mode(lk, path, flags, 0, mode); } +static inline int repo_hold_lock_file_for_update_mode(struct repository *r, + struct lock_file *lk, + const char *path, + int flags, int mode) +{ + return repo_hold_lock_file_for_update_timeout_mode(r, lk, path, flags, + 0, mode); +} + /* * Return a nonzero value iff `lk` is currently locked. */ -- 2.55.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] tempfile: stop using the_repository 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe ` (2 preceding siblings ...) 2026-07-14 17:59 ` [PATCH 3/5] lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}() René Scharfe @ 2026-07-14 17:59 ` René Scharfe 2026-07-15 9:52 ` Patrick Steinhardt 2026-07-14 17:59 ` [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos René Scharfe ` (2 subsequent siblings) 6 siblings, 1 reply; 14+ messages in thread From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw) To: git Remove the compatibility wrappers create_tempfile_mode() and create_tempfile() that have become unused. Signed-off-by: René Scharfe <l.s.r@web.de> --- tempfile.c | 7 ------- tempfile.h | 7 ------- 2 files changed, 14 deletions(-) diff --git a/tempfile.c b/tempfile.c index 3132eb4371..dc9ca4e645 100644 --- a/tempfile.c +++ b/tempfile.c @@ -42,8 +42,6 @@ * file created by its parent. */ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" #include "abspath.h" #include "path.h" @@ -134,11 +132,6 @@ static void deactivate_tempfile(struct tempfile *tempfile) } /* Make sure errno contains a meaningful value on error */ -struct tempfile *create_tempfile_mode(const char *path, int mode) -{ - return repo_create_tempfile_mode(the_repository, path, mode); -} - struct tempfile *repo_create_tempfile_mode(struct repository *r, const char *path, int mode) { diff --git a/tempfile.h b/tempfile.h index 2d17e4dad3..f571f3c609 100644 --- a/tempfile.h +++ b/tempfile.h @@ -94,16 +94,9 @@ struct tempfile { * `core.sharedRepository`, so it is not guaranteed to have the given * mode. */ -struct tempfile *create_tempfile_mode(const char *path, int mode); - struct tempfile *repo_create_tempfile_mode(struct repository *r, const char *path, int mode); -static inline struct tempfile *create_tempfile(const char *path) -{ - return create_tempfile_mode(path, 0666); -} - static inline struct tempfile *repo_create_tempfile(struct repository *r, const char *path) { -- 2.55.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] tempfile: stop using the_repository 2026-07-14 17:59 ` [PATCH 4/5] tempfile: stop using the_repository René Scharfe @ 2026-07-15 9:52 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-07-15 9:52 UTC (permalink / raw) To: René Scharfe; +Cc: git On Tue, Jul 14, 2026 at 07:59:55PM +0200, René Scharfe wrote: > Remove the compatibility wrappers create_tempfile_mode() and > create_tempfile() that have become unused. Ah, so we do get rid of it. Nice. We might tease in the preceding commit message that we'll get rid of it in a subsequent patch. Patrick ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe ` (3 preceding siblings ...) 2026-07-14 17:59 ` [PATCH 4/5] tempfile: stop using the_repository René Scharfe @ 2026-07-14 17:59 ` René Scharfe 2026-07-15 9:52 ` Patrick Steinhardt 2026-07-14 20:45 ` [PATCH 0/5] tempfile: stop using the_repository Junio C Hamano 2026-07-15 9:51 ` Patrick Steinhardt 6 siblings, 1 reply; 14+ messages in thread From: René Scharfe @ 2026-07-14 17:59 UTC (permalink / raw) To: git Apply the config setting core.sharedRepository from the repository at hand instead of from the_repository. Signed-off-by: René Scharfe <l.s.r@web.de> --- apply.c | 10 ++++++---- builtin/difftool.c | 2 +- builtin/gc.c | 2 +- builtin/history.c | 2 +- builtin/sparse-checkout.c | 3 ++- bundle.c | 4 ++-- commit-graph.c | 9 +++++---- config.c | 4 ++-- loose.c | 6 ++++-- midx-write.c | 7 ++++--- odb/source-files.c | 3 ++- refs/files-backend.c | 10 ++++++---- refs/packed-backend.c | 7 +++---- refs/packed-backend.h | 2 +- repack-midx.c | 3 ++- repository.c | 2 +- rerere.c | 6 +++--- 17 files changed, 46 insertions(+), 36 deletions(-) diff --git a/apply.c b/apply.c index 5e54453f79..ac1bfc7f85 100644 --- a/apply.c +++ b/apply.c @@ -4287,7 +4287,8 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list) } } - hold_lock_file_for_update(&lock, state->fake_ancestor, LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(state->repo, &lock, state->fake_ancestor, + LOCK_DIE_ON_ERROR); res = write_locked_index(&result, &lock, COMMIT_LOCK); discard_index(&result); @@ -4945,9 +4946,10 @@ static int apply_patch(struct apply_state *state, state->update_index = (state->check_index || state->ita_only) && state->apply; if (state->update_index && !is_lock_file_locked(&state->lock_file)) { if (state->index_file) - hold_lock_file_for_update(&state->lock_file, - state->index_file, - LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(state->repo, + &state->lock_file, + state->index_file, + LOCK_DIE_ON_ERROR); else repo_hold_locked_index(state->repo, &state->lock_file, LOCK_DIE_ON_ERROR); diff --git a/builtin/difftool.c b/builtin/difftool.c index 26778f8515..99c01c92ef 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -636,7 +636,7 @@ static int run_dir_diff(struct repository *repo, struct lock_file lock = LOCK_INIT; strbuf_reset(&buf); strbuf_addf(&buf, "%s/wtindex", tmpdir.buf); - if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 || + if (repo_hold_lock_file_for_update(repo, &lock, buf.buf, 0) < 0 || write_locked_index(&wtindex, &lock, COMMIT_LOCK)) { ret = error("could not write %s", buf.buf); goto finish; diff --git a/builtin/gc.c b/builtin/gc.c index d32af422af..7153a49aca 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1790,7 +1790,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, struct repository *r = the_repository; char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path); - if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { + if (repo_hold_lock_file_for_update(r, &lk, lock_path, LOCK_NO_DEREF) < 0) { /* * Another maintenance command is running. * diff --git a/builtin/history.c b/builtin/history.c index fd83de8265..7e5177bc0a 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -764,7 +764,7 @@ static int write_ondisk_index(struct repository *repo, prime_cache_tree(repo, &index, tree); - if (hold_lock_file_for_update(&lock, path, 0) < 0) { + if (repo_hold_lock_file_for_update(repo, &lock, path, 0) < 0) { ret = error_errno(_("unable to acquire index lock")); goto out; } diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 0863d0fb46..cb4a037b77 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -341,7 +341,8 @@ static int write_patterns_and_update(struct repository *repo, if (safe_create_leading_directories(repo, sparse_filename)) die(_("failed to create directory for sparse-checkout file")); - hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(repo, &lk, sparse_filename, + LOCK_DIE_ON_ERROR); result = update_working_directory(repo, pl); if (result) { diff --git a/bundle.c b/bundle.c index fd2db2c837..b64716f252 100644 --- a/bundle.c +++ b/bundle.c @@ -519,8 +519,8 @@ int create_bundle(struct repository *r, const char *path, if (bundle_to_stdout) bundle_fd = 1; else - bundle_fd = hold_lock_file_for_update(&lock, path, - LOCK_DIE_ON_ERROR); + bundle_fd = repo_hold_lock_file_for_update(r, &lock, path, + LOCK_DIE_ON_ERROR); if (version == -1) version = min_version; diff --git a/commit-graph.c b/commit-graph.c index c6d9c5c740..1b073b367a 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2122,8 +2122,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) if (ctx->split) { char *lock_name = get_commit_graph_chain_filename(ctx->odb_source); - hold_lock_file_for_update_mode(&lk, lock_name, - LOCK_DIE_ON_ERROR, 0444); + repo_hold_lock_file_for_update_mode(ctx->r, &lk, lock_name, + LOCK_DIE_ON_ERROR, 0444); free(lock_name); graph_layer = mks_tempfile_m(ctx->graph_name, 0444); @@ -2141,8 +2141,9 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) f = hashfd(ctx->r->hash_algo, get_tempfile_fd(graph_layer), get_tempfile_path(graph_layer)); } else { - hold_lock_file_for_update_mode(&lk, ctx->graph_name, - LOCK_DIE_ON_ERROR, 0444); + repo_hold_lock_file_for_update_mode(ctx->r, &lk, + ctx->graph_name, + LOCK_DIE_ON_ERROR, 0444); f = hashfd(ctx->r->hash_algo, get_lock_file_fd(&lk), get_lock_file_path(&lk)); } diff --git a/config.c b/config.c index 6a0de86e3a..d29425ab8e 100644 --- a/config.c +++ b/config.c @@ -3034,7 +3034,7 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, * The lock serves a purpose in addition to locking: the new * contents of .git/config will be written into it. */ - fd = hold_lock_file_for_update(&lock, config_filename, 0); + fd = repo_hold_lock_file_for_update(r, &lock, config_filename, 0); if (fd < 0) { error_errno(_("could not lock config file %s"), config_filename); ret = CONFIG_NO_LOCK; @@ -3379,7 +3379,7 @@ static int repo_config_copy_or_rename_section_in_file( if (!config_filename) config_filename = filename_buf = repo_git_path(r, "config"); - out_fd = hold_lock_file_for_update(&lock, config_filename, 0); + out_fd = repo_hold_lock_file_for_update(r, &lock, config_filename, 0); if (out_fd < 0) { ret = error(_("could not lock config file %s"), config_filename); goto out; diff --git a/loose.c b/loose.c index 0b626c1b85..a79cafd38a 100644 --- a/loose.c +++ b/loose.c @@ -138,7 +138,8 @@ int repo_write_loose_object_map(struct repository *repo) return 0; repo_common_path_replace(repo, &path, "objects/loose-object-idx"); - fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1); + fd = repo_hold_lock_file_for_update_timeout(repo, &lock, path.buf, + LOCK_DIE_ON_ERROR, -1); iter = kh_begin(map); if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0) goto errout; @@ -180,7 +181,8 @@ static int write_one_object(struct odb_source_loose *loose, struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; strbuf_addf(&path, "%s/loose-object-idx", loose->base.path); - hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1); + repo_hold_lock_file_for_update_timeout(loose->base.odb->repo, &lock, + path.buf, LOCK_DIE_ON_ERROR, -1); fd = open(path.buf, O_WRONLY | O_CREAT | O_APPEND, 0666); if (fd < 0) diff --git a/midx-write.c b/midx-write.c index 8c1837f6df..580724d21a 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1627,8 +1627,8 @@ static int write_midx_internal(struct write_midx_opts *opts) struct strbuf lock_name = STRBUF_INIT; get_midx_chain_filename(opts->source, &lock_name); - hold_lock_file_for_update(&lk, lock_name.buf, - LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(r, &lk, lock_name.buf, + LOCK_DIE_ON_ERROR); strbuf_release(&lock_name); } @@ -1647,7 +1647,8 @@ static int write_midx_internal(struct write_midx_opts *opts) f = hashfd(r->hash_algo, get_tempfile_fd(incr), get_tempfile_path(incr)); } else { - hold_lock_file_for_update(&lk, midx_name.buf, LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(r, &lk, midx_name.buf, + LOCK_DIE_ON_ERROR); f = hashfd(r->hash_algo, get_lock_file_fd(&lk), get_lock_file_path(&lk)); } diff --git a/odb/source-files.c b/odb/source-files.c index 6c8e935c75..db83a9745c 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -218,7 +218,8 @@ static int odb_source_files_write_alternate(struct odb_source *source, int found = 0; int ret; - hold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(source->odb->repo, &lock, path, + LOCK_DIE_ON_ERROR); out = fdopen_lock_file(&lock, "w"); if (!out) { ret = error_errno(_("unable to fdopen alternates lockfile")); diff --git a/refs/files-backend.c b/refs/files-backend.c index 3df56c25c8..1953610c03 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -842,7 +842,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, goto error_return; } - if (hold_lock_file_for_update_timeout( + if (repo_hold_lock_file_for_update_timeout(refs->base.repo, &lock->lk, ref_file.buf, LOCK_NO_DEREF, get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) { int myerr = errno; @@ -1250,8 +1250,8 @@ struct create_reflock_cb { static int create_reflock(const char *path, void *cb) { struct create_reflock_cb *data = cb; - return hold_lock_file_for_update_timeout( - data->lk, path, LOCK_NO_DEREF, + return repo_hold_lock_file_for_update_timeout( + data->repo, data->lk, path, LOCK_NO_DEREF, get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0; } @@ -3581,7 +3581,9 @@ static int files_reflog_expire(struct ref_store *ref_store, * work we need, including cleaning up if the program * exits unexpectedly. */ - if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) { + if (repo_hold_lock_file_for_update(ref_store->repo, + &reflog_lock, log_file, + 0) < 0) { struct strbuf err = STRBUF_INIT; unable_to_lock_message(log_file, errno, &err); error("%s", err.buf); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 7e65d9580e..0cfef881be 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1246,10 +1246,9 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) * don't write new content to it, but rather to a separate * tempfile. */ - if (hold_lock_file_for_update_timeout( - &refs->lock, - refs->path, - flags, timeout_value) < 0) { + if (repo_hold_lock_file_for_update_timeout(ref_store->repo, &refs->lock, + refs->path, flags, + timeout_value) < 0) { unable_to_lock_message(refs->path, errno, err); return -1; } diff --git a/refs/packed-backend.h b/refs/packed-backend.h index 1db48e801d..8a7b323825 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -21,7 +21,7 @@ struct ref_store *packed_ref_store_init(struct repository *repo, /* * Lock the packed-refs file for writing. Flags is passed to - * hold_lock_file_for_update(). Return 0 on success. On errors, write + * repo_hold_lock_file_for_update(). Return 0 on success. On errors, write * an error message to `err` and return a nonzero value. */ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err); diff --git a/repack-midx.c b/repack-midx.c index 7c7c3620e5..64c7f8d0f4 100644 --- a/repack-midx.c +++ b/repack-midx.c @@ -954,7 +954,8 @@ static int write_midx_incremental(struct repack_write_midx_opts *opts) lock_name.buf)) die_errno(_("unable to create leading directories of %s"), lock_name.buf); - hold_lock_file_for_update(&lf, lock_name.buf, LOCK_DIE_ON_ERROR); + repo_hold_lock_file_for_update(opts->existing->repo, &lf, lock_name.buf, + LOCK_DIE_ON_ERROR); if (!fdopen_lock_file(&lf, "w")) { ret = error_errno(_("unable to open multi-pack-index chain file")); diff --git a/repository.c b/repository.c index 73d80bcffd..11fbc69781 100644 --- a/repository.c +++ b/repository.c @@ -472,5 +472,5 @@ int repo_hold_locked_index(struct repository *repo, { if (!repo->index_file) BUG("the repo hasn't been setup"); - return hold_lock_file_for_update(lf, repo->index_file, flags); + return repo_hold_lock_file_for_update(repo, lf, repo->index_file, flags); } diff --git a/rerere.c b/rerere.c index 8232542585..2d1e99ec11 100644 --- a/rerere.c +++ b/rerere.c @@ -911,9 +911,9 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags) if (flags & RERERE_READONLY) fd = 0; else - fd = hold_lock_file_for_update(&write_lock, - git_path_merge_rr(r), - LOCK_DIE_ON_ERROR); + fd = repo_hold_lock_file_for_update(r, &write_lock, + git_path_merge_rr(r), + LOCK_DIE_ON_ERROR); read_rr(r, merge_rr); return fd; } -- 2.55.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos 2026-07-14 17:59 ` [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos René Scharfe @ 2026-07-15 9:52 ` Patrick Steinhardt 2026-07-18 6:35 ` René Scharfe 0 siblings, 1 reply; 14+ messages in thread From: Patrick Steinhardt @ 2026-07-15 9:52 UTC (permalink / raw) To: René Scharfe; +Cc: git On Tue, Jul 14, 2026 at 07:59:56PM +0200, René Scharfe wrote: > Apply the config setting core.sharedRepository from the repository at > hand instead of from the_repository. We only do this for a subset of callsites, apparently. How did you select which subsystems to convert and which not to? To make this explicit: I don't mind a partial migration, but I think the commit message should briefly explain the reasoning behind it. Also, as you don't get rid of the old functions that still implicitly depend on `the_repository`, I think we should have an additional commit on top that guards all functions that have this implicit dependency with `USE_THE_REPOSITORY_VARIABLE`. This ensures that we cannot accidentally call such functions from other subsystems that already got rid of the global dependency. Thanks! Patrick ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos 2026-07-15 9:52 ` Patrick Steinhardt @ 2026-07-18 6:35 ` René Scharfe 2026-07-19 19:11 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: René Scharfe @ 2026-07-18 6:35 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git On 7/15/26 11:52 AM, Patrick Steinhardt wrote: > On Tue, Jul 14, 2026 at 07:59:56PM +0200, René Scharfe wrote: >> Apply the config setting core.sharedRepository from the repository at >> hand instead of from the_repository. > > We only do this for a subset of callsites, apparently. How did you > select which subsystems to convert and which not to? To make this > explicit: I don't mind a partial migration, but I think the commit > message should briefly explain the reasoning behind it. All those that have a repository reference other than the_repository. > Also, as you don't get rid of the old functions that still implicitly > depend on `the_repository`, I think we should have an additional commit > on top that guards all functions that have this implicit dependency with > `USE_THE_REPOSITORY_VARIABLE`. This ensures that we cannot accidentally > call such functions from other subsystems that already got rid of the > global dependency. Probably, but the lockfile conversions deserve their own patch series. Patch 5 is only included here because it was easy to write. We can drop it and leave the low-hanging fruit on the tree if that's preferable. René ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos 2026-07-18 6:35 ` René Scharfe @ 2026-07-19 19:11 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2026-07-19 19:11 UTC (permalink / raw) To: René Scharfe; +Cc: Patrick Steinhardt, git René Scharfe <l.s.r@web.de> writes: > On 7/15/26 11:52 AM, Patrick Steinhardt wrote: >> On Tue, Jul 14, 2026 at 07:59:56PM +0200, René Scharfe wrote: >>> Apply the config setting core.sharedRepository from the repository at >>> hand instead of from the_repository. >> >> We only do this for a subset of callsites, apparently. How did you >> select which subsystems to convert and which not to? To make this >> explicit: I don't mind a partial migration, but I think the commit >> message should briefly explain the reasoning behind it. > > All those that have a repository reference other than the_repository. > >> Also, as you don't get rid of the old functions that still implicitly >> depend on `the_repository`, I think we should have an additional commit >> on top that guards all functions that have this implicit dependency with >> `USE_THE_REPOSITORY_VARIABLE`. This ensures that we cannot accidentally >> call such functions from other subsystems that already got rid of the >> global dependency. > > Probably, but the lockfile conversions deserve their own patch series. > Patch 5 is only included here because it was easy to write. We can drop > it and leave the low-hanging fruit on the tree if that's preferable. I am personally indifferent as to what we do immediately in this series, as long as we all agree on the longer-term direction. It seems we are in agreement on providing additional safety in the medium term? Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] tempfile: stop using the_repository 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe ` (4 preceding siblings ...) 2026-07-14 17:59 ` [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos René Scharfe @ 2026-07-14 20:45 ` Junio C Hamano 2026-07-15 9:51 ` Patrick Steinhardt 6 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2026-07-14 20:45 UTC (permalink / raw) To: René Scharfe; +Cc: git René Scharfe <l.s.r@web.de> writes: > create_tempfile_mode() and create_tempfile() use the_repository > internally to call adjust_shared_perm(). Expose that dependency and > push it out to their callers. > > Patch 5 is a bonus; it converts lockfile users that already work with > other repositories. > > tempfile: add repo_create_tempfile{,_mode}() > refs/packed: use repo_create_tempfile() > lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}() > tempfile: stop using the_repository > use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos Will queue. If I have a chance I may revisit the topic a bit deeper, but nothing stood out as glaringly wrong to my cursory reading so far. Thanks. > > apply.c | 10 ++++++---- > builtin/difftool.c | 2 +- > builtin/gc.c | 2 +- > builtin/history.c | 2 +- > builtin/sparse-checkout.c | 3 ++- > bundle.c | 4 ++-- > commit-graph.c | 9 +++++---- > config.c | 4 ++-- > lockfile.c | 30 ++++++++++++++++++++++-------- > lockfile.h | 31 +++++++++++++++++++++++++++++++ > loose.c | 6 ++++-- > midx-write.c | 7 ++++--- > odb/source-files.c | 3 ++- > refs/files-backend.c | 10 ++++++---- > refs/packed-backend.c | 9 ++++----- > refs/packed-backend.h | 2 +- > repack-midx.c | 3 ++- > repository.c | 2 +- > rerere.c | 6 +++--- > tempfile.c | 7 +++---- > tempfile.h | 10 +++++++--- > 21 files changed, 110 insertions(+), 52 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] tempfile: stop using the_repository 2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe ` (5 preceding siblings ...) 2026-07-14 20:45 ` [PATCH 0/5] tempfile: stop using the_repository Junio C Hamano @ 2026-07-15 9:51 ` Patrick Steinhardt 6 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-07-15 9:51 UTC (permalink / raw) To: René Scharfe; +Cc: git On Tue, Jul 14, 2026 at 07:59:51PM +0200, René Scharfe wrote: > create_tempfile_mode() and create_tempfile() use the_repository > internally to call adjust_shared_perm(). Expose that dependency and > push it out to their callers. Yay! I was just starting to have a look at this area yesterday because the implicit dependency got in my way. Happy to see that you tackle it :) Patrick ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-07-19 19:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 17:59 [PATCH 0/5] tempfile: stop using the_repository René Scharfe
2026-07-14 17:59 ` [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}() René Scharfe
2026-07-15 9:52 ` Patrick Steinhardt
2026-07-15 11:25 ` René Scharfe
2026-07-14 17:59 ` [PATCH 2/5] refs/packed: use repo_create_tempfile() René Scharfe
2026-07-14 17:59 ` [PATCH 3/5] lockfile: add repo_hold_lock_file_for_update{,_timeout}{,_mode}() René Scharfe
2026-07-14 17:59 ` [PATCH 4/5] tempfile: stop using the_repository René Scharfe
2026-07-15 9:52 ` Patrick Steinhardt
2026-07-14 17:59 ` [PATCH 5/5] use repo_hold_lock_file_for_update{,_mode,_timeout}() with custom repos René Scharfe
2026-07-15 9:52 ` Patrick Steinhardt
2026-07-18 6:35 ` René Scharfe
2026-07-19 19:11 ` Junio C Hamano
2026-07-14 20:45 ` [PATCH 0/5] tempfile: stop using the_repository Junio C Hamano
2026-07-15 9:51 ` Patrick Steinhardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox