Skip to content

Commit ea655f3

Browse files
fix!(gix-object): Allow round-tripping for Trees
Change the internal representation of `EntryMode` to fix a round-trip issue. Prior to this change, both `b"040000"` and `b"40000"` mapped to `0o40000u16`. After this change, * `b"040000"` is represented by `0o140000` * `b"40000"` is represented by `0o40000` *Tests*: We can see in the `as_bytes` test that the behaviour is fixed now. We also add a test to show we now can round-trip on the existing test fixtures which contain examples of this situation. *Performance*: We pay a cost for this compared to the parent commit: ``` TreeRefIter() time: [50.403 ns 50.611 ns 50.830 ns] change: [+8.6240% +9.0871% +9.5776%] (p = 0.00 < 0.05) Performance has regressed. ``` but we already did enough optimizations to pay for this earlier in this PR: * `main`: `~55 ns` * `parent`: `~46 ns` * `this commit`: `~50 ns` Fixes 1887
1 parent 1a86629 commit ea655f3

File tree

3 files changed

+23
-4
lines changed

3 files changed

+23
-4
lines changed

Diff for: gix-object/src/tree/mod.rs

+18-2
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,13 @@ impl TryFrom<u32> for tree::EntryMode {
6464
impl EntryMode {
6565
/// Expose the value as a u16 (lossy, unlike the internal representation that is hidden)
6666
pub const fn value(self) -> u16 {
67-
self.internal
67+
// Demangle the hack: In the case where the second leftmost octet is 4 (Tree), the leftmost bit is
68+
// there to represent whether the bytes representation should have 5 or 6 octets
69+
if self.internal & IFMT == 0o140000 {
70+
0o040000
71+
} else {
72+
self.internal
73+
}
6874
}
6975

7076
/// Return the representation as used in the git internal format, which is octal and written
@@ -79,8 +85,14 @@ impl EntryMode {
7985
let digit = (self.internal & oct_mask) >> bit_pos;
8086
*backing_octet = b'0' + digit as u8;
8187
}
88+
// Hack: `0o140000` represents `"040000"`, `0o40000` represents `"40000"`
8289
if backing[1] == b'4' {
83-
&backing[1..6]
90+
if backing[0] == b'1' {
91+
backing[0] = b'0';
92+
&backing[0..6]
93+
} else {
94+
&backing[1..6]
95+
}
8496
} else {
8597
&backing[0..6]
8698
}
@@ -118,6 +130,10 @@ impl EntryMode {
118130
mode = (mode << 3) + (b - b'0') as u16;
119131
idx += 1;
120132
}
133+
// Hack: `0o140000` represents `"040000"`, `0o40000` represents `"40000"`
134+
if mode == 0o040000 && i[0] == b'0' {
135+
mode += 0o100000;
136+
}
121137
Some((Self { internal: mode }, &i[(space_pos + 1)..]))
122138
}
123139

Diff for: gix-object/tests/object/tree/entry_mode.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,9 @@ fn as_bytes() {
7070
EntryMode::from_bytes(b"100644 ".as_ref()).expect("valid"),
7171
"100644".into(),
7272
),
73-
// Show incorrect behaviour: b"040000" doesn't round-trip
7473
(
7574
EntryMode::from_bytes(b"040000".as_ref()).expect("valid"),
76-
"40000".into(),
75+
"040000".into(),
7776
),
7877
(EntryMode::from_bytes(b"40000".as_ref()).expect("valid"), "40000".into()),
7978
] {

Diff for: gix-object/tests/object/tree/from_bytes.rs

+4
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ fn special_trees() -> crate::Result {
101101
expected_entry_count,
102102
"{name}"
103103
);
104+
// Show we can roundtrip
105+
let mut buf: Vec<u8> = Default::default();
106+
actual.write_to(&mut buf).expect("Failed to write bytes to buffer");
107+
assert_eq!(buf, fixture);
104108
}
105109
Ok(())
106110
}

0 commit comments

Comments
 (0)