Merge branch 'ps/refs-avoid-chdir-notify-reparent' into seen

The reference backends have been converted to always use absolute
paths internally. This allows dropping the calls to
`chdir_notify_reparent()` and fixes a memory leak in how the
reference database is constructed with an "onbranch" condition.

* ps/refs-avoid-chdir-notify-reparent:
  refs: always use absolute paths for reference stores
  refs: drop local buffer in `refs_compute_filesystem_location()`
  refs: fix recursing `get_main_ref_store()` with "onbranch" config
  repository: free main reference database
  chdir-notify: drop unused `chdir_notify_reparent()`
  refs: unregister reference stores from "chdir_notify"
  setup: don't apply "GIT_REFERENCE_BACKEND" without a repository
  setup: stop applying repository format twice
  setup: inline `check_and_apply_repository_format()`
This commit is contained in:
Junio C Hamano
2026-06-12 15:58:20 -07:00
12 changed files with 83 additions and 119 deletions

View File

@@ -43,32 +43,6 @@ void chdir_notify_unregister(const char *name, chdir_notify_callback cb,
}
}
static void reparent_cb(const char *name,
const char *old_cwd,
const char *new_cwd,
void *data)
{
char **path = data;
char *tmp = *path;
if (!tmp)
return;
*path = reparent_relative_path(old_cwd, new_cwd, tmp);
free(tmp);
if (name) {
trace_printf_key(&trace_setup_key,
"setup: reparent %s to '%s'",
name, *path);
}
}
void chdir_notify_reparent(const char *name, char **path)
{
chdir_notify_register(name, reparent_cb, path);
}
int chdir_notify(const char *new_cwd)
{
struct strbuf old_cwd = STRBUF_INIT;

View File

@@ -19,10 +19,7 @@
* chdir_notify_register("description", foo, data);
*
* In practice most callers will want to move a relative path to the new root;
* they can use the reparent_relative_path() helper for that. If that's all
* you're doing, you can also use the convenience function:
*
* chdir_notify_reparent("description", &my_path);
* they can use the reparent_relative_path() helper for that.
*
* Whenever a chdir event occurs, that will update my_path (if it's relative)
* to adjust for the new cwd by freeing any existing string and allocating a
@@ -43,7 +40,6 @@ typedef void (*chdir_notify_callback)(const char *name,
void chdir_notify_register(const char *name, chdir_notify_callback cb, void *data);
void chdir_notify_unregister(const char *name, chdir_notify_callback cb,
void *data);
void chdir_notify_reparent(const char *name, char **path);
/*
*

35
refs.c
View File

@@ -2351,15 +2351,31 @@ void ref_store_release(struct ref_store *ref_store)
struct ref_store *get_main_ref_store(struct repository *r)
{
enum ref_storage_format format;
if (r->refs_private)
return r->refs_private;
if (!r->gitdir)
BUG("attempting to get main_ref_store outside of repository");
r->refs_private = ref_store_init(r, r->ref_storage_format,
r->gitdir, REF_STORE_ALL_CAPS);
/*
* When constructing the reference backend we'll end up reading the Git
* configuration. This means we'll also try to evaluate "onbranch"
* conditions.
*
* We cannot read branches when constructing the refdb, so it is not
* possible to evaluate those conditions in the first place. To gate
* their evaluation we check whether or not the reference storage
* format has been configured -- we thus have to temporarily set it to
* UNKNOWN here so that we don't end up recursing.
*/
format = r->ref_storage_format;
r->ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
r->refs_private = ref_store_init(r, format, r->gitdir, REF_STORE_ALL_CAPS);
r->refs_private = maybe_debug_wrap_ref_store(r->gitdir, r->refs_private);
r->ref_storage_format = format;
return r->refs_private;
}
@@ -3555,8 +3571,6 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
bool *is_worktree, struct strbuf *refdir,
struct strbuf *ref_common_dir)
{
struct strbuf sb = STRBUF_INIT;
*is_worktree = get_common_dir_noenv(ref_common_dir, gitdir);
if (!payload) {
@@ -3565,15 +3579,16 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
* worktree path, as the 'gitdir' here is already the worktree
* path and is different from 'commondir' denoted by 'ref_common_dir'.
*/
strbuf_reset(refdir);
strbuf_addstr(refdir, gitdir);
return;
goto out;
}
if (!is_absolute_path(payload)) {
strbuf_addf(&sb, "%s/%s", ref_common_dir->buf, payload);
strbuf_realpath(ref_common_dir, sb.buf, 1);
strbuf_addf(ref_common_dir, "/%s", payload);
} else {
strbuf_realpath(ref_common_dir, payload, 1);
strbuf_reset(ref_common_dir);
strbuf_addstr(ref_common_dir, payload);
}
strbuf_addbuf(refdir, ref_common_dir);
@@ -3585,5 +3600,7 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
strbuf_addf(refdir, "/worktrees/%s", wt_id + 1);
}
strbuf_release(&sb);
out:
strbuf_realpath(ref_common_dir, ref_common_dir->buf, 1);
strbuf_realpath(refdir, refdir->buf, 1);
}

