On Sun, Oct 16, 2011 at 10:17:10AM -0700, Junio C Hamano wrote: > >> * jk/pull-rebase-with-work-tree (2011-10-13) 1 commit > >> - pull,rebase: handle GIT_WORK_TREE better > >> > >> Looked reasonable. > >> Will merge to 'next'. > > > > I'm not so sure. Didn't you demonstrate that cd_to_toplevel as-is will > > not actually go to the toplevel if we're outside of the work tree? > > > > And changing it is non-trivial, because there may be weird cases that > > rely on staying there? See my final note in the thread: > > > > http://article.gmane.org/gmane.comp.version-control.git/183519 > > Hmm, I might be mistaken, but my impression was that sane people do not do > so, that the discussion that originated this proposed patch was not such a > use case, and most importantly that fixing unsane ones is just the matter > for them to set GIT_WORKING_TREE correctly. So if anything, wouldn't > getting this in as early as possible to 'master' or at least 'next' help > catching a flaw in the above logic and possible downside in the real > world? Hmm. I thought there were two separate problems: 1. my analysis was wrong, and "git rev-parse --show-toplevel" did not actually show the root of the work tree when we were outside it (and therefore cd_to_toplevel did not actually go anywhere) 2. some people might be outside of the work tree, set GIT_DIR explicitly, but not bother setting GIT_WORK_TREE But I was wrong on (1). If GIT_WORK_TREE is set, we _will_ actually go to its work tree, which is what we want. If it's not set, then we go nowhere, and assume the cwd is the work tree. Which is compatible with current behavior, and makes (2) still work. So I was thinking there was more problem then there is. I agree we should let it go to 'next' to shake out any bugs. Sorry for the noise. -Peff -- 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