Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
Duy Nguyenwrites: > On Thu, Dec 3, 2015 at 7:17 PM, Nguyễn Thái Ngọc Duy > wrote: >> The solution in d95138e is reverted in this commit. Instead we reuse the >> solution from c056261 [4]. c056261 fixes another setup-messed-up-by-alias >> by saving and restoring env for git-clone and git-init. Now I conclude >> that setup-messed-by-alias is always evil. So the env restoration is >> done for _all_ commands whenever aliases are involved. It fixes what >> d95138e tries to fix. And it does not upset git-clone-inside-hooks. > > (Reviewer hat on) I wrote env restoration is done for all commands, > but the patch is actually about all _builtin_ commands. External ones > do not receive this treatment. FIx is in the next reroll. Thanks for being careful. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
On Thu, Dec 3, 2015 at 7:17 PM, Nguyễn Thái Ngọc Duywrote: > The solution in d95138e is reverted in this commit. Instead we reuse the > solution from c056261 [4]. c056261 fixes another setup-messed-up-by-alias > by saving and restoring env for git-clone and git-init. Now I conclude > that setup-messed-by-alias is always evil. So the env restoration is > done for _all_ commands whenever aliases are involved. It fixes what > d95138e tries to fix. And it does not upset git-clone-inside-hooks. (Reviewer hat on) I wrote env restoration is done for all commands, but the patch is actually about all _builtin_ commands. External ones do not receive this treatment. FIx is in the next reroll. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
Nguyễn Thái Ngọc Duywrites: > ... Now I conclude > that setup-messed-by-alias is always evil. So the env restoration is > done for _all_ commands whenever aliases are involved. So a side effect of this is whenever an alias is involved, all commands are re-spawned, not just the external ones but builtins as well. > This may be a brilliant fix, or another invitation for regressions. ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
On Fri, Dec 4, 2015 at 9:35 PM, Junio C Hamanowrote: > Nguyễn Thái Ngọc Duy writes: > >> ... Now I conclude >> that setup-messed-by-alias is always evil. So the env restoration is >> done for _all_ commands whenever aliases are involved. > > So a side effect of this is whenever an alias is involved, all > commands are re-spawned, not just the external ones but builtins as > well. I missed that while re-reading c056261, but yes that's true. So Windows folks will be grumpy anyway. Maybe we can avoid that in certain (safe) cases, when we know the second setup_git_... will be executed by the builtin command and won't have any side effects, which is probably the common case. But let's see how it goes. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html