Skip to content

Commit 834ca6c

Browse files
committed
fix: use semver-aware version comparison for GHCR tags
Lexicographic sorting was picking wrong tags (e.g. 2026.2.9 over 2026.2.23, or v-prefixed date tags over clean versions). Parse versions with the semver crate, handling calver, v-prefix stripping, two-component versions, and -rN release revisions.
1 parent 4fd7fa6 commit 834ca6c

7 files changed

Lines changed: 205 additions & 24 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ rayon = "1.11"
3939
regex = "1"
4040
reqwest = { version = "0.13", default-features = false }
4141
rusqlite = { version = "0.38", features = ["bundled"] }
42+
semver = "1"
4243
serde = { version = "1", features = ["derive"] }
4344
serde_json = "1"
4445
saphyr = "0.0.6"

sbuild-cache/src/mongo.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,9 @@ impl MongoDatabase {
256256
) -> Result<i32> {
257257
let filter = doc! { "pkg_id": pkg_id, "host_triplet": host_triplet };
258258
let options = FindOneOptions::builder()
259-
.projection(doc! { "base_version": 1, "remote_version": 1, "revision": 1, "recipe_hash": 1 })
259+
.projection(
260+
doc! { "base_version": 1, "remote_version": 1, "revision": 1, "recipe_hash": 1 },
261+
)
260262
.build();
261263

262264
let result = self

sbuild-cache/src/sqlite.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use std::path::Path;
66

77
use crate::error::{Error, Result};
88
use crate::models::*;
9-
use crate::schema::{CREATE_SCHEMA, CREATE_VIEWS, MIGRATE_V1_TO_V2, MIGRATE_V2_TO_V3, SCHEMA_VERSION};
9+
use crate::schema::{
10+
CREATE_SCHEMA, CREATE_VIEWS, MIGRATE_V1_TO_V2, MIGRATE_V2_TO_V3, SCHEMA_VERSION,
11+
};
1012

1113
/// SQLite cache database
1214
pub struct CacheDatabase {

sbuild-meta/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ log.workspace = true
1414
regex.workspace = true
1515
reqwest = { workspace = true, features = ["json", "rustls"] }
1616
rusqlite.workspace = true
17+
semver.workspace = true
1718
serde.workspace = true
1819
serde_json.workspace = true
1920
saphyr.workspace = true

sbuild-meta/src/registry.rs

Lines changed: 170 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
//! Provides functionality to interact with GitHub Container Registry
44
//! for fetching manifests, tags, and package metadata.
55
6+
use std::cmp::Ordering;
7+
68
use reqwest::header::{HeaderMap, HeaderValue, ACCEPT, AUTHORIZATION};
79
use serde::Deserialize;
810

@@ -86,12 +88,12 @@ impl RegistryClient {
8688
.collect()
8789
}
8890

89-
/// Get the latest tag for an architecture
91+
/// Get the latest tag for an architecture using version-aware comparison
9092
pub fn get_latest_arch_tag<'a>(tags: &'a [String], arch: &str) -> Option<&'a String> {
9193
Self::filter_tags_by_arch(tags, arch)
9294
.into_iter()
9395
.filter(|t| !t.starts_with("latest"))
94-
.max_by(|a, b| a.cmp(b)) // Version sort
96+
.max_by(|a, b| version_compare_tags(a, b, arch))
9597
}
9698

9799
/// Fetch manifest for a specific tag
@@ -163,6 +165,110 @@ impl Default for RegistryClient {
163165
}
164166
}
165167

