Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Duy Nguyen <hidden>
Date: 2016-10-25 12:39:06
On Thu, Oct 20, 2016 at 1:24 PM, Jeff King [off-list ref] wrote:
quoted hunk ↗ jump to hunk
This passes the test suite (after the adjustments in the previous patches), but there's a risk of regression for any cases where the fallback usually works fine but the code isn't exercised by the test suite. So by itself, this commit is a potential step backward, but lets us take two steps forward once we've identified and fixed any such instances. Signed-off-by: Jeff King <redacted> --- environment.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/environment.c b/environment.c index cd5aa57..b1743e6 100644 --- a/environment.c +++ b/environment.c@@ -164,8 +164,11 @@ static void setup_git_env(void) const char *replace_ref_base; git_dir = getenv(GIT_DIR_ENVIRONMENT); - if (!git_dir) + if (!git_dir) { + if (!startup_info->have_repository) + die("BUG: setup_git_env called without repository");
YES!!! Thank you for finally fixing this. The "once we've identified" part could be tricky though. This message alone will not give us any clue where it's called since it's buried deep in git_path() usually, which is buried deep elsewhere. Without falling back to core dumps (with debug info), glibc's backtrace (platform specifc), the best we could do is turn git_path() into a macro that takes __FILE__ and __LINE__ and somehow pass the info down here, but "..." in macros is C99 specific, sigh.. Is it too bad to turn git_path() into a macro when we know the compiler is C99 ? Older compilers will have no source location info in git_path(), Hopefully they are rare, which means chances of this fault popping up are also reduced. -- Duy