Skip to content

Commit 7a5447c

Browse files
committed
feat: Add tree::Editor::get() to get entries directly from the editor.
This is useful if in the middle of an edit you'd like to lookup what's there to choose a non-conflicting name, for example.
1 parent f1591fb commit 7a5447c

File tree

3 files changed

+166
-33
lines changed

3 files changed

+166
-33
lines changed

gix-object/src/tree/editor.rs

+82-26
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,16 @@ impl<'a> Editor<'a> {
4646
find,
4747
object_hash,
4848
trees: HashMap::from_iter(Some((empty_path(), root))),
49-
path_buf: Vec::with_capacity(256).into(),
49+
path_buf: BString::from(Vec::with_capacity(256)).into(),
5050
tree_buf: Vec::with_capacity(512),
5151
}
5252
}
5353
}
5454

5555
/// Operations
5656
impl Editor<'_> {
57-
/// Write the entire in-memory state of all changed trees (and only changed trees) to `out`.
57+
/// Write the entire in-memory state of all changed trees (and only changed trees) to `out`, and remove
58+
/// written portions from our state except for the root tree, which affects [`get()`](Editor::get()).
5859
/// Note that the returned object id *can* be the empty tree if everything was removed or if nothing
5960
/// was added to the tree.
6061
///
@@ -72,7 +73,7 @@ impl Editor<'_> {
7273
/// It is absolutely and intentionally possible to write out invalid trees with this method.
7374
/// Higher layers are expected to perform detailed validation.
7475
pub fn write<E>(&mut self, out: impl FnMut(&Tree) -> Result<ObjectId, E>) -> Result<ObjectId, E> {
75-
self.path_buf.clear();
76+
self.path_buf.borrow_mut().clear();
7677
self.write_at_pathbuf(out, WriteMode::Normal)
7778
}
7879

@@ -85,10 +86,24 @@ impl Editor<'_> {
8586
I: IntoIterator<Item = C>,
8687
C: AsRef<BStr>,
8788
{
88-
self.path_buf.clear();
89+
self.path_buf.borrow_mut().clear();
8990
self.upsert_or_remove_at_pathbuf(rela_path, None)
9091
}
9192

93+
/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
94+
/// to that point.
95+
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
96+
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
97+
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
98+
pub fn get<I, C>(&self, rela_path: I) -> Option<&tree::Entry>
99+
where
100+
I: IntoIterator<Item = C>,
101+
C: AsRef<BStr>,
102+
{
103+
self.path_buf.borrow_mut().clear();
104+
self.get_inner(rela_path)
105+
}
106+
92107
/// Insert a new entry of `kind` with `id` at `rela_path`, an iterator over each path component in the tree,
93108
/// like `a/b/c`. Names are matched case-sensitively.
94109
///
@@ -108,10 +123,41 @@ impl Editor<'_> {
108123
I: IntoIterator<Item = C>,
109124
C: AsRef<BStr>,
110125
{
111-
self.path_buf.clear();
126+
self.path_buf.borrow_mut().clear();
112127
self.upsert_or_remove_at_pathbuf(rela_path, Some((kind, id, UpsertMode::Normal)))
113128
}
114129

