Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust

From: Elijah Newren <newren@gmail.com>
Date: Aug 25, 2025, 19:16 UTC
Message-ID: <CABPp-BF44xgh5uJhCKXE8aSN5otyHOAJYNqB_bfLj1Z7_FANCw@mail.gmail.com>
In-Reply-To: <xmqq8qj9vrpf.fsf@gitster.g>

On Sat, Aug 23, 2025 at 11:05 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> > diff --git a/interop/ivec.c b/interop/ivec.c
> > new file mode 100644
> > index 000000000000..9bc2258c04ad
> > --- /dev/null
> > +++ b/interop/ivec.c
>
> Even though this is a shim to somebody else's code, it still is a
> part of our codebase, so our CodingGuidelines for C programs should
> apply.

Sorry, I should have caught these in my preliminary review before he sent this off to the list. One question, though...

Show 19 quoted lines

> > diff --git a/interop/ivec.h b/interop/ivec.h
> > new file mode 100644
> > index 000000000000..98be4bbeb54a
> > --- /dev/null
> > +++ b/interop/ivec.h
> > @@ -0,0 +1,52 @@
> > +#ifndef IVEC_H
> > +#define IVEC_H
> > +
> > +#include "../git-compat-util.h"
>
> As we use -I. on the command line, there is no need to add "../"
> here; just writing
>
>         #include <git-compat-util.h>
>
> should be enough.  Also, if this file does not depend on the
> services compat-util header provides (and I do not think it does
> from a brief look at its contents), it is better not to include it.

Should this rather be

   #include "git-compat-util.h"

with quotes rather than angle brackets?  In particular:

$ git grep include.*git-compat-util -- '*.[ch]' | wc -l 362 $ git grep include.*git-compat-util -- '*/*.[ch]' | wc -l 125

So, we have 362 includes of git-compat-util.h in our codebase, 125 from subdirectories. Of those:

$ git grep include.*git-compat-util -- '*.[ch]' | grep '"' | wc -l 361 $ git grep include.*git-compat-util -- '*.[ch]' | grep '<' | wc -l 1

Only one of these include statements uses angle brackets -- the compiler-tricks/not-constant.c file (which appears to be a temporary hack that we'll eventually delete). I had always assumed <> were for system includes and "" for project includes, but a quick Google search shows the actual situation is quite a bit murkier than I'd realized. Still, our current project practice appears to be double quotes; is that fine here or are you suggesting you'd like the current project practice to be changed?