Re: [PATCH v7 06/15] bugreport: add compiler info

[Emily Shaffer <emilyshaffer@google.com>]

On Wed, Feb 19, 2020 at 03:23:34PM +0100, Johannes Schindelin wrote:
> Hi Emily,
> 
> On Thu, 13 Feb 2020, Emily Shaffer wrote:
> 
> > To help pinpoint the source of a regression, it is useful to know some
> > info about the compiler which the user's Git client was built with. By
> > adding a generic get_compiler_info() in 'compat/' we can choose which
> > relevant information to share per compiler; to get started, let's
> > demonstrate the version of glibc if the user built with 'gcc'.
> 
> I agree with the need for the compiler information, but in the patch I
> only see information about glibc being printed out. Shouldn't we use
> `__GNUC__` and `__GNUC_MINOR__` here?
> 
> Don't get me wrong, the glibc version is good and all, but the compiler
> information might be even more crucial. Git for Windows had to hold back
> compiling with GCC v8.x for a while, for example, because the stacksmasher
> was broken. Similar issues are not unheard of, and could help pinpoint
> compiler-related problems a lot quicker.

Hm, sure. Good point - thanks.

This does make me start to wonder, though - does it really make sense to
have ifdef-gated redefinitions of the whole get_compiler_info() method
like I do now? I wonder if it makes more sense to have only one
definition, so we can write down everything we know regardless of which
pieces are put together. My thinking is something like this - what if I
am using glibc, but not a GNU compiler? (The GNU docs on __GCC__
indicate this is a situation that might occur - "a non-GCC compiler that
claims to accept the GNU C dialects") Is there some striking reason not
to implement this compat command thusly instead:

  #ifdef __GLIBC__
  #include <gnu/libc-version.h>
  #endif

  static inline void get_compiler_info(struct strbuf *info)
  {
  	#ifdef __GLIBC__
	strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version());
	#endif

	#ifdef __GNUC__
	strbuf_addf(info, "gnuc: %d.%d\n", __GNUC__, __GNUC_MINOR__);
	#endif

	#ifdef _MSC_VER
	strbuf_addf(info, "msc runtime: %s\n", some_msc_info());
	#endif
  }

The thinking being - this way if I decide to use, say, LLVM + glibc,
then I don't need to reimplement this command with all the glibc
diagnostics again. Or, if someone else already wrote diagnostics for
LLVM with some other libc, then it even Just Works for me and my new
combination.

That said, I'm reasoning about these combinations of compilers and libcs
and whatever else from an inexperienced viewpoint, so maybe this isn't
necessary?

 - Emily