130+
fn get_inner<I, C>(&self, rela_path: I) -> Option<&tree::Entry>
131+
where
132+
I: IntoIterator<Item = C>,
133+
C: AsRef<BStr>,
134+
{
135+
let mut path_buf = self.path_buf.borrow_mut();
136+
let mut cursor = self.trees.get(path_buf.as_bstr()).expect("root is always present");
137+
let mut rela_path = rela_path.into_iter().peekable();
138+
while let Some(name) = rela_path.next() {
139+
let name = name.as_ref();
140+
let is_last = rela_path.peek().is_none();
141+
match cursor
142+
.entries
143+
.binary_search_by(|e| cmp_entry_with_name(e, name, true))
144+
.or_else(|_| cursor.entries.binary_search_by(|e| cmp_entry_with_name(e, name, false)))
145+
{
146+
Ok(idx) if is_last => return Some(&cursor.entries[idx]),
147+
Ok(idx) => {
148+
if cursor.entries[idx].mode.is_tree() {
149+
push_path_component(&mut path_buf, name);
150+
cursor = self.trees.get(path_buf.as_bstr())?;
151+
} else {
152+
break;
153+
}
154+
}
155+
Err(_) => break,
156+
};
157+
}
158+
None
159+
}
160+
115161
fn write_at_pathbuf<E>(
116162
&mut self,
117163
mut out: impl FnMut(&Tree) -> Result<ObjectId, E>,
@@ -120,11 +166,12 @@ impl Editor<'_> {
120166
assert_ne!(self.trees.len(), 0, "there is at least the root tree");
121167

122168
// back is for children, front is for parents.
169+
let path_buf = self.path_buf.borrow_mut();
123170
let mut parents = vec![(
124171
None::<usize>,
125-
self.path_buf.clone(),
172+
path_buf.clone(),
126173
self.trees
127-
.remove(&path_hash(&self.path_buf))
174+
.remove(path_buf.as_bstr())
128175
.expect("root tree is always present"),
129176
)];
130177
let mut children = Vec::new();
@@ -133,7 +180,7 @@ impl Editor<'_> {
133180
for entry in &tree.entries {
134181
if entry.mode.is_tree() {
135182
let prev_len = push_path_component(&mut rela_path, &entry.filename);
136-
if let Some(sub_tree) = self.trees.remove(&path_hash(&rela_path)) {
183+
if let Some(sub_tree) = self.trees.remove(&rela_path) {
137184
all_entries_unchanged_or_written = false;
138185
let next_parent_idx = parents.len();
139186
children.push((Some(next_parent_idx), rela_path.clone(), sub_tree));
@@ -167,7 +214,7 @@ impl Editor<'_> {
167214
}
168215
} else if parents.is_empty() {
169216
debug_assert!(children.is_empty(), "we consume children before parents");
170-
debug_assert_eq!(rela_path, self.path_buf, "this should always be the root tree");
217+
debug_assert_eq!(rela_path, **path_buf, "this should always be the root tree");
171218

172219
// There may be left-over trees if they are replaced with blobs for example.
173220
match out(&tree) {
@@ -207,10 +254,8 @@ impl Editor<'_> {
207254
I: IntoIterator<Item = C>,
208255
C: AsRef<BStr>,
209256
{
210-
let mut cursor = self
211-
.trees
212-
.get_mut(&path_hash(&self.path_buf))
213-
.expect("root is always present");
257+
let mut path_buf = self.path_buf.borrow_mut();
258+
let mut cursor = self.trees.get_mut(path_buf.as_bstr()).expect("root is always present");
214259
let mut rela_path = rela_path.into_iter().peekable();
215260
let new_kind_is_tree = kind_and_id.map_or(false, |(kind, _, _)| kind == EntryKind::Tree);
216261
while let Some(name) = rela_path.next() {
@@ -294,9 +339,8 @@ impl Editor<'_> {
294339
if is_last && kind_and_id.map_or(false, |(_, _, mode)| mode == UpsertMode::Normal) {
295340
break;
296341
}
297-
push_path_component(&mut self.path_buf, name);
298-
let path_id = path_hash(&self.path_buf);
299-
cursor = match self.trees.entry(path_id) {
342+
push_path_component(&mut path_buf, name);
343+
cursor = match self.trees.entry(path_buf.clone()) {
300344
hash_map::Entry::Occupied(e) => e.into_mut(),
301345
hash_map::Entry::Vacant(e) => e.insert(
302346
if let Some(tree_id) = tree_to_lookup.filter(|tree_id| !tree_id.is_empty_tree()) {
@@ -307,6 +351,7 @@ impl Editor<'_> {
307351
),
308352
};
309353
}
354+
drop(path_buf);
310355
Ok(self)
311356
}
312357

@@ -325,7 +370,7 @@ impl Editor<'_> {
325370
mod cursor {
326371
use crate::tree::editor::{Cursor, UpsertMode, WriteMode};
327372
use crate::tree::{Editor, EntryKind};
328-
use crate::Tree;
373+
use crate::{tree, Tree};
329374
use bstr::{BStr, BString};
330375
use gix_hash::ObjectId;
331376

@@ -350,26 +395,41 @@ mod cursor {
350395
I: IntoIterator<Item = C>,
351396
C: AsRef<BStr>,
352397
{
353-
self.path_buf.clear();
398+
self.path_buf.borrow_mut().clear();
354399
self.upsert_or_remove_at_pathbuf(
355400
rela_path,
356401
Some((EntryKind::Tree, self.object_hash.null(), UpsertMode::AssureTreeOnly)),
357402
)?;
403+
let prefix = self.path_buf.borrow_mut().clone();
358404
Ok(Cursor {
359-
prefix: self.path_buf.clone(), /* set during the upsert call */
405+
prefix, /* set during the upsert call */
360406
parent: self,
361407
})
362408
}
363409
}
364410

365411
impl Cursor<'_, '_> {
412+
/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
413+
/// to that point.
414+
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
415+
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
416+
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
417+
pub fn get<I, C>(&self, rela_path: I) -> Option<&tree::Entry>
418+
where
419+
I: IntoIterator<Item = C>,
420+
C: AsRef<BStr>,
421+
{
422+
self.parent.path_buf.borrow_mut().clone_from(&self.prefix);
423+
self.parent.get_inner(rela_path)
424+
}
425+
366426
/// Like [`Editor::upsert()`], but with the constraint of only editing in this cursor's tree.
367427
pub fn upsert<I, C>(&mut self, rela_path: I, kind: EntryKind, id: ObjectId) -> Result<&mut Self, super::Error>
368428
where
369429
I: IntoIterator<Item = C>,
370430
C: AsRef<BStr>,
371431
{
372-
self.parent.path_buf.clone_from(&self.prefix);
432+
self.parent.path_buf.borrow_mut().clone_from(&self.prefix);
373433
self.parent
374434
.upsert_or_remove_at_pathbuf(rela_path, Some((kind, id, UpsertMode::Normal)))?;
375435
Ok(self)
@@ -381,14 +441,14 @@ mod cursor {
381441
I: IntoIterator<Item = C>,
382442
C: AsRef<BStr>,
383443
{
384-
self.parent.path_buf.clone_from(&self.prefix);
444+
self.parent.path_buf.borrow_mut().clone_from(&self.prefix);
385445
self.parent.upsert_or_remove_at_pathbuf(rela_path, None)?;
386446
Ok(self)
387447
}
388448

389449
/// Like [`Editor::write()`], but will write only the subtree of the cursor.
390450
pub fn write<E>(&mut self, out: impl FnMut(&Tree) -> Result<ObjectId, E>) -> Result<ObjectId, E> {
391-
self.parent.path_buf.clone_from(&self.prefix);
451+
self.parent.path_buf.borrow_mut().clone_from(&self.prefix);
392452
self.parent.write_at_pathbuf(out, WriteMode::FromCursor)
393453
}
394454
}
@@ -424,10 +484,6 @@ fn empty_path() -> BString {
424484
BString::default()
425485
}
426486

427-
fn path_hash(path: &[u8]) -> BString {
428-
path.to_vec().into()
429-
}
430-
431487
fn push_path_component(base: &mut BString, component: &[u8]) -> usize {
432488
let prev_len = base.len();
433489
debug_assert!(base.last() != Some(&b'/'));

gix-object/src/tree/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::{
22
bstr::{BStr, BString},
33
tree, Tree, TreeRef,
44
};
5+
use std::cell::RefCell;
56
use std::cmp::Ordering;
67

78
///
@@ -30,7 +31,7 @@ pub struct Editor<'a> {
3031
/// dropped when writing at the latest.
3132
trees: std::collections::HashMap<BString, Tree>,
3233
/// A buffer to build up paths when finding the tree to edit.
33-
path_buf: BString,
34+
path_buf: RefCell<BString>,
3435
/// Our buffer for storing tree-data in, right before decoding it.
3536
tree_buf: Vec<u8>,
3637
}

gix-object/tests/object/tree/editor.rs

+82-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use gix_object::tree::EntryKind;
1+
use gix_object::tree::{Entry, EntryKind};
22
use gix_object::Tree;
33

44
#[test]
@@ -32,6 +32,19 @@ fn from_empty_cursor() -> crate::Result {
3232
"only one item is left in the tree, which also keeps it alive"
3333
);
3434
assert_eq!(num_writes_and_clear(), 1, "root tree");
35+
assert_eq!(
36+
cursor.get(None::<&str>),
37+
None,
38+
"the 'root' can't be obtained, no entry exists for it, ever"
39+
);
40+
assert_eq!(
41+
cursor.get(Some("empty-dir-via-cursor")),
42+
Some(&Entry {
43+
mode: EntryKind::Tree.into(),
44+
filename: "empty-dir-via-cursor".into(),
45+
oid: gix_hash::ObjectId::empty_tree(gix_hash::Kind::Sha1),
46+
}),
47+
);
3548

3649
let actual = edit.write(&mut write)?;
3750
assert_eq!(
@@ -52,6 +65,20 @@ fn from_empty_cursor() -> crate::Result {
5265
let mut cursor = edit.cursor_at(cursor_path)?;
5366
let actual = cursor.remove(Some("empty-dir-via-cursor"))?.write(&mut write)?;
5467
assert_eq!(actual, empty_tree(), "it keeps the empty tree like the editor would");
68+
assert_eq!(
69+
edit.get(["some", "deeply", "nested", "path"]),
70+
Some(&Entry {
71+
mode: EntryKind::Tree.into(),
72+
filename: "path".into(),
73+
oid: gix_hash::Kind::Sha1.null(),
74+
}),
75+
"the directory leading to the removed one is still present"
76+
);
77+
assert_eq!(
78+
edit.get(["some", "deeply", "nested", "path", "empty-dir-via-cursor"]),
79+
None,
80+
"but the removed entry is indee removed"
81+
);
5582

5683
let actual = edit.write(&mut write)?;
5784
assert_eq!(
@@ -400,6 +427,11 @@ fn from_empty_add() -> crate::Result {
400427
"4b825dc642cb6eb9a060e54bf8d69288fbee4904\n"
401428
);
402429
assert_eq!(odb.access_count_and_clear(), 0);
430+
assert_eq!(
431+
edit.get(None::<&str>),
432+
None,
433+
"the 'root' can't be obtained, no entry exists for it, ever"
434+
);
403435

404436
let actual = edit
405437
.upsert(Some("hi"), EntryKind::Blob, gix_hash::Kind::Sha1.null())?
@@ -431,12 +463,28 @@ fn from_empty_add() -> crate::Result {
431463
assert_eq!(storage.borrow().len(), 1, "still nothing but empty trees");
432464
assert_eq!(odb.access_count_and_clear(), 0);
433465

434-
let actual = edit
435-
.upsert(["a", "b"], EntryKind::Tree, empty_tree())?
466+
edit.upsert(["a", "b"], EntryKind::Tree, empty_tree())?
436467
.upsert(["a", "b", "c"], EntryKind::Tree, empty_tree())?
437-
.upsert(["a", "b", "d", "e"], EntryKind::Tree, empty_tree())?
438-
.write(&mut write)
439-
.expect("it's OK to write empty trees");
468+
.upsert(["a", "b", "d", "e"], EntryKind::Tree, empty_tree())?;
469+
assert_eq!(
470+
edit.get(["a", "b"]),
471+
Some(&Entry {
472+
mode: EntryKind::Tree.into(),
473+
filename: "b".into(),
474+
oid: hex_to_id("4b825dc642cb6eb9a060e54bf8d69288fbee4904"),
475+
}),
476+
"before writing, entries are still present, just like they were written"
477+
);
478+
assert_eq!(
479+
edit.get(["a", "b", "c"]),
480+
Some(&Entry {
481+
mode: EntryKind::Tree.into(),
482+
filename: "c".into(),
483+
oid: gix_hash::ObjectId::empty_tree(gix_hash::Kind::Sha1),
484+
}),
485+
);
486+
487+
let actual = edit.write(&mut write).expect("it's OK to write empty trees");
440488
assert_eq!(
441489
display_tree(actual, &storage),
442490
"bf91a94ae659ac8a9da70d26acf42df1a36adb6e
@@ -450,6 +498,16 @@ fn from_empty_add() -> crate::Result {
450498
);
451499
assert_eq!(num_writes_and_clear(), 4, "it wrote the trees it needed to write");
452500
assert_eq!(odb.access_count_and_clear(), 0);
501+
assert_eq!(edit.get(["a", "b"]), None, "nothing exists here");
502+
assert_eq!(
503+
edit.get(Some("a")),
504+
Some(&Entry {
505+
mode: EntryKind::Tree.into(),
506+
filename: "a".into(),
507+
oid: hex_to_id("850bf83c26003cb0541318718bc9217c4a5bde6d"),
508+
}),
509+
"but the top-level tree is still available and can yield its entries, as written with proper ids"
510+
);
453511

454512
let actual = edit
455513
.upsert(["a"], EntryKind::Blob, any_blob())?
@@ -532,6 +590,24 @@ fn from_empty_add() -> crate::Result {
532590
"still, only the root-tree changes effectively"
533591
);
534592
assert_eq!(odb.access_count_and_clear(), 0);
593+
594+
let actual = edit
595+
.upsert(["a", "b"], EntryKind::Tree, empty_tree())?
596+
.upsert(["a", "b", "c"], EntryKind::BlobExecutable, any_blob())?
597+
// .upsert(["a", "b", "d"], EntryKind::Blob, any_blob())?
598+
.write(&mut write)?;
599+
assert_eq!(
600+
display_tree(actual, &storage),
601+
"d8d3f558776965f70452625b72363234f517b290
602+
└── a
603+
└── b
604+
└── c bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.100755
605+
",
606+
"the intermediate tree is rewritten to be suitable to hold the blob"
607+
);
608+
assert_eq!(num_writes_and_clear(), 3, "root, and two child-trees");
609+
assert_eq!(odb.access_count_and_clear(), 0);
610+
535611
Ok(())
536612
}
537613

0 commit comments

Comments
 (0)