View File

@@ -21,7 +21,6 @@
#include "../lockfile.h"
#include "../path.h"
#include "../dir.h"
#include "../chdir-notify.h"
#include "../setup.h"
#include "../worktree.h"
#include "../wrapper.h"
@@ -128,12 +127,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs);
chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir);
chdir_notify_reparent("files-backend $GIT_COMMONDIR",
&refs->gitcommondir);
strbuf_release(&refdir);
return ref_store;
}

View File

@@ -13,7 +13,6 @@
#include "packed-backend.h"
#include "../iterator.h"
#include "../lockfile.h"
#include "../chdir-notify.h"
#include "../statinfo.h"
#include "../worktree.h"
#include "../wrapper.h"
@@ -226,10 +225,9 @@ struct ref_store *packed_ref_store_init(struct repository *repo,
base_ref_store_init(ref_store, repo, gitdir, &refs_be_packed);
refs->store_flags = opts->access_flags;
strbuf_addf(&sb, "%s/packed-refs", gitdir);
refs->path = strbuf_detach(&sb, NULL);
chdir_notify_reparent("packed-refs", &refs->path);
return ref_store;
}

View File

@@ -1,6 +1,5 @@
#include "../git-compat-util.h"
#include "../abspath.h"
#include "../chdir-notify.h"
#include "../config.h"
#include "../dir.h"
#include "../environment.h"
@@ -445,8 +444,6 @@ static struct ref_store *reftable_be_init(struct repository *repo,
goto done;
}
chdir_notify_reparent("reftables-backend $GIT_DIR", &refs->base.gitdir);
done:
assert(refs->err != REFTABLE_API_ERROR);
strbuf_release(&ref_common_dir);

View File

@@ -422,6 +422,11 @@ void repo_clear(struct repository *repo)
FREE_AND_NULL(repo->remote_state);
}
if (repo->refs_private) {
ref_store_release(repo->refs_private);
FREE_AND_NULL(repo->refs_private);
}
strmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)
ref_store_release(e->value);
strmap_clear(&repo->submodule_ref_stores, 1);

96
setup.c
View File

