Skip to content

Commit 407997c

Browse files
pks-tgitster
authored andcommitted
setup: fix bug with "includeIf.onbranch" when initializing dir
It was reported that git-init(1) can fail when initializing an existing directory in case the config contains an "includeIf.onbranch:" condition: $ mkdir repo $ git -c includeIf.onbranch:main.path=nonexistent init repo BUG: refs.c:2056: reference backend is unknown The same error can also be triggered when re-initializing an already existing repository. The bug has been introduced in 173761e (setup: start tracking ref storage format, 2023-12-29), which wired up the ref storage format. The root cause is in `init_db()`, which tries to read the config before we have initialized `the_repository` and most importantly its ref storage format. We eventually end up calling `include_by_branch()` and execute `refs_resolve_ref_unsafe()`, but because we have not initialized the ref storage format yet this will trigger the above bug. Interestingly, `include_by_branch()` has a mechanism that will only cause us to resolve the ref when `the_repository->gitdir` is set. This is also the reason why this only happens when we initialize an already existing directory or repository: `gitdir` is set in those cases, but not when creating a new directory. Now there are two ways to address the issue: - We can adapt `include_by_branch()` to also make the code conditional on whether `the_repository->ref_storage_format` is set. - We can shift around code such that we initialize the repository format before we read the config. While the first approach would be safe, it may also cause us to paper over issues where a ref store should have been set up. In our case for example, it may be reasonable to expect that re-initializing the repo will cause the "onbranch:" condition to trigger, but we would not do that if the ref storage format was not set up yet. This also used to work before the above commit that introduced this bug. Rearrange the code such that we set up the repository format before reading the config. This fixes the bug and ensures that "onbranch:" conditions can trigger. Reported-by: Heghedus Razvan <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]> Tested-by: Heghedus Razvan <[email protected]> [jc: fixed a test and backported to v2.44.0 codebase] Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3c2a3fd commit 407997c

File tree

2 files changed

+108
-19
lines changed

2 files changed

+108
-19
lines changed

setup.c

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2196,13 +2196,6 @@ int init_db(const char *git_dir, const char *real_git_dir,
21962196
}
21972197
startup_info->have_repository = 1;
21982198

2199-
/* Ensure `core.hidedotfiles` is processed */
2200-
git_config(platform_core_config, NULL);
2201-
2202-
safe_create_dir(git_dir, 0);
2203-
2204-
prev_bare_repository = is_bare_repository();
2205-
22062199
/* Check to see if the repository version is right.
22072200
* Note that a newly created repository does not have
22082201
* config file, so this will not fail. What we are catching
@@ -2213,17 +2206,28 @@ int init_db(const char *git_dir, const char *real_git_dir,
22132206
validate_hash_algorithm(&repo_fmt, hash);
22142207
validate_ref_storage_format(&repo_fmt, ref_storage_format);
22152208

2216-
reinit = create_default_files(template_dir, original_git_dir,
2217-
&repo_fmt, prev_bare_repository,
2218-
init_shared_repository);
2219-
22202209
/*
22212210
* Now that we have set up both the hash algorithm and the ref storage
22222211
* format we can update the repository's settings accordingly.
22232212
*/
22242213
repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
22252214
repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);
22262215

2216+
/*
2217+
* Ensure `core.hidedotfiles` is processed. This must happen after we
2218+
* have set up the repository format such that we can evaluate
2219+
* includeIf conditions correctly in the case of re-initialization.
2220+
*/
2221+
git_config(platform_core_config, NULL);
2222+
2223+
safe_create_dir(git_dir, 0);
2224+
2225+
prev_bare_repository = is_bare_repository();
2226+
2227+
reinit = create_default_files(template_dir, original_git_dir,
2228+
&repo_fmt, prev_bare_repository,
2229+
init_shared_repository);
2230+
22272231
if (!(flags & INIT_DB_SKIP_REFDB))
22282232
create_reference_database(repo_fmt.ref_storage_format,
22292233
initial_branch, flags & INIT_DB_QUIET);

t/t0001-init.sh

Lines changed: 93 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -584,14 +584,39 @@ test_expect_success 'init with --ref-format=files' '
584584
test_cmp expect actual
585585
'
586586

