Make the CI checks say what they mean, and format the workspace
The Android job's "Verify minimum API level" step has never verified the minimum API level. It took the first `*.so` anywhere under the target directory, which is a host proc-macro from debug/deps — an x86-64 object built by the runner's gcc, whose .comment section cannot mention Android and so can never contradict the expected value. It now reads the artifact under the target triple, compares against MIN_API parsed from the Dockerfile rather than a second copy of the number, and fails on a mismatch. Both sides are checked non-empty first: two failed parses would otherwise compare equal and pass, which is the same silent success in a new costume. The Android image installs one SDK package per layer and keeps the output. sdkmanager is a JVM program that aborts when it cannot get memory, and the single `> /dev/null` step reported that as a bare "exit code 134" while a retry re-downloaded everything that had already succeeded. tools/ci-local.sh runs all four jobs — desktop, android, layering, traceability — against the host toolchain, which is pinned to the same 1.92.0 CI installs. Its matrix check compares regeneration against the working tree rather than against HEAD: CI starts from a clean checkout, so git's answer is the right one there and reports every local run stale here. The rest is rustfmt across the workspace, and the clippy findings that surfaced once it did: manual_contains in dr-thumbs and collections_ui, a map iterated as pairs for its keys, an index loop over a slice, and two runtime assertions on a constant now made at compile time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+101
-22
@@ -156,7 +156,9 @@ impl Cache {
|
||||
let ext = source_ref
|
||||
.rsplit_once('.')
|
||||
.map(|(_, e)| e.to_ascii_lowercase())
|
||||
.filter(|e| !e.is_empty() && e.len() <= 8 && e.chars().all(|c| c.is_ascii_alphanumeric()))
|
||||
.filter(|e| {
|
||||
!e.is_empty() && e.len() <= 8 && e.chars().all(|c| c.is_ascii_alphanumeric())
|
||||
})
|
||||
.unwrap_or_else(|| "bin".to_string());
|
||||
format!("{}.{ext}", image.0)
|
||||
}
|
||||
@@ -257,7 +259,10 @@ impl Cache {
|
||||
// The file is gone but the row says it is here. Believing the
|
||||
// row would report the image as locally available for ever
|
||||
// while every open failed.
|
||||
log::debug!("cached original {} missing, forgetting it: {e}", abs.display());
|
||||
log::debug!(
|
||||
"cached original {} missing, forgetting it: {e}",
|
||||
abs.display()
|
||||
);
|
||||
self.forget(conn, &[image])?;
|
||||
Ok(None)
|
||||
}
|
||||
@@ -357,7 +362,11 @@ impl Cache {
|
||||
)?;
|
||||
let mut usage = Usage::default();
|
||||
let rows = stmt.query_map(rusqlite::params![Tier::Original.stored()], |r| {
|
||||
Ok((r.get::<_, i64>(0)?, r.get::<_, i64>(1)?, r.get::<_, i64>(2)?))
|
||||
Ok((
|
||||
r.get::<_, i64>(0)?,
|
||||
r.get::<_, i64>(1)?,
|
||||
r.get::<_, i64>(2)?,
|
||||
))
|
||||
})?;
|
||||
for row in rows {
|
||||
let (pinned, count, bytes) = row?;
|
||||
@@ -522,9 +531,7 @@ mod tests {
|
||||
rusqlite::params![format!("Photos/img{i:03}.CR2")],
|
||||
)
|
||||
.unwrap();
|
||||
ids.push(ImageId(
|
||||
catalog.connection().last_insert_rowid() as u64
|
||||
));
|
||||
ids.push(ImageId(catalog.connection().last_insert_rowid() as u64));
|
||||
}
|
||||
let dir = tempdir();
|
||||
let cache = Cache::open(&dir, budget).unwrap();
|
||||
@@ -557,9 +564,15 @@ mod tests {
|
||||
let (cat, cache, _dir, ids) = fixture(3);
|
||||
// 400 each against a 1000 budget: storing the third puts it 200 over.
|
||||
let bytes = vec![0u8; 400];
|
||||
cache.store(cat.connection(), ids[0], "a.CR2", &bytes, false, 10).unwrap();
|
||||
cache.store(cat.connection(), ids[1], "b.CR2", &bytes, false, 20).unwrap();
|
||||
cache.store(cat.connection(), ids[2], "c.CR2", &bytes, false, 30).unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", &bytes, false, 10)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "b.CR2", &bytes, false, 20)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[2], "c.CR2", &bytes, false, 30)
|
||||
.unwrap();
|
||||
|
||||
// Touch the oldest so it is no longer the least recently used.
|
||||
cache.load(cat.connection(), ids[0], 40).unwrap();
|
||||
@@ -578,9 +591,15 @@ mod tests {
|
||||
let (cat, cache, _dir, ids) = fixture(3);
|
||||
let bytes = vec![0u8; 800];
|
||||
|
||||
cache.store(cat.connection(), ids[0], "a.CR2", &bytes, true, 10).unwrap();
|
||||
cache.store(cat.connection(), ids[1], "b.CR2", &bytes, false, 20).unwrap();
|
||||
cache.store(cat.connection(), ids[2], "c.CR2", &bytes, false, 30).unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", &bytes, true, 10)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "b.CR2", &bytes, false, 20)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[2], "c.CR2", &bytes, false, 30)
|
||||
.unwrap();
|
||||
|
||||
cache.enforce(cat.connection()).unwrap();
|
||||
|
||||
@@ -597,10 +616,24 @@ mod tests {
|
||||
// pinned.
|
||||
let (cat, cache, _dir, ids) = fixture(2);
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", &vec![0u8; 5000], true, 10)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[0],
|
||||
"a.CR2",
|
||||
&vec![0u8; 5000],
|
||||
true,
|
||||
10,
|
||||
)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "b.CR2", &vec![0u8; 500], false, 20)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[1],
|
||||
"b.CR2",
|
||||
&vec![0u8; 500],
|
||||
false,
|
||||
20,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Pinned use is far past the 1000 budget, but the passive 500 fits.
|
||||
@@ -617,7 +650,14 @@ mod tests {
|
||||
let (cat, cache, _dir, ids) = fixture_with(2, Budget::unlimited());
|
||||
for (i, id) in ids.iter().enumerate() {
|
||||
cache
|
||||
.store(cat.connection(), *id, "a.CR2", &vec![0u8; 100_000], false, i as i64)
|
||||
.store(
|
||||
cat.connection(),
|
||||
*id,
|
||||
"a.CR2",
|
||||
&vec![0u8; 100_000],
|
||||
false,
|
||||
i as i64,
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
assert_eq!(cache.enforce(cat.connection()).unwrap(), 0);
|
||||
@@ -692,7 +732,14 @@ mod tests {
|
||||
|
||||
// But now it is a candidate.
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "b.CR2", &vec![0u8; 800], false, 20)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[1],
|
||||
"b.CR2",
|
||||
&vec![0u8; 800],
|
||||
false,
|
||||
20,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(cache.enforce(cat.connection()).unwrap(), 1);
|
||||
assert!(!cache.holds_original(cat.connection(), ids[0]));
|
||||
@@ -718,8 +765,12 @@ mod tests {
|
||||
#[test]
|
||||
fn storing_the_same_image_twice_is_one_entry() {
|
||||
let (cat, cache, _dir, ids) = fixture(1);
|
||||
cache.store(cat.connection(), ids[0], "a.CR2", b"first", false, 10).unwrap();
|
||||
cache.store(cat.connection(), ids[0], "a.CR2", b"second try", false, 20).unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", b"first", false, 10)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "a.CR2", b"second try", false, 20)
|
||||
.unwrap();
|
||||
|
||||
let usage = cache.usage(cat.connection()).unwrap();
|
||||
assert_eq!(usage.passive_count, 1);
|
||||
@@ -737,10 +788,24 @@ mod tests {
|
||||
// other, which is the worst failure this cache could have.
|
||||
let (cat, cache, _dir, ids) = fixture(2);
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "Photos/IMG_0001.CR2", b"first", false, 10)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[0],
|
||||
"Photos/IMG_0001.CR2",
|
||||
b"first",
|
||||
false,
|
||||
10,
|
||||
)
|
||||
.unwrap();
|
||||
cache
|
||||
.store(cat.connection(), ids[1], "Trips/IMG_0001.CR2", b"second", false, 20)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[1],
|
||||
"Trips/IMG_0001.CR2",
|
||||
b"second",
|
||||
false,
|
||||
20,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
@@ -760,7 +825,14 @@ mod tests {
|
||||
let (cat, cache, _dir, ids) = fixture(3);
|
||||
for (i, id) in ids.iter().enumerate() {
|
||||
cache
|
||||
.store(cat.connection(), *id, "a.CR2", &vec![0u8; 400], false, i as i64)
|
||||
.store(
|
||||
cat.connection(),
|
||||
*id,
|
||||
"a.CR2",
|
||||
&vec![0u8; 400],
|
||||
false,
|
||||
i as i64,
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
// 1200 held against 1000: dropping one 400-byte entry suffices.
|
||||
@@ -772,7 +844,14 @@ mod tests {
|
||||
fn an_extensionless_source_still_gets_a_path() {
|
||||
let (cat, cache, _dir, ids) = fixture(1);
|
||||
cache
|
||||
.store(cat.connection(), ids[0], "Photos/no-extension", b"bytes", false, 10)
|
||||
.store(
|
||||
cat.connection(),
|
||||
ids[0],
|
||||
"Photos/no-extension",
|
||||
b"bytes",
|
||||
false,
|
||||
10,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
cache.load(cat.connection(), ids[0], 20).unwrap().as_deref(),
|
||||
|
||||
@@ -316,9 +316,8 @@ fn remove_within(
|
||||
}
|
||||
let mut removed = 0;
|
||||
{
|
||||
let mut stmt = conn.prepare(
|
||||
"DELETE FROM collection_members WHERE collection_id = ?1 AND image_id = ?2",
|
||||
)?;
|
||||
let mut stmt = conn
|
||||
.prepare("DELETE FROM collection_members WHERE collection_id = ?1 AND image_id = ?2")?;
|
||||
for img in images {
|
||||
removed += stmt.execute(rusqlite::params![id.0 as i64, img.0 as i64])?;
|
||||
}
|
||||
@@ -397,7 +396,8 @@ pub fn save_smart(
|
||||
}
|
||||
}
|
||||
|
||||
let json = serde_json::to_string(selector).map_err(|e| CatalogError::BadSelector(e.to_string()))?;
|
||||
let json =
|
||||
serde_json::to_string(selector).map_err(|e| CatalogError::BadSelector(e.to_string()))?;
|
||||
let n = conn.execute(
|
||||
"UPDATE collections SET selector_json = ?2, kind = 1 WHERE id = ?1 AND deleted = 0",
|
||||
rusqlite::params![id.0 as i64, json],
|
||||
@@ -470,7 +470,9 @@ pub fn collections_for_image(
|
||||
ORDER BY c.name",
|
||||
)?;
|
||||
let rows = stmt
|
||||
.query_map([image.0 as i64], |r| Ok(CollectionId(r.get::<_, i64>(0)? as u64)))?
|
||||
.query_map([image.0 as i64], |r| {
|
||||
Ok(CollectionId(r.get::<_, i64>(0)? as u64))
|
||||
})?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
Ok(rows)
|
||||
}
|
||||
@@ -492,9 +494,8 @@ pub fn descendants(
|
||||
let mut frontier = vec![root];
|
||||
let mut guard = 0usize;
|
||||
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT id FROM collections WHERE parent_id = ?1 AND deleted = 0",
|
||||
)?;
|
||||
let mut stmt =
|
||||
conn.prepare("SELECT id FROM collections WHERE parent_id = ?1 AND deleted = 0")?;
|
||||
|
||||
while let Some(next) = frontier.pop() {
|
||||
guard += 1;
|
||||
@@ -507,7 +508,9 @@ pub fn descendants(
|
||||
}
|
||||
|
||||
let children = stmt
|
||||
.query_map([next.0 as i64], |r| Ok(CollectionId(r.get::<_, i64>(0)? as u64)))?
|
||||
.query_map([next.0 as i64], |r| {
|
||||
Ok(CollectionId(r.get::<_, i64>(0)? as u64))
|
||||
})?
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
for child in children {
|
||||
@@ -668,7 +671,9 @@ fn require_exists(conn: &Connection, id: CollectionId) -> Result<(), CatalogErro
|
||||
|r| r.get(0),
|
||||
)
|
||||
.optional()?;
|
||||
found.map(|_| ()).ok_or(CatalogError::NoSuchCollection(id.0))
|
||||
found
|
||||
.map(|_| ())
|
||||
.ok_or(CatalogError::NoSuchCollection(id.0))
|
||||
}
|
||||
|
||||
/// A random UUID, formatted as the canonical 8-4-4-4-12.
|
||||
@@ -685,8 +690,22 @@ fn new_uuid() -> String {
|
||||
format!(
|
||||
"{:02x}{:02x}{:02x}{:02x}-{:02x}{:02x}-{:02x}{:02x}-{:02x}{:02x}-\
|
||||
{:02x}{:02x}{:02x}{:02x}{:02x}{:02x}",
|
||||
b[0], b[1], b[2], b[3], b[4], b[5], v6, b[7], v8, b[9], b[10], b[11], b[12], b[13],
|
||||
b[14], b[15]
|
||||
b[0],
|
||||
b[1],
|
||||
b[2],
|
||||
b[3],
|
||||
b[4],
|
||||
b[5],
|
||||
v6,
|
||||
b[7],
|
||||
v8,
|
||||
b[9],
|
||||
b[10],
|
||||
b[11],
|
||||
b[12],
|
||||
b[13],
|
||||
b[14],
|
||||
b[15]
|
||||
)
|
||||
}
|
||||
|
||||
@@ -797,7 +816,10 @@ mod tests {
|
||||
let rows = tree(c).unwrap();
|
||||
assert_eq!(rows.len(), 2);
|
||||
assert_eq!(rows[0].collection.name, "Trips");
|
||||
assert!(rows[0].has_children, "a parent must draw a disclosure arrow");
|
||||
assert!(
|
||||
rows[0].has_children,
|
||||
"a parent must draw a disclosure arrow"
|
||||
);
|
||||
assert_eq!(rows[1].depth, 1, "the child is indented one level");
|
||||
}
|
||||
|
||||
|
||||
@@ -472,7 +472,9 @@ mod tests {
|
||||
|
||||
let distinct: i64 = cat
|
||||
.connection()
|
||||
.query_row("SELECT count(DISTINCT uuid) FROM versions", [], |r| r.get(0))
|
||||
.query_row("SELECT count(DISTINCT uuid) FROM versions", [], |r| {
|
||||
r.get(0)
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(distinct, 200);
|
||||
}
|
||||
@@ -566,7 +568,10 @@ mod tests {
|
||||
fn a_bulk_write_over_an_empty_selection_is_a_no_op() {
|
||||
let cat = with_images(3);
|
||||
assert_eq!(set_rating_many(cat.connection(), &[], 5).unwrap(), 0);
|
||||
assert_eq!(set_flag_many(cat.connection(), &[], FlagState::Pick).unwrap(), 0);
|
||||
assert_eq!(
|
||||
set_flag_many(cat.connection(), &[], FlagState::Pick).unwrap(),
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -491,8 +491,6 @@ mod tests {
|
||||
c
|
||||
}
|
||||
|
||||
|
||||
|
||||
/// How many rows a named backfill touched, ignoring the others.
|
||||
///
|
||||
/// Asserting on the whole vector would couple every test to which other
|
||||
@@ -542,9 +540,11 @@ mod tests {
|
||||
|
||||
assert_eq!(backfilled(&c, "shadowed_by"), 1);
|
||||
let got: Option<i64> = c
|
||||
.query_row("SELECT shadowed_by FROM images WHERE id = ?1", [jpeg], |r| {
|
||||
r.get(0)
|
||||
})
|
||||
.query_row(
|
||||
"SELECT shadowed_by FROM images WHERE id = ?1",
|
||||
[jpeg],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(got, Some(raw));
|
||||
}
|
||||
@@ -600,11 +600,7 @@ mod tests {
|
||||
image(&c, Some(1), "a/IMG_1.JPG", "jpg");
|
||||
|
||||
assert_eq!(backfilled(&c, "shadowed_by"), 1);
|
||||
assert_eq!(
|
||||
backfilled(&c, "shadowed_by"),
|
||||
0,
|
||||
"second pass is a no-op"
|
||||
);
|
||||
assert_eq!(backfilled(&c, "shadowed_by"), 0, "second pass is a no-op");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -260,7 +260,9 @@ pub fn list(conn: &Connection, limit: usize) -> Result<Vec<TrashedImage>, Catalo
|
||||
// the current path keeps it listed and deletable rather than
|
||||
// invisible; a restore to the trash folder is a no-op the user
|
||||
// can see, where a hidden row is not.
|
||||
trashed_from: r.get::<_, Option<String>>(2)?.unwrap_or_else(|| source_ref.clone()),
|
||||
trashed_from: r
|
||||
.get::<_, Option<String>>(2)?
|
||||
.unwrap_or_else(|| source_ref.clone()),
|
||||
source_ref,
|
||||
trashed_at: r.get(3)?,
|
||||
file_id: r.get::<_, Option<i64>>(4)?.map(|v| v as u64),
|
||||
@@ -310,9 +312,7 @@ pub fn file_ids_for(conn: &Connection, images: &[ImageId]) -> Result<Vec<u64>, C
|
||||
let placeholders = std::iter::repeat_n("?", images.len())
|
||||
.collect::<Vec<_>>()
|
||||
.join(",");
|
||||
let sql = format!(
|
||||
"SELECT file_id FROM remote WHERE image_id IN ({placeholders})"
|
||||
);
|
||||
let sql = format!("SELECT file_id FROM remote WHERE image_id IN ({placeholders})");
|
||||
let params: Vec<rusqlite::types::Value> = images
|
||||
.iter()
|
||||
.map(|i| rusqlite::types::Value::Integer(i.0 as i64))
|
||||
@@ -364,9 +364,11 @@ mod tests {
|
||||
fn do_trash(cat: &Catalog, i: u64, now: i64) -> String {
|
||||
let c = cat.connection();
|
||||
let original: String = c
|
||||
.query_row("SELECT source_ref FROM images WHERE id = ?1", [i as i64], |r| {
|
||||
r.get(0)
|
||||
})
|
||||
.query_row(
|
||||
"SELECT source_ref FROM images WHERE id = ?1",
|
||||
[i as i64],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
let to = trash_path("PhotosRaw", img(i), &original);
|
||||
record_trashed(c, &[(img(i), to.clone())], now).unwrap();
|
||||
@@ -440,7 +442,9 @@ mod tests {
|
||||
let c = cat.connection();
|
||||
do_trash(&cat, 1, 5_000);
|
||||
|
||||
let back = restore_path(c, img(1)).unwrap().expect("knows where it came from");
|
||||
let back = restore_path(c, img(1))
|
||||
.unwrap()
|
||||
.expect("knows where it came from");
|
||||
assert_eq!(back, "PhotosRaw/2019/IMG_0001.CR2");
|
||||
|
||||
record_restored(c, &[(img(1), back.clone())]).unwrap();
|
||||
@@ -471,7 +475,10 @@ mod tests {
|
||||
|
||||
do_trash(&cat, 1, 2_000);
|
||||
let second = restore_path(c, img(1)).unwrap().unwrap();
|
||||
assert_eq!(first, second, "the origin is the library path, not the trash");
|
||||
assert_eq!(
|
||||
first, second,
|
||||
"the origin is the library path, not the trash"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -506,7 +513,9 @@ mod tests {
|
||||
);
|
||||
// And its path is untouched.
|
||||
let source: String = c
|
||||
.query_row("SELECT source_ref FROM images WHERE id = 2", [], |r| r.get(0))
|
||||
.query_row("SELECT source_ref FROM images WHERE id = 2", [], |r| {
|
||||
r.get(0)
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(source, "PhotosRaw/2019/IMG_0002.CR2");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user