Re: [PATCH v5 1/3] pager: include stdint.h because uintmax_t is used

From: Jeff King <peff@peff.net>
To: Kyle Lippincott <spectral@google.com>
Cc: Sven Strickroth <sven@cs-ware.de>,
	Junio C Hamano <gitster@pobox.com>,
	Calvin Wan <calvinwan@google.com>,
	git@vger.kernel.org, Jonathan Tan <jonathantanmy@google.com>,
	phillip.wood123@gmail.com
Subject: Re: [PATCH v5 1/3] pager: include stdint.h because uintmax_t is used
Date: Tue, 27 Feb 2024 04:05:47 -0500	[thread overview]
Message-ID: <20240227090547.GK3263678@coredump.intra.peff.net> (raw)
In-Reply-To: <20240227084529.GJ3263678@coredump.intra.peff.net>

On Tue, Feb 27, 2024 at 03:45:29AM -0500, Jeff King wrote:

> It is good to clean up old conditionals if they are no longer
> applicable, as they are a burden to reason about later (as this
> discussion shows). But I am not sure about your "just to demonstrate we
> can". It is good to try it out, but it looks like there is a non-zero
> chance that MSVC on Windows might break. It is probably better to try
> building there or looping in folks who can, rather than just making a
> change and seeing if anybody screams.
> 
> I think the "win+VS" test in the GitHub Actions CI job might cover this
> case. It is not run by default (because it was considered be mostly
> redundant with the mingw build), but it shouldn't be too hard to enable
> it for a one-off test.

Here's a successful run with the NO_INTTYPES_H line removed:

  https://github.com/peff/git/actions/runs/8062063219

Blaming the code around the inttypes.h reference in compat/mingw.h
turned up 0ef60afdd4 (MSVC: use shipped headers instead of fallback
definitions, 2016-03-30), which claims that inttypes.h was added to
VS2013.

That sounds old-ish in 2024, but I don't know how old is normal in the
Windows world.

All of which to me says that cleaning this up is something that should
involve Windows folks, who can make the judgement for their platform.

-Peff