Skip to content

Commit 80cb14d

Browse files
w4w4
committed
Remove panicking code paths of tag indexing
1 parent e4e00b1 commit 80cb14d

File tree

2 files changed

+119
-55
lines changed

2 files changed

+119
-55
lines changed

‎src/database/indexer.rs‎

Lines changed: 113 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ use std::{
66
path::{Path, PathBuf},
77
};
88

9+
use anyhow::Context;
910
use git2::{ErrorCode, Reference, Sort};
1011
use ini::Ini;
1112
use time::OffsetDateTime;
1213
use tracing::{error, info, info_span, instrument, warn};
1314

15+
use super::schema::tag::TagTree;
1416
use crate::database::schema::{
1517
commit::Commit,
1618
repository::{Repository, RepositoryId},
@@ -42,11 +44,21 @@ fn update_repository_metadata(scan_path: &Path, db: &sled::Db) {
4244
discover_repositories(scan_path, &mut discovered);
4345

4446
for repository in discovered {
45-
let relative = get_relative_path(scan_path, &repository);
47+
let Some(relative) = get_relative_path(scan_path, &repository) else {
48+
continue;
49+
};
50+
51+
let id = match Repository::open(db, relative) {
52+
Ok(v) => v.map_or_else(|| RepositoryId::new(db), |v| v.get().id),
53+
Err(error) => {
54+
// maybe we could nuke it ourselves, but we need to instantly trigger
55+
// a reindex and we could enter into an infinite loop if there's a bug
56+
// or something
57+
error!(%error, "Failed to open repository index {}, please consider nuking database", relative.display());
58+
continue;
59+
}
60+
};
4661

47-
let id = Repository::open(db, relative)
48-
.unwrap()
49-
.map_or_else(|| RepositoryId::new(db), |v| v.get().id);
5062
let Some(name) = relative.file_name().map(OsStr::to_string_lossy) else {
5163
continue;
5264
};
@@ -105,15 +117,29 @@ fn find_last_committed_time(repo: &git2::Repository) -> Result<OffsetDateTime, g
105117

106118
#[instrument(skip(db))]
107119
fn update_repository_reflog(scan_path: &Path, db: &sled::Db) {
108-
for (relative_path, db_repository) in Repository::fetch_all(db).unwrap() {
120+
let repos = match Repository::fetch_all(db) {
121+
Ok(v) => v,
122+
Err(error) => {
123+
error!(%error, "Failed to read repository index to update reflog, consider deleting database directory");
124+
return;
125+
}
126+
};
127+
128+
for (relative_path, db_repository) in repos {
109129
let Some(git_repository) = open_repo(scan_path, &relative_path, db_repository.get(), db)
110130
else {
111131
continue;
112132
};
113133

114-
for reference in git_repository.references().unwrap() {
115-
let reference = reference.unwrap();
134+
let references = match git_repository.references() {
135+
Ok(v) => v,
136+
Err(error) => {
137+
error!(%error, "Failed to read references for {relative_path}");
138+
continue;
139+
}
140+
};
116141

142+
for reference in references.filter_map(Result::ok) {
117143
let reference_name = String::from_utf8_lossy(reference.name_bytes());
118144
if !reference_name.starts_with("refs/heads/")
119145
&& !reference_name.starts_with("refs/tags/")
@@ -183,56 +209,86 @@ fn branch_index_update(
183209

184210
#[instrument(skip(db))]
185211
fn update_repository_tags(scan_path: &Path, db: &sled::Db) {
186-
for (relative_path, db_repository) in Repository::fetch_all(db).unwrap() {
212+
let repos = match Repository::fetch_all(db) {
213+
Ok(v) => v,
214+
Err(error) => {
215+
error!(%error, "Failed to read repository index to update tags, consider deleting database directory");
216+
return;
217+
}
218+
};
219+
220+
for (relative_path, db_repository) in repos {
187221
let Some(git_repository) = open_repo(scan_path, &relative_path, db_repository.get(), db)
188222
else {
189223
continue;
190224
};
191225

192-
let tag_tree = db_repository.get().tag_tree(db).unwrap();
226+
if let Err(error) = tag_index_scan(&relative_path, db_repository.get(), db, &git_repository)
227+
{
228+
error!(%error, "Failed to update tags for {relative_path}");
229+
}
230+
}
231+
}
232+
233+
#[instrument(skip(db_repository, db, git_repository))]
234+
fn tag_index_scan(
235+
relative_path: &str,
236+
db_repository: &Repository<'_>,
237+
db: &sled::Db,
238+
git_repository: &git2::Repository,
239+
) -> Result<(), anyhow::Error> {
240+
let tag_tree = db_repository
241+
.tag_tree(db)
242+
.context("Failed to read tag index tree")?;
243+
244+
let git_tags: HashSet<_> = git_repository
245+
.references()
246+
.context("Failed to scan indexes on git repository")?
247+
.filter_map(Result::ok)
248+
.filter(|v| v.name_bytes().starts_with(b"refs/tags/"))
249+
.map(|v| String::from_utf8_lossy(v.name_bytes()).into_owned())
250+
.collect();
251+
let indexed_tags: HashSet<String> = tag_tree.list().into_iter().collect();
252+
253+
// insert any git tags that are missing from the index
254+
for tag_name in git_tags.difference(&indexed_tags) {
255+
tag_index_update(tag_name, git_repository, &tag_tree)?;
256+
}
193257

194-
let git_tags: HashSet<_> = git_repository
195-
.references()
196-
.unwrap()
197-
.filter_map(Result::ok)
198-
.filter(|v| v.name_bytes().starts_with(b"refs/tags/"))
199-
.map(|v| String::from_utf8_lossy(v.name_bytes()).into_owned())
200-
.collect();
201-
let indexed_tags: HashSet<String> = tag_tree.list().into_iter().collect();
258+
// remove any extra tags that the index has
259+
// TODO: this also needs to check peel_to_tag
260+
for tag_name in indexed_tags.difference(&git_tags) {
261+
tag_index_delete(tag_name, &tag_tree)?;
262+
}
202263

203-
// insert any git tags that are missing from the index
204-
for tag_name in git_tags.difference(&indexed_tags) {
205-
let span = info_span!(
206-
"tag_index_update",
207-
reference = tag_name,
208-
repository = relative_path
209-
);
210-
let _entered = span.enter();
264+
Ok(())
265+
}
211266

212-
let reference = git_repository.find_reference(tag_name).unwrap();
267+
#[instrument(skip(git_repository, tag_tree))]
268+
fn tag_index_update(
269+
tag_name: &str,
270+
git_repository: &git2::Repository,
271+
tag_tree: &TagTree,
272+
) -> Result<(), anyhow::Error> {
273+
let reference = git_repository
274+
.find_reference(tag_name)
275+
.context("Failed to read newly discovered tag")?;
213276

214-
if let Ok(tag) = reference.peel_to_tag() {
215-
info!("Inserting newly discovered tag to index");
277+
if let Ok(tag) = reference.peel_to_tag() {
278+
info!("Inserting newly discovered tag to index");
216279

217-
Tag::new(tag.tagger().as_ref()).insert(&tag_tree, tag_name);
218-
}
219-
}
280+
Tag::new(tag.tagger().as_ref()).insert(tag_tree, tag_name)?;
281+
}
220282

221-
// remove any extra tags that the index has
222-
// TODO: this also needs to check peel_to_tag
223-
for tag_name in indexed_tags.difference(&git_tags) {
224-
let span = info_span!(
225-
"tag_index_update",
226-
reference = tag_name,
227-
repository = relative_path
228-
);
229-
let _entered = span.enter();
283+
Ok(())
284+
}
230285

231-
info!("Removing stale tag from index");
286+
#[instrument(skip(tag_tree))]
287+
fn tag_index_delete(tag_name: &str, tag_tree: &TagTree) -> Result<(), anyhow::Error> {
288+
info!("Removing stale tag from index");
289+
tag_tree.remove(tag_name)?;
232290

233-
tag_tree.remove(tag_name);
234-
}
235-
}
291+
Ok(())
236292
}
237293

238294
#[instrument(skip(scan_path, db_repository, db))]
@@ -260,14 +316,22 @@ fn open_repo<P: AsRef<Path> + Debug>(
260316
}
261317
}
262318

263-
fn get_relative_path<'a>(relative_to: &Path, full_path: &'a Path) -> &'a Path {
264-
full_path.strip_prefix(relative_to).unwrap()
319+
fn get_relative_path<'a>(relative_to: &Path, full_path: &'a Path) -> Option<&'a Path> {
320+
full_path.strip_prefix(relative_to).ok()
265321
}
266322

267323
fn discover_repositories(current: &Path, discovered_repos: &mut Vec<PathBuf>) {
268-
let dirs = std::fs::read_dir(current)
269-
.unwrap()
270-
.map(|v| v.unwrap().path())
324+
let current = match std::fs::read_dir(current) {
325+
Ok(v) => v,
326+
Err(error) => {
327+
error!(%error, "Failed to enter repository directory {}", current.display());
328+
return;
329+
}
330+
};
331+
332+
let dirs = current
333+
.filter_map(Result::ok)
334+
.map(|v| v.path())
271335
.filter(|path| path.is_dir());
272336

273337
for dir in dirs {

‎src/database/schema/tag.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ impl<'a> Tag<'a> {
2020
}
2121
}
2222

23-
pub fn insert(&self, batch: &TagTree, name: &str) {
24-
batch
25-
.insert(name.as_bytes(), bincode::serialize(self).unwrap())
26-
.unwrap();
23+
pub fn insert(&self, batch: &TagTree, name: &str) -> Result<(), anyhow::Error> {
24+
batch.insert(name.as_bytes(), bincode::serialize(self)?)?;
25+
26+
Ok(())
2727
}
2828
}
2929

@@ -44,8 +44,8 @@ impl TagTree {
4444
Self(tree)
4545
}
4646

47-
pub fn remove(&self, name: &str) -> bool {
48-
self.0.remove(name).unwrap().is_some()
47+
pub fn remove(&self, name: &str) -> Result<bool, sled::Error> {
48+
self.0.remove(name).map(|v| v.is_some())
4949
}
5050

5151
pub fn list(&self) -> HashSet<String> {

0 commit comments

Comments
 (0)