diff options
author | Ghanshyam Thakkar <shyamthakkar001@gmail.com> | 2024-03-04 20:48:11 +0530 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2024-03-04 10:18:31 -0800 |
commit | 8145a8fd024f8191f58e2611981b90da0c9b6453 (patch) | |
tree | 30ff813e832bbf1abd5ec9b8011b9a88278fdb57 /setup.c | |
parent | b387623c12f3f4a376e4d35a610fd3e55d7ea907 (diff) | |
download | git-8145a8fd024f8191f58e2611981b90da0c9b6453.tar.gz |
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/xmqqjzonpy9l.fsf@gitster.g/
Helped-by: Elijah Newren <newren@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'setup.c')
-rw-r--r-- | setup.c | 36 |
1 files changed, 3 insertions, 33 deletions
@@ -1961,7 +1961,6 @@ void create_reference_database(unsigned int ref_storage_format, static int create_default_files(const char *template_path, const char *original_git_dir, const struct repository_format *fmt, - int prev_bare_repository, int init_shared_repository) { struct stat st1; @@ -1996,34 +1995,8 @@ static int create_default_files(const char *template_path, */ if (init_shared_repository != -1) set_shared_repository(init_shared_repository); - /* - * TODO: heed core.bare from config file in templates if no - * command-line override given - */ - is_bare_repository_cfg = prev_bare_repository || !work_tree; - /* TODO (continued): - * - * 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. - */ + + is_bare_repository_cfg = !work_tree; /* * 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, 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; - int prev_bare_repository; if (real_git_dir) { struct stat st; @@ -2201,7 +2173,6 @@ int init_db(const char *git_dir, const char *real_git_dir, 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 @@ -2214,8 +2185,7 @@ int init_db(const char *git_dir, const char *real_git_dir, 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); + &repo_fmt, init_shared_repository); /* * Now that we have set up both the hash algorithm and the ref storage |