On Fri, Jun 26, 2015 at 6:56 PM, Jeff King <peff@xxxxxxxx> wrote: > On Fri, Jun 26, 2015 at 05:37:35PM +0700, Nguyễn Thái Ngọc Duy wrote: > >> This is where the "fun" is. The legacy behavior is, if $GIT_WORK_TREE is >> not set but $GIT_DIR is, cwd is chosen as worktree's top. If you happen >> to stand at worktree's top when you do this, all is well. If you are in >> a subdir "foo/bar" (real worktree's top is "foo"), this behavior bites >> you: your detected worktree is now "foo/bar", but the first run >> correctly detected worktree as "foo". You get "internal error: work tree >> has already been set" as a result. > > I think this makes sense. I feel like we've dealt with this before, but > the two previous rounds I found were basically: > > - we have GIT_IMPLICIT_WORK_TREE, but that is for the _opposite_ case. > I.e., when we do not have a work tree and must communicate so to > later code (including sub-processes). > > - a discussion about switching the "work tree defaults to '.' when > $GIT_DIR is set" behavior yielded almost the identical patch: > > http://article.gmane.org/gmane.comp.version-control.git/219196 > > but we were so wrapped up in the greater discussion we did not apply > that simple fix. :) There's also the patch "[PATCH v2] setup.c: set workdir when gitdir is not default" from Max Kirillov last year (gmane down, no link), so we have one patch about this every year since 2013 :) Junio if you are worried about unnecessary setenv, I think Max's approach is safer as he solves a specific use case. If this problem pops up again in another use case, we can deal with that again. >> + if (setenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1)) >> + error("Could not set GIT_WORK_TREE to '%s'", work_tree); > > Should this be die()? setenv() should basically never fail, but if it > does, it could be confusing and/or dangerous to run without the variable > set. It's a straight copy from set_git_dir() but I guess the situation is a bit different. If setenv fails in gitdir, no repo is found but if it fails here we may select a wrong worktree and could wipe it out by mistake. Will fix if Junio chooses this patch instead of Max's. -- 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