git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"

From
Duy Nguyen <pclouds@gmail.com>
Date
Oct 25, 2016, 12:38 UTC
Message-ID
<CACsJy8DZWN0RRaDy9w2BVG5pn4FWCY=1YQDsP0V5obrr1wSzZQ@mail.gmail.com>
In-Reply-To
<20161020062430.rxupwheaeydtcvf3@sigill.intra.peff.net>
On Thu, Oct 20, 2016 at 1:24 PM, Jeff King <peff@peff.net> wrote:
Show 25 quoted lines
> 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@peff.net>
> ---
>  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
Previous: Jeff KingNext: Jeff King
Message 12 of 29 in “stop blind fallback to ".git"”
  1. 0/7 stop blind fallback to ".git"Jeff King, Oct 20, 2016
  2. 1/7 read info/{attributes,exclude} only when in repositoryJeff King, Oct 20, 2016
  3. Duy NguyenOct 25, 2016
  4. Jeff KingOct 25, 2016
  5. 2/7 test-*-cache-tree: setup git dirJeff King, Oct 20, 2016
  6. 3/7 find_unique_abbrev: use 4-buffer ringJeff King, Oct 20, 2016
  7. 4/7 diff_unique_abbrev: rename to diff_aligned_abbrevJeff King, Oct 20, 2016
  8. 5/7 diff_aligned_abbrev: use "struct oid"Jeff King, Oct 20, 2016
  9. 6/7 diff: handle sha1 abbreviations outside of repositoryJeff King, Oct 20, 2016
  10. Jeff KingOct 20, 2016
  11. 7/7 setup_git_env: avoid blind fall-back to ".git"Jeff King, Oct 20, 2016
  12. Duy NguyenOct 25, 2016
  13. Jeff KingOct 25, 2016
  14. Duy NguyenOct 26, 2016
  15. Jeff KingOct 26, 2016
  16. Duy NguyenOct 26, 2016
  17. Jeff KingOct 26, 2016
  18. Jonathan NiederNov 22, 2016
  19. Jeff KingNov 22, 2016
  20. remote helpers: avoid blind fall-back to ".git" when setting GIT_DIRJonathan Nieder, Dec 30, 2016
  21. Stefan BellerDec 30, 2016
  22. Jeff KingDec 30, 2016
  23. Jeff KingDec 30, 2016
  24. Jeff KingFeb 14, 2017
  25. Junio C HamanoFeb 14, 2017
  26. Jeff KingFeb 14, 2017
  27. 1/2 remote: avoid reading $GIT_DIR config in non-repoJeff King, Feb 14, 2017
  28. 2/2 remote helpers: avoid blind fall-back to ".git" when setting GIT_DIRJeff King, Feb 14, 2017
  29. Junio C HamanoNov 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.