Skip to content

Commit

Permalink
setup: fix bug with "includeIf.onbranch" when initializing dir
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
pks-t authored and gitster committed May 23, 2024
1 parent 3c2a3fd commit 407997c
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 19 deletions.
26 changes: 15 additions & 11 deletions setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -2196,13 +2196,6 @@ int init_db(const char *git_dir, const char *real_git_dir,
}
startup_info->have_repository = 1;

/* Ensure `core.hidedotfiles` is processed */
git_config(platform_core_config, NULL);

safe_create_dir(git_dir, 0);

prev_bare_repository = is_bare_repository();

/* Check to see if the repository version is right.
* Note that a newly created repository does not have
* config file, so this will not fail. What we are catching
Expand All @@ -2213,17 +2206,28 @@ int init_db(const char *git_dir, const char *real_git_dir,
validate_hash_algorithm(&repo_fmt, hash);
validate_ref_storage_format(&repo_fmt, ref_storage_format);

reinit = create_default_files(template_dir, original_git_dir,
&repo_fmt, prev_bare_repository,
init_shared_repository);

/*
* Now that we have set up both the hash algorithm and the ref storage
* format we can update the repository's settings accordingly.
*/
repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format);

/*
* Ensure `core.hidedotfiles` is processed. This must happen after we
* have set up the repository format such that we can evaluate
* includeIf conditions correctly in the case of re-initialization.
*/
git_config(platform_core_config, NULL);

safe_create_dir(git_dir, 0);

prev_bare_repository = is_bare_repository();

reinit = create_default_files(template_dir, original_git_dir,
&repo_fmt, prev_bare_repository,
init_shared_repository);

if (!(flags & INIT_DB_SKIP_REFDB))
create_reference_database(repo_fmt.ref_storage_format,
initial_branch, flags & INIT_DB_QUIET);
Expand Down
101 changes: 93 additions & 8 deletions t/t0001-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -584,14 +584,39 @@ test_expect_success 'init with --ref-format=files' '
test_cmp expect actual
'

test_expect_success 're-init with same format' '
test_when_finished "rm -rf refformat" &&
git init --ref-format=files refformat &&
git init --ref-format=files refformat &&
echo files >expect &&
git -C refformat rev-parse --show-ref-format >actual &&
test_cmp expect actual
'
backends="files"
for from_format in $backends
do
test_expect_success "re-init with same format ($from_format)" '
test_when_finished "rm -rf refformat" &&
git init --ref-format=$from_format refformat &&
git init --ref-format=$from_format refformat &&
echo $from_format >expect &&
git -C refformat rev-parse --show-ref-format >actual &&
test_cmp expect actual
'

for to_format in $backends
do
if test "$from_format" = "$to_format"
then
continue
fi

test_expect_success "re-init with different format fails ($from_format -> $to_format)" '
test_when_finished "rm -rf refformat" &&
git init --ref-format=$from_format refformat &&
cat >expect <<-EOF &&
fatal: attempt to reinitialize repository with different reference storage format
EOF
test_must_fail git init --ref-format=$to_format refformat 2>err &&
test_cmp expect err &&
echo $from_format >expect &&
git -C refformat rev-parse --show-ref-format >actual &&
test_cmp expect actual
'
done
done

test_expect_success 'init with --ref-format=garbage' '
test_when_finished "rm -rf refformat" &&
Expand Down Expand Up @@ -678,4 +703,64 @@ test_expect_success 'branch -m with the initial branch' '
test_cmp expect actual
'

test_expect_success 'init with includeIf.onbranch condition' '
test_when_finished "rm -rf repo" &&
git -c includeIf.onbranch:main.path=nonexistent init repo &&
echo $GIT_DEFAULT_REF_FORMAT >expect &&
git -C repo rev-parse --show-ref-format >actual &&
test_cmp expect actual
'

test_expect_success 'init with includeIf.onbranch condition with existing directory' '
test_when_finished "rm -rf repo" &&
mkdir repo &&
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
echo $GIT_DEFAULT_REF_FORMAT >expect &&
git -C repo rev-parse --show-ref-format >actual &&
test_cmp expect actual
'

test_expect_success 're-init with includeIf.onbranch condition' '
test_when_finished "rm -rf repo" &&
git init repo &&
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
echo $GIT_DEFAULT_REF_FORMAT >expect &&
git -C repo rev-parse --show-ref-format >actual &&
test_cmp expect actual
'

test_expect_success 're-init with includeIf.onbranch condition' '
test_when_finished "rm -rf repo" &&
git init repo &&
git -c includeIf.onbranch:nonexistent.path=/does/not/exist init repo &&
echo $GIT_DEFAULT_REF_FORMAT >expect &&
git -C repo rev-parse --show-ref-format >actual &&
test_cmp expect actual
'

test_expect_success 're-init skips non-matching includeIf.onbranch' '
test_when_finished "rm -rf repo config" &&
cat >config <<-EOF &&
[
garbage
EOF
git init repo &&
git -c includeIf.onbranch:nonexistent.path="$(test-tool path-utils absolute_path config)" init repo
'

test_expect_success 're-init reads matching includeIf.onbranch' '
test_when_finished "rm -rf repo config" &&
cat >config <<-EOF &&
[
garbage
EOF
path="$(test-tool path-utils absolute_path config)" &&
git init --initial-branch=branch repo &&
cat >expect <<-EOF &&
fatal: bad config line 1 in file $path
EOF
test_must_fail git -c includeIf.onbranch:branch.path="$path" init repo 2>err &&
test_cmp expect err
'

test_done

0 comments on commit 407997c

Please sign in to comment.