Re: [PATCH v5 15/18] compat/mingw: split out POSIX-related bits

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 17, 2025, 12:47 UTC
Message-ID: <Z7MvzH3AVe4_waUH@pks.im>
In-Reply-To: <e202a870-921b-8f38-58af-3d44e94c447d@gmx.de>

On Thu, Feb 13, 2025 at 07:22:57PM +0100, Johannes Schindelin wrote:

Show 44 quoted lines

> Hi Hannes,
> 
> thank you for looping me in.
> 
> On Sun, 9 Feb 2025, Johannes Sixt wrote:
> 
> > Am 07.02.25 um 12:52 schrieb Patrick Steinhardt:
> > > Split out the POSIX-related bits from "compat/mingw.h". This is in
> > > preparation for splitting up "git-compat-utils.h" into a header that
> > > provides POSIX-compatibility and a header that provides common wrappers
> > > used by the Git project.
> > >
> > > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > > ---
> > >  compat/{mingw.c => mingw/compat-util.c} |  28 ++--
> > >  compat/mingw/compat-util.h              | 220 ++++++++++++++++++++++++++++++++
> > >  compat/{mingw.h => mingw/posix.h}       | 216 +------------------------------
> > >  compat/msvc.c                           |   6 -
> > >  compat/msvc/compat-util.c               |   6 +
> > >  compat/msvc/compat-util.h               |   7 +
> > >  compat/{msvc.h => msvc/posix.h}         |   8 +-
> > >  config.mak.uname                        |   6 +-
> > >  contrib/buildsystems/CMakeLists.txt     |   2 +-
> > >  git-compat-util.h                       |   4 +-
> > >  meson.build                             |   8 +-
> > >  11 files changed, 266 insertions(+), 245 deletions(-)
> >
> > I can agree with this rearrangement of compat/mingw.c. But the
> > Git-for-Windows repository has much higher stakes with all the
> > additional changes. I put Dscho on Cc.
> 
> This indeed causes massive friction. See for example
> https://github.com/git-for-windows/git/actions/workflows/main.yml?query=branch%3Ashears%2Fseen,
> which should ideally be updated with green builds as frequently as `seen`
> is updated. It's dark red, and there are tons of gaps (which means that I
> was not able to finish adapting to these changes).
> 
> Another problem introduced by this series is that now the same filename is
> used in multiple directories, which makes it unnecessarily hard e.g. when
> setting breakpoints in `gdb`, or when trying to follow `#include`s.
> 
> Granted, the problem already exists, e.g. with range-diff.c vs
> builtin/range-diff.c, but I would hope that Git's source code would be
> changed away from duplicate filenames instead of adding more.

The question is what ways we have to improve. Filenames being the same can be avoided, e.g. by naming the split-out files e.g. "mingw-posix.h" instead of "mingw/posix.h". But what alternatives do we have when it comes to splitting out the logic in the first place to reduce the conflicts on your side?

A couple of alternatives I see:

  - Use `ifdef`s to conditionally only make POSIX bits available. I
    think this would be quite awkward.

  - Just ignore the issue on Windows altogether and expose non-POSIX
    bits there. I don't think this is great either, as it means that
    Windows becomes even more of an outlier than it already is.

Do you have any other ideas?

I can send a version that avoids moving stuff around and really only splits out relevant functions into "compat/{mingw,msvc}-posix.h". But I'm unsure whether that's enough of a help to you.

Patrick