Skip to content

Commit 1a75357

Browse files
committed
commit traversals on shallow clones are non-fatal by default (#364)
But we make it easy to detect if the traversal was shallow or not. That way there will be no 'fail by default' issues in apps using gitoxide when they encountered partial graph (at least as long as they don't try to do anything with their parents).
1 parent f606f88 commit 1a75357

File tree

2 files changed

+55
-26
lines changed

2 files changed

+55
-26
lines changed

git-repository/src/id.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ pub mod ancestors {
141141
.sorting(sorting)
142142
.parents(parents),
143143
),
144+
is_shallow: None,
145+
error_on_missing_commit: false,
144146
}
145147
}
146148
}
@@ -149,13 +151,50 @@ pub mod ancestors {
149151
pub struct Iter<'repo> {
150152
repo: &'repo crate::Repository,
151153
inner: Box<dyn Iterator<Item = Result<git_hash::ObjectId, git_traverse::commit::ancestors::Error>> + 'repo>,
154+
error_on_missing_commit: bool,
155+
// TODO: tests
156+
/// After iteration this flag is true if the iteration was stopped prematurely due to missing parent commits.
157+
/// Note that this flag won't be `Some` if any iteration error occours, which is the case if
158+
/// [`error_on_missing_commit()`][Iter::error_on_missing_commit()] was called.
159+
///
160+
/// This happens if a repository is a shallow clone.
161+
/// Note that this value is `None` as long as the iteration isn't complete.
162+
pub is_shallow: Option<bool>,
163+
}
164+
165+
impl<'repo> Iter<'repo> {
166+
// TODO: tests
167+
/// Once invoked, the iteration will return an error if a commit cannot be found in the object database. This typicall happens
168+
/// when operating on a shallow clone and thus is non-critical by default.
169+
///
170+
/// Check the [`is_shallow`][Iter::is_shallow] field once the iteration ended otherwise to learn if a shallow commit graph
171+
/// was encountered.
172+
pub fn error_on_missing_commit(mut self) -> Self {
173+
self.error_on_missing_commit = true;
174+
self
175+
}
152176
}
153177

154178
impl<'repo> Iterator for Iter<'repo> {
155179
type Item = Result<Id<'repo>, git_traverse::commit::ancestors::Error>;
156180

157181
fn next(&mut self) -> Option<Self::Item> {
158-
self.inner.next().map(|res| res.map(|oid| oid.attach(self.repo)))
182+
match self.inner.next() {
183+
None => {
184+
self.is_shallow = Some(false);
185+
None
186+
}
187+
Some(Ok(oid)) => Some(Ok(oid.attach(self.repo))),
188+
Some(Err(err @ git_traverse::commit::ancestors::Error::FindExisting { .. })) => {
189+
if self.error_on_missing_commit {
190+
Some(Err(err))
191+
} else {
192+
self.is_shallow = Some(true);
193+
None
194+
}
195+
}
196+
Some(Err(err)) => Some(Err(err)),
197+
}
159198
}
160199
}
161200
}

git-repository/src/reference/iter.rs

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,23 @@ pub struct Iter<'r> {
1818
repo: &'r crate::Repository,
1919
}
2020

21+
impl<'r> Iter<'r> {
22+
fn new(repo: &'r crate::Repository, platform: git_ref::file::iter::LooseThenPacked<'r, 'r>) -> Self {
23+
Iter {
24+
inner: platform,
25+
peel: false,
26+
repo,
27+
}
28+
}
29+
}
30+
2131
impl<'r> Platform<'r> {
2232
/// Return an iterator over all references in the repository.
2333
///
2434
/// Even broken or otherwise unparsible or inaccessible references are returned and have to be handled by the caller on a
2535
/// case by case basis.
2636
pub fn all(&self) -> Result<Iter<'_>, init::Error> {
27-
Ok(Iter {
28-
inner: self.platform.all()?,
29-
peel: false,
30-
repo: self.repo,
31-
})
37+
Ok(Iter::new(self.repo, self.platform.all()?))
3238
}
3339

3440
/// Return an iterator over all references that match the given `prefix`.
@@ -37,47 +43,31 @@ impl<'r> Platform<'r> {
3743
// TODO: Create a custom `Path` type that enforces the requirements of git naturally, this type is surprising possibly on windows
3844
// and when not using a trailing '/' to signal directories.
3945
pub fn prefixed(&self, prefix: impl AsRef<Path>) -> Result<Iter<'_>, init::Error> {
40-
Ok(Iter {
41-
inner: self.platform.prefixed(prefix)?,
42-
peel: false,
43-
repo: self.repo,
44-
})
46+
Ok(Iter::new(self.repo, self.platform.prefixed(prefix)?))
4547
}
4648

4749
// TODO: tests
4850
/// Return an iterator over all references that are tags.
4951
///
5052
/// They are all prefixed with `refs/tags`.
5153
pub fn tags(&self) -> Result<Iter<'_>, init::Error> {
52-
Ok(Iter {
53-
inner: self.platform.prefixed("refs/tags/")?,
54-
peel: false,
55-
repo: self.repo,
56-
})
54+
Ok(Iter::new(self.repo, self.platform.prefixed("refs/tags/")?))
5755
}
5856

5957
// TODO: tests
6058
/// Return an iterator over all local branches.
6159
///
6260
/// They are all prefixed with `refs/heads`.
6361
pub fn local_branches(&self) -> Result<Iter<'_>, init::Error> {
64-
Ok(Iter {
65-
inner: self.platform.prefixed("refs/heads/")?,
66-
peel: false,
67-
repo: self.repo,
68-
})
62+
Ok(Iter::new(self.repo, self.platform.prefixed("refs/heads/")?))
6963
}
7064

7165
// TODO: tests
7266
/// Return an iterator over all remote branches.
7367
///
7468
/// They are all prefixed with `refs/remotes`.
7569
pub fn remote_branches(&self) -> Result<Iter<'_>, init::Error> {
76-
Ok(Iter {
77-
inner: self.platform.prefixed("refs/remotes/")?,
78-
peel: false,
79-
repo: self.repo,
80-
})
70+
Ok(Iter::new(self.repo, self.platform.prefixed("refs/remotes/")?))
8171
}
8272
}
8373

0 commit comments

Comments
 (0)