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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 28, 2025, 19:40 UTC
Message-ID: <xmqqa51addc8.fsf@gitster.g>
In-Reply-To: <aQCKD--ZmKnwBWs9@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 21 quoted lines

> On Mon, Oct 27, 2025 at 12:43:54AM +0000, brian m. carlson wrote:
>> diff --git a/src/hash.rs b/src/hash.rs
>> new file mode 100644
>> index 0000000000..0219391820
>> --- /dev/null
>> +++ b/src/hash.rs
>> @@ -0,0 +1,21 @@
>> +// This program is free software; you can redistribute it and/or modify
>> +// it under the terms of the GNU General Public License as published by
>> +// the Free Software Foundation: version 2 of the License, dated June 1991.
>> +//
>> +// This program is distributed in the hope that it will be useful,
>> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +// GNU General Public License for more details.
>> +//
>> +// You should have received a copy of the GNU General Public License along
>> +// with this program; if not, see <https://www.gnu.org/licenses/>.
>
> We typically don't have these headers for our C code, so why have it
> over here?

Yeah, another thing that puzzles me is if src/ is a good name for the directory in the longer run (unless we plan to rewrite everything in Rust, that is) for housing our source code written in Rust (I am assuming that *.c files are unwelcome in that directory). But it may be a separate topic, perhaps?

Show 19 quoted lines

>> +pub const GIT_MAX_RAWSZ: usize = 32;
>> +
>> +/// A binary object ID.
>> +#[repr(C)]
>> +#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
>> +pub struct ObjectID {
>> +    pub hash: [u8; GIT_MAX_RAWSZ],
>> +    pub algo: u32,
>> +}
>
> An alternative to represent this type would be to use an enum:
>
>     pub enum ObjectID {
>         SHA1([u8; GIT_SHA1_RAWSZ]),
>         SHA256([u8; GIT_SHA256_RAWSZ]),
>     }
>
> That would give us some type safety going forward, but it might be
> harder to work with for us?

Can the latter be made interoperate with the C side well, with the same memory layout? Perhaps there may be a way, but the way written in the patch looks more obviously identical to what we have on the C side, so...