Skip to content

Commit 8145a8f

Browse files
spectre10gitster
authored andcommitted
setup: remove unnecessary variable
The TODO comment suggested to heed core.bare from template config file if no command line override given. And the prev_bare_repository variable seems to have been placed for this sole purpose as it is not used anywhere else. However, it was clarified by Junio [1] that such values (including core.bare) are ignored intentionally and does not make sense to propagate them from template config to repository config. Also, the directories for the worktree and repository are already created, and therefore the bare/non-bare decision has already been made, by the point we reach the codepath where the TODO comment is placed. Therefore, prev_bare_repository does not have a usecase with/without supporting core.bare from template. And the removal of prev_bare_repository is safe as proved by the later part of the comment: "Unfortunately, the line above is equivalent to is_bare_repository_cfg = !work_tree; which ignores the config entirely even if no `--[no-]bare` command line option was present. To see why, note that before this function, there was this call: prev_bare_repository = is_bare_repository() expanding the right hand side: = is_bare_repository_cfg && !get_git_work_tree() = is_bare_repository_cfg && !work_tree note that the last simplification above is valid because nothing calls repo_init() or set_git_work_tree() between any of the relevant calls in the code, and thus the !get_git_work_tree() calls will return the same result each time. So, what we are interested in computing is the right hand side of the line of code just above this comment: prev_bare_repository || !work_tree = is_bare_repository_cfg && !work_tree || !work_tree = !work_tree because "A && !B || !B == !B" for all boolean values of A & B." Therefore, remove the TODO comment and remove prev_bare_repository variable. Also, update relevant testcases and remove one redundant testcase. [1]: https://lore.kernel.org/git/[email protected]/ Helped-by: Elijah Newren <[email protected]> Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Ghanshyam Thakkar <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b387623 commit 8145a8f

File tree

3 files changed

+8
-49
lines changed

3 files changed

+8
-49
lines changed

setup.c

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1961,7 +1961,6 @@ void create_reference_database(unsigned int ref_storage_format,
19611961
static int create_default_files(const char *template_path,
19621962
const char *original_git_dir,
19631963
const struct repository_format *fmt,
1964-
int prev_bare_repository,
19651964
int init_shared_repository)
19661965
{
19671966
struct stat st1;
@@ -1996,34 +1995,8 @@ static int create_default_files(const char *template_path,
19961995
*/
19971996
if (init_shared_repository != -1)
19981997
set_shared_repository(init_shared_repository);
1999-
/*
2000-
* TODO: heed core.bare from config file in templates if no
2001-
* command-line override given
2002-
*/
2003-
is_bare_repository_cfg = prev_bare_repository || !work_tree;
2004-
/* TODO (continued):
2005-
*
2006-
* Unfortunately, the line above is equivalent to
2007-
* is_bare_repository_cfg = !work_tree;
2008-
* which ignores the config entirely even if no `--[no-]bare`
2009-
* command line option was present.
2010-
*
2011-
* To see why, note that before this function, there was this call:
2012-
* prev_bare_repository = is_bare_repository()
2013-
* expanding the right hand side:
2014-
* = is_bare_repository_cfg && !get_git_work_tree()
2015-
* = is_bare_repository_cfg && !work_tree
2016-
* note that the last simplification above is valid because nothing
2017-
* calls repo_init() or set_git_work_tree() between any of the
2018-
* relevant calls in the code, and thus the !get_git_work_tree()
2019-
* calls will return the same result each time. So, what we are
2020-
* interested in computing is the right hand side of the line of
2021-
* code just above this comment:
2022-
* prev_bare_repository || !work_tree
2023-
* = is_bare_repository_cfg && !work_tree || !work_tree
2024-
* = !work_tree
2025-
* because "A && !B || !B == !B" for all boolean values of A & B.
2026-
*/
1998+
1999+
is_bare_repository_cfg = !work_tree;
20272000

20282001
/*
20292002
* We would have created the above under user's umask -- under
@@ -2175,7 +2148,6 @@ int init_db(const char *git_dir, const char *real_git_dir,
21752148
int exist_ok = flags & INIT_DB_EXIST_OK;
21762149
char *original_git_dir = real_pathdup(git_dir, 1);
21772150
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
2178-
int prev_bare_repository;
21792151

21802152
if (real_git_dir) {
21812153
struct stat st;
@@ -2201,7 +2173,6 @@ int init_db(const char *git_dir, const char *real_git_dir,
22012173

22022174
safe_create_dir(git_dir, 0);
22032175

2204-
prev_bare_repository = is_bare_repository();
22052176

22062177
/* Check to see if the repository version is right.
22072178
* Note that a newly created repository does not have
@@ -2214,8 +2185,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
22142185
validate_ref_storage_format(&repo_fmt, ref_storage_format);
22152186

22162187
reinit = create_default_files(template_dir, original_git_dir,
2217-
&repo_fmt, prev_bare_repository,
2218-
init_shared_repository);
2188+
&repo_fmt, init_shared_repository);
22192189

22202190
/*
22212191
* Now that we have set up both the hash algorithm and the ref storage

t/t1301-shared-repo.sh

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,15 @@ test_expect_success 'shared=all' '
5252
test 2 = $(git config core.sharedrepository)
5353
'
5454

55-
test_expect_failure 'template can set core.bare' '
55+
test_expect_success 'template cannot set core.bare' '
5656
test_when_finished "rm -rf subdir" &&
5757
test_when_finished "rm -rf templates" &&
5858
test_config core.bare true &&
5959
umask 0022 &&
6060
mkdir -p templates/ &&
6161
cp .git/config templates/config &&
6262
git init --template=templates subdir &&
63-
test_path_exists subdir/HEAD
64-
'
65-
66-
test_expect_success 'template can set core.bare but overridden by command line' '
67-
test_when_finished "rm -rf subdir" &&
68-
test_when_finished "rm -rf templates" &&
69-
test_config core.bare true &&
70-
umask 0022 &&
71-
mkdir -p templates/ &&
72-
cp .git/config templates/config &&
73-
git init --no-bare --template=templates subdir &&
74-
test_path_exists subdir/.git/HEAD
63+
test_path_is_missing subdir/HEAD
7564
'
7665

7766
test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository' '

t/t5606-clone-options.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,14 @@ test_expect_success 'prefers -c config over --template config' '
120120
121121
'
122122

123-
test_expect_failure 'prefers --template config even for core.bare' '
123+
test_expect_success 'ignore --template config for core.bare' '
124124
125125
template="$TRASH_DIRECTORY/template-with-bare-config" &&
126126
mkdir "$template" &&
127127
git config --file "$template/config" core.bare true &&
128128
git clone "--template=$template" parent clone-bare-config &&
129-
test "$(git -C clone-bare-config config --local core.bare)" = "true" &&
130-
test_path_is_file clone-bare-config/HEAD
129+
test "$(git -C clone-bare-config config --local core.bare)" = "false" &&
130+
test_path_is_missing clone-bare-config/HEAD
131131
'
132132

133133
test_expect_success 'prefers config "clone.defaultRemoteName" over default' '

0 commit comments

Comments
 (0)