168+
/// Extract the version portion from a tag by stripping the `-{arch}` suffix.
169+
/// e.g. "v1.2.3-x86_64-linux" -> "v1.2.3", "2026.2.23-aarch64-linux" -> "2026.2.23"
170+
fn extract_version_from_tag<'a>(tag: &'a str, arch: &str) -> &'a str {
171+
let arch_suffix = format!("-{}", arch);
172+
tag.strip_suffix(&arch_suffix)
173+
.or_else(|| {
174+
// case-insensitive fallback
175+
let lower = tag.to_lowercase();
176+
let suffix_lower = arch_suffix.to_lowercase();
177+
if lower.ends_with(&suffix_lower) {
178+
Some(&tag[..tag.len() - arch_suffix.len()])
179+
} else {
180+
None
181+
}
182+
})
183+
.unwrap_or(tag)
184+
}
185+
186+
/// Parse a version string into a semver::Version, handling common non-semver formats.
187+
/// Strips leading 'v'/'V', handles calver (2026.2.23), release suffixes (-r1), etc.
188+
fn parse_version_lenient(version: &str) -> Option<semver::Version> {
189+
let v = version
190+
.strip_prefix('v')
191+
.or(version.strip_prefix('V'))
192+
.unwrap_or(version);
193+
194+
// Check for -rN release revision suffix first (e.g. "0.16.1-r2")
195+
// These should sort HIGHER than the base version, but semver treats
196+
// pre-release as LOWER, so we handle them specially.
197+
let (base_str, revision) = extract_release_revision(v);
198+
199+
// Split on first '-' to separate version from extra info (dates, hashes, etc.)
200+
let (numeric_base, extra) = match base_str.find('-') {
201+
Some(idx) => (&base_str[..idx], Some(&base_str[idx + 1..])),
202+
None => (base_str, None),
203+
};
204+
205+
// Split base into numeric parts
206+
let parts: Vec<u64> = numeric_base
207+
.split('.')
208+
.filter_map(|p| p.parse().ok())
209+
.collect();
210+
211+
if parts.is_empty() {
212+
return None;
213+
}
214+
215+
let major = parts[0];
216+
let minor = if parts.len() > 1 { parts[1] } else { 0 };
217+
let mut patch = if parts.len() > 2 { parts[2] } else { 0 };
218+
219+
// Bump patch by revision so r2 > r1 > base
220+
if let Some(rev) = revision {
221+
patch += rev;
222+
}
223+
224+
let mut ver = semver::Version::new(major, minor, patch);
225+
226+
// Any extra info (dates, hashes) goes into pre-release so it sorts LOWER
227+
// than the clean version.
228+
if let Some(extra_str) = extra {
229+
let normalized: String = extra_str
230+
.chars()
231+
.map(|c| {
232+
if c.is_alphanumeric() || c == '.' {
233+
c
234+
} else {
235+
'.'
236+
}
237+
})
238+
.collect();
239+
if let Ok(pre) = semver::Prerelease::new(&normalized) {
240+
ver.pre = pre;
241+
}
242+
}
243+
244+
Some(ver)
245+
}
246+
247+
/// Extract a trailing -rN release revision from a version string.
248+
/// Returns (base_without_rN, Some(N)) or (original, None).
249+
fn extract_release_revision(v: &str) -> (&str, Option<u64>) {
250+
if let Some(idx) = v.rfind("-r") {
251+
let after = &v[idx + 2..];
252+
if let Ok(rev) = after.parse::<u64>() {
253+
return (&v[..idx], Some(rev));
254+
}
255+
}
256+
(v, None)
257+
}
258+
259+
/// Compare two tags by their version, using semver-aware sorting.
260+
fn version_compare_tags(a: &str, b: &str, arch: &str) -> Ordering {
261+
let ver_a = extract_version_from_tag(a, arch);
262+
let ver_b = extract_version_from_tag(b, arch);
263+
264+
match (parse_version_lenient(ver_a), parse_version_lenient(ver_b)) {
265+
(Some(va), Some(vb)) => va.cmp(&vb),
266+
(Some(_), None) => Ordering::Greater,
267+
(None, Some(_)) => Ordering::Less,
268+
(None, None) => a.cmp(b), // fallback to lexicographic
269+
}
270+
}
271+
166272
#[cfg(test)]
167273
mod tests {
168274
use super::*;
@@ -196,6 +302,68 @@ mod tests {
196302
assert_eq!(latest, Some(&"v1.1.0-x86_64-linux".to_string()));
197303
}
198304

305+
#[test]
306+
fn test_get_latest_calver() {
307+
let tags = vec![
308+
"2026.1.8-x86_64-linux".to_string(),
309+
"2026.1.12-x86_64-linux".to_string(),
310+
"2026.2.9-x86_64-linux".to_string(),
311+
"2026.2.17-x86_64-linux".to_string(),
312+
"2026.2.23-x86_64-linux".to_string(),
313+
];
314+
315+
let latest = RegistryClient::get_latest_arch_tag(&tags, "x86_64-linux");
316+
assert_eq!(latest, Some(&"2026.2.23-x86_64-linux".to_string()));
317+
}
318+
319+
#[test]
320+
fn test_get_latest_with_old_date_tags() {
321+
let tags = vec![
322+
"v0.16.1-2026-01-01_1767256526-x86_64-linux".to_string(),
323+
"0.16.1-v0.16.1-2026-01-22_1769071133-x86_64-linux".to_string(),
324+
"0.16.1-x86_64-linux".to_string(),
325+
"0.16.1-r1-x86_64-linux".to_string(),
326+
"0.16.1-r2-x86_64-linux".to_string(),
327+
];
328+
329+
let latest = RegistryClient::get_latest_arch_tag(&tags, "x86_64-linux");
330+
assert_eq!(latest, Some(&"0.16.1-r2-x86_64-linux".to_string()));
331+
}
332+
333+
#[test]
334+
fn test_get_latest_release_revisions() {
335+
let tags = vec![
336+
"1.0.0-x86_64-linux".to_string(),
337+
"1.0.0-r1-x86_64-linux".to_string(),
338+
"1.0.0-r2-x86_64-linux".to_string(),
339+
];
340+
341+
let latest = RegistryClient::get_latest_arch_tag(&tags, "x86_64-linux");
342+
assert_eq!(latest, Some(&"1.0.0-r2-x86_64-linux".to_string()));
343+
}
344+
345+
#[test]
346+
fn test_version_compare_v_prefix_vs_no_prefix() {
347+
let tags = vec![
348+
"v0.16.1-x86_64-linux".to_string(),
349+
"0.16.2-x86_64-linux".to_string(),
350+
];
351+
352+
let latest = RegistryClient::get_latest_arch_tag(&tags, "x86_64-linux");
353+
assert_eq!(latest, Some(&"0.16.2-x86_64-linux".to_string()));
354+
}
355+
356+
#[test]
357+
fn test_two_component_version() {
358+
let tags = vec![
359+
"1.5-x86_64-linux".to_string(),
360+
"1.12-x86_64-linux".to_string(),
361+
];
362+
363+
let latest = RegistryClient::get_latest_arch_tag(&tags, "x86_64-linux");
364+
assert_eq!(latest, Some(&"1.12-x86_64-linux".to_string()));
365+
}
366+
199367
#[test]
200368
fn test_download_url() {
201369
let url = RegistryClient::get_download_url(

sbuild/src/commands/build.rs

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -393,28 +393,22 @@ async fn post_build_processing(
393393
};
394394

395395
let (base_version, remote_version) = {
396-
let version_content = std::fs::read_dir(outdir)
397-
.ok()
398-
.and_then(|entries| {
399-
entries
400-
.filter_map(|e| e.ok())
401-
.find(|e| {
402-
e.path()
403-
.extension()
404-
.map(|ext| ext == "version")
405-
.unwrap_or(false)
406-
})
407-
.and_then(|e| std::fs::read_to_string(e.path()).ok())
408-
});
396+
let version_content = std::fs::read_dir(outdir).ok().and_then(|entries| {
397+
entries
398+
.filter_map(|e| e.ok())
399+
.find(|e| {
400+
e.path()
401+
.extension()
402+
.map(|ext| ext == "version")
403+
.unwrap_or(false)
404+
})
405+
.and_then(|e| std::fs::read_to_string(e.path()).ok())
406+
});
409407

410408
match version_content {
411409
Some(content) => {
412410
let lines: Vec<&str> = content.lines().collect();
413-
let base = lines
414-
.first()
415-
.unwrap_or(&"")
416-
.trim()
417-
.to_string();
411+
let base = lines.first().unwrap_or(&"").trim().to_string();
418412
let base = if base.is_empty() {
419413
"latest".to_string()
420414
} else {
@@ -451,7 +445,13 @@ async fn post_build_processing(
451445
if !uri.is_empty() {
452446
match sbuild_cache::MongoDatabase::connect(&uri).await {
453447
Ok(mongo_db) => mongo_db
454-
.get_revision(cache_pkg_id, &host, &base_version, remote_version.as_deref(), None)
448+
.get_revision(
449+
cache_pkg_id,
450+
&host,
451+
&base_version,
452+
remote_version.as_deref(),
453+
None,
454+
)
455455
.await
456456
.ok(),
457457
Err(e) => {
@@ -465,7 +465,13 @@ async fn post_build_processing(
465465
} else if let Some(ref cache_path) = cli.cache {
466466
match sbuild_cache::CacheDatabase::open(cache_path) {
467467
Ok(cache_db) => cache_db
468-
.get_revision(cache_pkg_id, &host, &base_version, remote_version.as_deref(), None)
468+
.get_revision(
469+
cache_pkg_id,
470+
&host,
471+
&base_version,
472+
remote_version.as_deref(),
473+
None,
474+
)
469475
.ok(),
470476
Err(e) => {
471477
warn!("Failed to open build cache: {}", e);

0 commit comments

Comments
 (0)