@@ -1808,32 +1808,6 @@ int apply_repository_format(struct repository *repo,
return 0;
}
/*
* Check the repository format version in the path found in repo_get_git_dir(repo),
* and die if it is a version we don't understand. Generally one would
* set_git_dir() before calling this, and use it only for "are we in a valid
* repo?".
*
* If successful and fmt is not NULL, fill fmt with data.
*/
static void check_and_apply_repository_format(struct repository *repo,
struct repository_format *fmt,
enum apply_repository_format_flags flags)
{
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
struct strbuf err = STRBUF_INIT;
if (!fmt)
fmt = &repo_fmt;
check_repository_format_gently(repo_get_git_dir(repo), fmt, NULL);
if (apply_repository_format(repo, fmt, flags, &err) < 0)
die("%s", err.buf);
startup_info->have_repository = 1;
clear_repository_format(&repo_fmt);
}
const char *enter_repo(struct repository *repo, const char *path, unsigned flags)
{
static struct strbuf validated_path = STRBUF_INIT;
@@ -1907,9 +1881,17 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
}
if (is_git_directory(".")) {
struct repository_format fmt = REPOSITORY_FORMAT_INIT;
struct strbuf err = STRBUF_INIT;
set_git_dir(repo, ".", 0);
check_and_apply_repository_format(repo, NULL,
APPLY_REPOSITORY_FORMAT_HONOR_ENV);
check_repository_format_gently(".", &fmt, NULL);
if (apply_repository_format(repo, &fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
startup_info->have_repository = 1;
clear_repository_format(&fmt);
strbuf_release(&err);
return path;
}
@@ -1944,7 +1926,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
static struct strbuf cwd = STRBUF_INIT;
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
const char *prefix = NULL;
const char *ref_backend_uri;
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
/*
@@ -2061,6 +2042,8 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
startup_info->have_repository ||
/* GIT_DIR_EXPLICIT */
getenv(GIT_DIR_ENVIRONMENT)) {
const char *ref_backend_uri;
if (!repo->gitdir) {
const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
if (!gitdir)
@@ -2068,6 +2051,24 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setup_git_env_internal(repo, gitdir);
}
/*
* The env variable should override the repository config
* for 'extensions.refStorage'.
*/
ref_backend_uri = getenv(GIT_REFERENCE_BACKEND_ENVIRONMENT);
if (ref_backend_uri) {
char *format;
free(repo_fmt.ref_storage_payload);
parse_reference_uri(ref_backend_uri, &format, &repo_fmt.ref_storage_payload);
repo_fmt.ref_storage_format = ref_storage_format_by_name(format);
if (repo_fmt.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)
die(_("unknown ref storage format: '%s'"), format);
free(format);
}
if (startup_info->have_repository) {
struct strbuf err = STRBUF_INIT;
@@ -2095,25 +2096,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
}
/*
* The env variable should override the repository config
* for 'extensions.refStorage'.
*/
ref_backend_uri = getenv(GIT_REFERENCE_BACKEND_ENVIRONMENT);
if (ref_backend_uri) {
char *backend, *payload;
enum ref_storage_format format;
parse_reference_uri(ref_backend_uri, &backend, &payload);
format = ref_storage_format_by_name(backend);
if (format == REF_STORAGE_FORMAT_UNKNOWN)
die(_("unknown ref storage format: '%s'"), backend);
repo_set_ref_storage_format(repo, format, payload);
free(backend);
free(payload);
}
setup_original_cwd(repo);
strbuf_release(&dir);
@@ -2748,8 +2730,7 @@ out:
return ret;
}
static void repository_format_configure(struct repository *repo,
struct repository_format *repo_fmt,
static void repository_format_configure(struct repository_format *repo_fmt,
int hash, enum ref_storage_format ref_format)
{
struct default_format_config cfg = {
@@ -2786,7 +2767,6 @@ static void repository_format_configure(struct repository *repo,
} else if (cfg.hash != GIT_HASH_UNKNOWN) {
repo_fmt->hash_algo = cfg.hash;
}
repo_set_hash_algo(repo, repo_fmt->hash_algo);
env = getenv("GIT_DEFAULT_REF_FORMAT");
if (repo_fmt->version >= 0 &&
@@ -2824,9 +2804,6 @@ static void repository_format_configure(struct repository *repo,
free(backend);
}
repo_set_ref_storage_format(repo, repo_fmt->ref_storage_format,
repo_fmt->ref_storage_payload);
}
int init_db(struct repository *repo,
@@ -2840,6 +2817,7 @@ int init_db(struct repository *repo,
int exist_ok = flags & INIT_DB_EXIST_OK;
char *original_git_dir = real_pathdup(git_dir, 1);
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
struct strbuf err = STRBUF_INIT;
if (real_git_dir) {
struct stat st;
@@ -2866,10 +2844,11 @@ int init_db(struct repository *repo,
* config file, so this will not fail. What we are catching
* is an attempt to reinitialize new repository with an old tool.
*/
check_and_apply_repository_format(repo, &repo_fmt,
APPLY_REPOSITORY_FORMAT_HONOR_ENV);
repository_format_configure(repo, &repo_fmt, hash, ref_storage_format);
check_repository_format_gently(repo_get_git_dir(repo), &repo_fmt, NULL);
repository_format_configure(&repo_fmt, hash, ref_storage_format);
if (apply_repository_format(repo, &repo_fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
startup_info->have_repository = 1;
/*
* Ensure `core.hidedotfiles` is processed. This must happen after we
@@ -2924,6 +2903,7 @@ int init_db(struct repository *repo,
}
clear_repository_format(&repo_fmt);
strbuf_release(&err);
free(original_git_dir);
return 0;
}

View File

@@ -237,7 +237,7 @@ test_expect_success 'reject packed-refs with unterminated line' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%s" "$HEAD refs/zzzzz" >>.git/packed-refs &&
echo "fatal: unterminated line in .git/packed-refs: $HEAD refs/zzzzz" >expected_err &&
echo "fatal: unterminated line in $(pwd)/.git/packed-refs: $HEAD refs/zzzzz" >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'
@@ -246,7 +246,7 @@ test_expect_success 'reject packed-refs containing junk' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%s\n" "bogus content" >>.git/packed-refs &&
echo "fatal: unexpected line in .git/packed-refs: bogus content" >expected_err &&
echo "fatal: unexpected line in $(pwd)/.git/packed-refs: bogus content" >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'
@@ -255,7 +255,7 @@ test_expect_success 'reject packed-refs with a short SHA-1' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%.7s %s\n" $HEAD refs/zzzzz >>.git/packed-refs &&
printf "fatal: unexpected line in .git/packed-refs: %.7s %s\n" $HEAD refs/zzzzz >expected_err &&
printf "fatal: unexpected line in $(pwd)/.git/packed-refs: %.7s %s\n" $HEAD refs/zzzzz >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'

View File

@@ -96,7 +96,7 @@ test_expect_success 'non-empty directory blocks create' - <<\EOT
: >.git/$prefix/foo/bar/baz.lock &&
test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
cat >expected <<-EOF &&
fatal: cannot lock ref '$prefix/foo': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'
fatal: cannot lock ref '$prefix/foo': there is a non-empty directory '$(pwd)/.git/$prefix/foo' blocking reference '$prefix/foo'
EOF
printf "%s\n" "update $prefix/foo $C" |
test_must_fail git update-ref --stdin 2>output.err &&
@@ -135,7 +135,7 @@ test_expect_success 'non-empty directory blocks indirect create' - <<\EOT
: >.git/$prefix/foo/bar/baz.lock &&
test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
cat >expected <<-EOF &&
fatal: cannot lock ref '$prefix/symref': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'
fatal: cannot lock ref '$prefix/symref': there is a non-empty directory '$(pwd)/.git/$prefix/foo' blocking reference '$prefix/foo'
EOF
printf "%s\n" "update $prefix/symref $C" |
test_must_fail git update-ref --stdin 2>output.err &&

View File

@@ -145,7 +145,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
test_path_is_dir "$BACKEND_PATH" &&
test_refs_backend . $from_format "$to_format://$BACKEND_PATH" "$method"
)
'
@@ -160,7 +161,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
test_path_is_dir "$BACKEND_PATH" &&
test_refs_backend . $from_format "$to_format://$BACKEND_PATH" "$method" &&
@@ -187,7 +189,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
test_path_is_dir "$BACKEND_PATH" &&
run_with_uri . "$from_format" "$to_format://$BACKEND_PATH" \
"worktree add ../wt 2" "$method" &&

View File

@@ -1741,7 +1741,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti
cd case_insensitive &&
git remote add origin -- ../case_sensitive_df &&
test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err &&
test_grep "cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}./refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}" err &&
test_grep "cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}$(pwd)/refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}" err &&
git rev-parse refs/heads/main >expect &&
git rev-parse refs/heads/Foo/bar >actual &&
test_cmp expect actual