On Thu, Oct 20, 2016 at 1:24 PM, Jeff King <peff@xxxxxxxx> wrote:
> 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 <peff@xxxxxxxx>
> ---
> 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