Discretisation of EntryMode to EntryKind is almost certainly incorrect

[xmo-odoo]

Looked at 3ac5d0b after hitting the privatisation of EntryMode's state¹ and I might be misreading the code but I think the normalisation behaviour is not correct: https://github.com/Byron/gitoxide/blob/main/gix-object/src/tree/mod.rs#L77-L90

This converts exact tree, link, and commit (submodules) modes to the corresponding kind, then everything else is converted to an executable blob if 0o100 is set, and to a non-executable blob otherwise.

Git's own mode canonicalisation works very differently though: it uses the S_IS* macros from stat.h to parse the various modes. To my understanding this means it masks in the upper nibble, then checks that exactly against one of the type constants. So e.g.

0o40534

is considered a directory, because the permission bits are discarded before dispatch. In gix-object, I think this currently would parse as an executable blob.

Git checks for BLOB, LINK, then TREE but that should not matter overly much as all of those are equality checks, however it then falls back to GITLINK (Commit) so e.g. 0o10000 or 0o140000 should be interpreted as Commit, not Blob.

That would also make the first four is_ functions incorrect as they check for exact values rather than follow normalisation.

Footnotes

1. incidentally the ability to create an arbitrary EntryMode might be useful to test git implementations themselves, but the payload is not pub and there seems to be no way to convert a u16 to an EntryMode. ↩

[Byron]

Thanks so much for pointing this out! I agree that the is_* and kind() methods wouldn't yield the correct results for some older trees which didn't store their mode in a canonicalized form.

I will see to fixing it.

Also, for now it's only supported to turn EntryKind into EntryMode by means of let mode: EntryMode = kind.into(), which assures only canonicalized values are used which should be enough and an encouragement to only write trees with canonicalized values.

[xmo-odoo]

Also, for now it's only supported to turn EntryKind into EntryMode by means of let mode: EntryMode = kind.into(), which assures only canonicalized values are used which should be enough and an encouragement to only write trees with canonicalized values.

Oh yeah I agree with that, the footnote was mostly because it can be useful to output non-canonical values to test other implementations, but that may not be relevant to your goals obviously.

[Byron]

It's implemented now, as it did help to validate the new implementation more easily to assure the classification works as it should.

[Byron]

Reopened until @xmo-odoo gave their OK that's truly correct and fixed.

[xmo-odoo]

I checked my notes and didn't see anything else pertaining to the specific values, the only bit was indeed

100664 is assimilated to 100644

so g+w being set is just ignored, and since the code checked for 0o100 all along that's never been a concern. I'll close this as I think it can be considered fixed by #1280.