587-
test_expect_success 're-init with same format' '
588-
test_when_finished "rm -rf refformat" &&
589-
git init --ref-format=files refformat &&
590-
git init --ref-format=files refformat &&
591-
echo files >expect &&
592-
git -C refformat rev-parse --show-ref-format >actual &&
593-
test_cmp expect actual
594-
'
587+
backends="files"
588+
for from_format in $backends
589+
do
590+
test_expect_success "re-init with same format ($from_format)" '
591+
test_when_finished "rm -rf refformat" &&
592+
git init --ref-format=$from_format refformat &&
593+
git init --ref-format=$from_format refformat &&
594+
echo $from_format >expect &&
595+
git -C refformat rev-parse --show-ref-format >actual &&
596+
test_cmp expect actual
597+
'
598+
599+
for to_format in $backends
600+
do
601+
if test "$from_format" = "$to_format"
602+
then
603+
continue
604+
fi
605+
606+
test_expect_success "re-init with different format fails ($from_format -> $to_format)" '
607+
test_when_finished "rm -rf refformat" &&
608+
git init --ref-format=$from_format refformat &&
609+
cat >expect <<-EOF &&
610+
fatal: attempt to reinitialize repository with different reference storage format
611+
EOF
612+
test_must_fail git init --ref-format=$to_format refformat 2>err &&
613+
test_cmp expect err &&
614+
echo $from_format >expect &&
615+
git -C refformat rev-parse --show-ref-format >actual &&
616+
test_cmp expect actual
617+
'
618+
done
619+
done
595620

596621
test_expect_success 'init with --ref-format=garbage' '
597622
test_when_finished "rm -rf refformat" &&
@@ -678,4 +703,64 @@ test_expect_success 'branch -m with the initial branch' '
678703
test_cmp expect actual
679704
'
680705

706+
test_expect_success 'init with includeIf.onbranch condition' '
707+
test_when_finished "rm -rf repo" &&
708+
git -c includeIf.onbranch:main.path=nonexistent init repo &&
709+
echo $GIT_DEFAULT_REF_FORMAT >expect &&
710+
git -C repo rev-parse --show-ref-format >actual &&
711+
test_cmp expect actual
712+
'
713+
714+
test_expect_success 'init with includeIf.onbranch condition with existing directory' '
715+
test_when_finished "rm -rf repo" &&
716+
mkdir repo &&
717+
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
718+
echo $GIT_DEFAULT_REF_FORMAT >expect &&
719+
git -C repo rev-parse --show-ref-format >actual &&
720+
test_cmp expect actual
721+
'
722+
723+
test_expect_success 're-init with includeIf.onbranch condition' '
724+
test_when_finished "rm -rf repo" &&
725+
git init repo &&
726+
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
727+
echo $GIT_DEFAULT_REF_FORMAT >expect &&
728+
git -C repo rev-parse --show-ref-format >actual &&
729+
test_cmp expect actual
730+
'
731+
732+
test_expect_success 're-init with includeIf.onbranch condition' '
733+
test_when_finished "rm -rf repo" &&
734+
git init repo &&
735+
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
736+
echo $GIT_DEFAULT_REF_FORMAT >expect &&
737+
git -C repo rev-parse --show-ref-format >actual &&
738+
test_cmp expect actual
739+
'
740+
741+
test_expect_success 're-init skips non-matching includeIf.onbranch' '
742+
test_when_finished "rm -rf repo config" &&
743+
cat >config <<-EOF &&
744+
[
745+
garbage
746+
EOF
747+
git init repo &&
748+
git -c includeIf.onbranch:nonexistent.path="$(test-tool path-utils absolute_path config)" init repo
749+
'
750+
751+
test_expect_success 're-init reads matching includeIf.onbranch' '
752+
test_when_finished "rm -rf repo config" &&
753+
cat >config <<-EOF &&
754+
[
755+
garbage
756+
EOF
757+
path="$(test-tool path-utils absolute_path config)" &&
758+
git init --initial-branch=branch repo &&
759+
cat >expect <<-EOF &&
760+
fatal: bad config line 1 in file $path
761+
EOF
762+
test_must_fail git -c includeIf.onbranch:branch.path="$path" init repo 2>err &&
763+
test_cmp expect err
764+
'
765+
681766
test_done

0 commit comments

Comments
 (0)