Taylor Blau <me@xxxxxxxxxxxx> writes: > Thanks for working on this! I'm excited to see some patches here, though > I'm not totally convinced of this direction. More below. > > To summarize, this proposal attempts to work around the problem of > embedding bare repositories in non-bare checkouts by providing a way to > opt-out of bare repository discovery (which is to only discover things > that are listed in the safe.bareRepository configuration). > > I agree that this would prevent the problem you're trying to solve, but > I have significant concerns that this patch is going too far (at the > risk of future damage to unrelated workflows) in order to accomplish > that goal. > > My concern is that if we ever flipped the default (i.e. that > "safe.bareRepository" might someday be ""), that many legitimate cases > of using bare repositories would be broken. I think there are many such > legitimate use cases that _do_ rely on discovering bare repositories > (i.e., Git invocations that do not have a `--git-dir` in their > command-line). I think 99% of such use is to chdir into the directory with HEAD, refs/ and objects/ in it and let git recognise the cwd is a git directory. Am I mistaken, or are there tools that chdir into objects/08/ and rely on setup_git_directory_gently_1() to find the parent directory of that 'objects' directory to be a git directory? I am wondering if another knob to help that particular use case easier may be sufficient. If you are a forge operator, you'd just set a boolean configuration variable to say "it is sufficient to chdir into a directory to use it a bare repository without exporting the environment variable GIT_DIR=." It is likely that end-user human users would not want to enable such a variable, of course, but I wonder if a simple single knob would be sufficient to help other use cases you are worried about? While I wish "extensions and nothing else", i.e. we use "degraded access", not "refuse to give access at all", were workable, I am pessimistic that it would work well in practice. Saying "nothing else" is easy, but we do "if X exists, use it" for hook, and to implement "nothing else", you'd need to find such a code and say "even if X exists, because we are in this strange embedded bare thing, ignore this part of the logic" for every X. We've been casually saying "potentially risky config" and then started mixing "hooks" in the discussion, but who knows what other things are used from the repository by third-party tools that we need to yet add to the mix? > I'm still not convinced that just reading repository extensions while > ignoring the rest of config and hooks is too confusing, so I'd be more > in favor of something like: I do not think it would be confusing. I am worried about it being error prone. > $ git.compile rev-parse --absolute-git-dir > warning: ignoring repository config and hooks > advice: to permit bare repository discovery (which > advice: will read config and hooks), consider running: > advice: > advice: $ git config --global --add safe.bareRepository /home/ttaylorr/repo.git > /home/ttaylorr/repo.git Is the last line meant to be an output from "rev-parse --absolute-git-dir"? IOW, the warning says you are ignoring, but we are still recognising it as a repository? By the way, do we need safe.bareRepository? Shouldn't safe.directory cover the same purpose? If a directory is on the latter, you are saying that (1) the directory is OK to use as a repository, and (2) it is so even if the directory is owned by somebody else, not you. Theoretically you can argue that there can be cases where you only want (1) and not (2), but as long as you control such a directory (like an embedded repository in your project's checkout) yourself, you do not have to worry about the "ok even if it is owned by somebody else" part.
