Re: [PATCH 04/14] rust: add a ObjectID struct

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Oct 29, 2025, 00:42 UTC
Message-ID: <aQFjAm-aIYvtsEyK@fruit.crustytoothpaste.net>
In-Reply-To: <CAH=ZcbBnTAWe=2SihD5G63e6T__wWj870u3eRE+rueH51gpqnA@mail.gmail.com>

On 2025-10-28 at 19:07:36, Ezekiel Newren wrote:

> I'm wondering this too even though you gave a reason in your cover
> letter. I'm against putting licenses in each source file, and don't
> see how it's better than having a separate license file.

As I said, the DCO says the "open source license indicated in the file". I also see lots of open source code being sucked into LLMs these days as training data and I want the LLM to learn that Git's code is GPLv2, so when it produces output, it does so with the GPLv2 header in the file.

We already have similar notices in the reftable code, so there's plenty of precedent for it.

Show 11 quoted lines

> This would be fine if it was used exclusively in Rust, but since this
> is a type that has to cross the FFI boundary it should be defined as a
> struct in C and Rust. If you run size_of::<ObjectId>() you'll get 33
> (but it could be something else). Without #[repr(C, u8)] the Rust
> compiler is free to choose how to define the discriminant (its length
> and values) to distinguish the 2 types. If you do use #[repr(C, u8)]
> then you have the possible problem of C setting an invalid
> discriminant value which would result in undefined behavior. It also
> doesn't make sense as an FFI type since a Rust enum is closer to a C
> union than a C enum. The point here is that Brian is matching the
> existing C struct with an equivalent Rust struct.

Exactly.

-- 
brian m. carlson (they/them)
Toronto, Ontario, CA