On Tue, Mar 1, 2016 at 3:35 PM, Jeff King <peff@xxxxxxxx> wrote: > On Mon, Feb 29, 2016 at 07:52:34PM -0500, David Turner wrote: > >> Usually, git calls some form of setup_git_directory at startup. But >> sometimes, it doesn't. Usually, that's OK because it's not really >> using the repository. But in some cases, it is using the repo. In >> those cases, either setup_git_directory_gently must be called, or the >> repository (e.g. the refs) must not be accessed. > > It's actually not just setup_git_directory(). We can also use > check_repository_format(), which is used by enter_repo() (and hence by > things like upload-pack). I think the rule really ought to be: if we > didn't have check_repository_format_gently() tell us we have a valid > repo, we should not access any repo elements (refs, objects, etc). Agreed. There's also a lighter version of check_repo.. which is is_git_directory(). Most of the time we just want to answer the question "is it a valid repository? support or not does not matter". We probably need more eyes on submodule case when this functino is used. For example in 25/33 [1] we check if a repo is non-bare (a variant of is_git_directory) then we peek the config file inside. Should check_repository_format() be done in this case? You know what, forget my question. The answer is yes. After writing all that, I remember that part of the config file may be moved away in the next version of multiple worktrees [2]. We need proper repo validation before reading anything inside. [1] http://article.gmane.org/gmane.comp.version-control.git/287959 [2] http://article.gmane.org/gmane.comp.version-control.git/284803 > I started earlier today on a patch series to identify and fix these > cases independent of your series. Yes this sounds like a separate problem, even though it's raised by lmdb topic. > The basic strategy was to adapt the > existing "struct startup_info" to be available everywhere, and have > relevant bits of code assert() on it, or even behave differently (e.g., > if some library code should do different things in a repo versus not). startup_info is NULL for external programs if I remember correctly, or do you make it available to all of them too? -- Duy -- 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