Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> setup.c | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index e067292..43a8609 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -350,14 +350,17 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> /* config may override worktree */
> if (check_repository_format_gently(nongit_ok))
> return NULL;
> + set_git_dir(gitdirenv);
> return retval;
> }
> if (check_repository_format_gently(nongit_ok))
> return NULL;
> retval = get_relative_cwd(buffer, sizeof(buffer) - 1,
> get_git_work_tree());
> - if (!retval || !*retval)
> + if (!retval || !*retval) {
> + set_git_dir(gitdirenv);
> return NULL;
> + }
> set_git_dir(make_absolute_path(gitdirenv));
> if (chdir(work_tree_env) < 0)
> die_errno ("Could not chdir to '%s'", work_tree_env);
> @@ -392,8 +395,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> offset = len = strlen(cwd);
> for (;;) {
> gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
> - if (gitfile_dir || is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT)) {
> - if (gitfile_dir && set_git_dir(gitfile_dir))
> + if (!gitfile_dir && is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
> + gitfile_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
> + if (gitfile_dir) {
> + if (set_git_dir(gitfile_dir))
> die("Repository setup failed");
> inside_git_dir = 0;
> if (!work_tree_env)
Yes, you have more calls to set_git_dir() than before. But it is not
explained why "calling set_git_dir explicitly" is a good thing anywhere in
the series.
Can you write down the set of rules for your setup sequence, the goal
after everything from this 37-patch series is applied? Something along
the lines of (just illustrating the kinds of things to be described):
- Definition.
The following state variables belong to the setup system:
- git_dir: holds the location of $GIT_DIR as a path relative to cwd
- is_bare_repository(): returns foo;
- is_inside_working_tree(): returns bar;
- ...
- Rule for the callers of the setup system:
Once main() starts, this and that needs to be called in this order
before trying to access any of the above state. Specifically, the
following call could access state variables, and should not be called
before this set-up is done:
- git_config(): needs to know where git_dir is;
- setup_revisions(), parse_options(), ...: needs to know the prefix;
- ...
- Rule for the implementation of the setup system:
Upon the first call the caller makes into the setup system:
1. If the caller does not care about being in a git repository,
skip everything up to step #n.
2. First inspect GIT_DIR; if set, go to step #5.
3. Otherwise try to find GIT_DIR by checking .git or "." is a git
directory; repeat this step by checking one directory closer to the
root level until GIT_DIR is found or CEILING is reached.
4. If there is no GIT_DIR, then chdir back to the original location,
and skip everything up to step #m.
5. As a side effect of finding the GIT_DIR, the configuration file is
read from there, and we will know the value of core.worktree.
6. Inspect GIT_WORK_TREE; if set then git_work_tree is known.
Otherwise, if core.worktree is found in step #5, that is the value
of git_work_tree. Otherwise determine git_work_tree in this
fashon. ...
...
Without seeing such a design that you can clearly explain to others, the
series looks nothing more than a sequence of "I noticed this so am
applying this random patch" and is impossible to review.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html