On Mon, Apr 17, 2023 at 9:29 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Elijah Newren <newren@xxxxxxxxx> writes:
>
> >> 5579f44d2f ("treewide: remove unnecessary cache.h inclusion", 2023-04-11)
> >> broke connections via git protocol because it removed the inclusion of
> >> the default port macro. While some may consider this transport to be
> >> deprecated, it still serves some purpose.
> >
> > In particular the problem is that
> >
> > const char *port = STR(DEFAULT_GIT_PORT);
> >
> > translates now to
> >
> > const char *port = "DEFAULT_GIT_PORT";
> >
> > instead of
> >
> > const char *port = "9418";
>
> Wow, that is a bad one. If people do want "DEFAULT_GIT_PORT", they
> would never write STR(DEFAULT_GIT_PORT). I wonder if we can have a
> bit more clever STR() macro that catches this kind of mistake at the
> compile time.
>
> If this is worth fixing, the fix would probably be worth protecting
> with a test or two, but the networking test with fixed port is not
> something we can easily do without a sealed environment, so...
>
> Thanks Michael for catching this.
Yup.
> > I've got a patch that does precisely this that I just submitted as
> > part of my follow-on to the en/header-split-cache-h series. I've included
> > that patch below in case Junio wants to advance it faster than the rest of
> > that series.
>
> Yeah, burying it in a 24-patch series is a bit unfortunate.
I didn't know it was a fix for anything when I wrote it; it was in the
24-patch series just as a further refactoring. Then I found out after
this report and doing a little digging I found it might be considered
a good fix for the issue so I included it here too.
If you apply this patch directly on this series, I'm happy to drop it
from the other series or do whatever makes things easiest.
