Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> /*
> * We cannot decide in this function whether we are in the work tree or
> * not, since the config can only be read _after_ this function was called.
> + *
> + * Also, we avoid changing any global state (such as the current working
> + * directory) to allow early callers.
> + *
> + * The directory where the search should start needs to be passed in via the
> + * `dir` parameter; upon return, the `dir` buffer will contain the path of
> + * the directory where the search ended, and `gitdir` will contain the path of
> + * the discovered .git/ directory, if any. This path may be relative against
> + * `dir` (i.e. *not* necessarily the cwd).
I had to read the last sentence three times because my earlier two
attempts misread/misinterpreted as "this may be relative to cwd, but
not necessarily, because this may be relative to dir". The correct
reading is "when this is relative, it is relative to dir. It would
not be relative to cwd if you start with dir that is not cwd".
It would be nicer if all readers can get to that without re-reading
three times.
> -static const char *setup_git_directory_gently_1(int *nongit_ok)
> +static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
> ...
> @@ -889,63 +888,104 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> */
> one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0);
> if (one_filesystem)
> - current_device = get_device_or_die(".", NULL, 0);
> + current_device = get_device_or_die(dir->buf, NULL, 0);
> for (;;) {
> - gitfile = (char*)read_gitfile(DEFAULT_GIT_DIR_ENVIRONMENT);
> - if (gitfile)
> - gitdirenv = gitfile = xstrdup(gitfile);
> - else {
> - if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
> - gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
> + int offset = dir->len;
> +
> + if (offset > min_offset)
> + strbuf_addch(dir, '/');
> + strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
> + gitdirenv = read_gitfile(dir->buf);
> + if (!gitdirenv && is_git_directory(dir->buf))
> + gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
> + strbuf_setlen(dir, offset);
> + if (gitdirenv) {
> + strbuf_addstr(gitdir, gitdirenv);
> + return GIT_DIR_DISCOVERED;
> }
I commented on this part more extensively in a later step of the
series after the read_gitfile() call is replaced with the non-dying
version. I see that issues in corner cases are inherited from the
code even before this step.
We'd either want to at least document the corner cases that are not
handled with /* NEEDSWORK: */ in-code comments , or address them in
a follow-up series before we forget. They are not new problems, so
I am OK if we choose to leave them broken, as people haven't triggered
them in the current code.
Thanks.