Skip to content

Commit 75e478e

Browse files
QaidVoidQaidVoid
authored
fix: correct binary resolution, per-host side files, and index noise (#14)
* fix(meta): keep desktop resources out of binaries * feat(port): pin side files per host, clarify audit output * feat(validate): catch one url serving several hosts * refactor(meta): stop deriving boilerplate notes * fix(meta): publish install paths as written
1 parent 9d9a0a6 commit 75e478e

5 files changed

Lines changed: 127 additions & 77 deletions

File tree

‎src/commands/port.rs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ pub async fn run(command: PortCommands) -> Result<(), String> {
207207
let egaps = hashfill::extra_gaps(&root);
208208
if !egaps.is_empty() {
209209
println!("hashing {} side files ...", egaps.len());
210-
let mut per_file: std::collections::BTreeMap<PathBuf, Vec<(String, String, String, String)>> =
210+
let mut per_file: std::collections::BTreeMap<PathBuf, Vec<(String, String, Option<String>, String, String)>> =
211211
Default::default();
212212
let mut efailed = 0;
213213
let mut estream = futures::stream::iter(egaps.iter().map(|g| {
@@ -228,7 +228,7 @@ pub async fn run(command: PortCommands) -> Result<(), String> {
228228
Ok((b3, sha, _)) => per_file
229229
.entry(g.path.clone())
230230
.or_default()
231-
.push((g.url.clone(), g.to.clone(), b3, sha)),
231+
.push((g.url.clone(), g.to.clone(), g.host.clone(), b3, sha)),
232232
Err(e) => {
233233
efailed += 1;
234234
eprintln!(" {} {}: {e}", "FAIL".red(), g.url);
@@ -260,9 +260,13 @@ pub async fn run(command: PortCommands) -> Result<(), String> {
260260
}
261261
println!("\n{ok}/{} verified against real archive contents", findings.len());
262262
if !unlistable.is_empty() {
263-
// Single-file compression carries no member list, so these
264-
// are unchecked rather than wrong.
265-
println!("\n{} unlistable (single-file compression):", unlistable.len());
263+
// A bare binary, or one compressed on its own, has no members
264+
// to list. The artifact is the file the package installs, so
265+
// there are no interior paths that could be wrong.
266+
println!(
267+
"\n{} not archives, nothing to verify inside:",
268+
unlistable.len()
269+
);
266270
for f in &unlistable {
267271
println!(" {} ({})", f.package, f.host);
268272
}

‎src/port/hashfill.rs‎

Lines changed: 63 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ use sha2::{Digest, Sha256};
1212

1313
/// A side file declared in pkg.toml that this version has not pinned yet.
1414
pub struct ExtraGap {
15+
/// Which host this gap is for, when the URL is arch-dependent.
16+
pub host: Option<String>,
1517
pub path: std::path::PathBuf,
1618
/// URL the client will fetch it from.
1719
pub url: String,
@@ -46,27 +48,59 @@ pub fn extra_gaps(root: &Path) -> Vec<ExtraGap> {
4648
paths.sort();
4749
for (v, path) in p.versions.iter().zip(paths) {
4850
for e in &p.pkg.extra {
49-
let pinned = v
50-
.extra
51-
.iter()
52-
.any(|x| x.to == e.to && x.blake3.is_some());
53-
if pinned {
54-
continue;
55-
}
56-
// A vendored licence resolves to this repository rather than
57-
// upstream, which is the point: some hosts rate-limit and some
58-
// upstreams are gone.
59-
let (url, local) = match (&e.url, &e.license) {
60-
(Some(u), _) => (u.replace("${version}", &v.version), None),
61-
(None, Some(spdx)) => (
62-
format!(
63-
"https://raw.githubusercontent.com/pkgforge/soarpkgs/main/licenses/{spdx}.txt"
64-
),
65-
Some(root.join("licenses").join(format!("{spdx}.txt"))),
66-
),
67-
_ => continue,
51+
// A side file whose URL names an architecture is a different
52+
// file on every host, so it is pinned once per host rather
53+
// than once per package.
54+
let hosts: Vec<Option<String>> = match &e.url {
55+
Some(u) if u.contains("${arch}") => p
56+
.pkg
57+
.host
58+
.supported
59+
.iter()
60+
.map(|h| Some(h.clone()))
61+
.collect(),
62+
_ => vec![None],
6863
};
69-
out.push(ExtraGap { path: path.clone(), url, to: e.to.clone(), local });
64+
for host in hosts {
65+
let pinned = v
66+
.extra
67+
.iter()
68+
.any(|x| x.to == e.to && x.host == host && x.blake3.is_some());
69+
if pinned {
70+
continue;
71+
}
72+
// A vendored licence resolves to this repository rather
73+
// than upstream, which is the point: some hosts rate-limit
74+
// and some upstreams are gone.
75+
let (url, local) = match (&e.url, &e.license) {
76+
(Some(u), _) => (u.replace("${version}", &v.version), None),
77+
(None, Some(spdx)) => (
78+
format!(
79+
"https://raw.githubusercontent.com/pkgforge/soarpkgs/main/licenses/{spdx}.txt"
80+
),
81+
Some(root.join("licenses").join(format!("{spdx}.txt"))),
82+
),
83+
_ => continue,
84+
};
85+
// `${arch}` is whatever upstream calls the architecture,
86+
// which is not always what the host is called.
87+
let url = match &host {
88+
Some(h) => {
89+
let raw = h.split('-').next().unwrap_or(h);
90+
let arch =
91+
p.pkg.arch.get(raw).cloned().unwrap_or_else(|| raw.to_string());
92+
url.replace("${arch}", &arch)
93+
}
94+
None => url,
95+
};
96+
out.push(ExtraGap {
97+
host,
98+
path: path.clone(),
99+
url,
100+
to: e.to.clone(),
101+
local,
102+
});
103+
}
70104
}
71105
}
72106
}
@@ -152,14 +186,18 @@ pub async fn digests(client: &reqwest::Client, url: &str) -> Result<(String, Str
152186
/// Append resolved side files to a version file.
153187
pub fn merge_extras(
154188
path: &Path,
155-
new: &[(String, String, String, String)],
189+
new: &[(String, String, Option<String>, String, String)],
156190
) -> Result<(), String> {
157191
let raw = fs::read_to_string(path).map_err(|e| e.to_string())?;
158192
let mut s = raw.trim_end().to_string();
159-
for (url, to, b3, sha) in new {
160-
s.push_str(&format!(
161-
"\n\n[[extra]]\nurl = {url:?}\nto = {to:?}\nblake3 = {b3:?}\nsha256 = {sha:?}"
162-
));
193+
for (url, to, host, b3, sha) in new {
194+
s.push_str("\n\n[[extra]]");
195+
s.push_str(&format!("\nurl = {url:?}"));
196+
s.push_str(&format!("\nto = {to:?}"));
197+
if let Some(h) = host {
198+
s.push_str(&format!("\nhost = {h:?}"));
199+
}
200+
s.push_str(&format!("\nblake3 = {b3:?}\nsha256 = {sha:?}"));
163201
}
164202
s.push('\n');
165203
fs::write(path, s).map_err(|e| e.to_string())

‎src/port/meta.rs‎

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub struct Entry {
3131
pub homepage: Vec<String>,
3232
pub license: Vec<String>,
3333
pub maintainer: Vec<String>,
34+
#[serde(skip_serializing_if = "Vec::is_empty")]
3435
pub note: Vec<String>,
3536
pub category: Vec<String>,
3637
pub provides: Vec<String>,
@@ -70,57 +71,36 @@ pub struct Binary {
7071
pub link_as: Option<String>,
7172
}
7273

73-
/// Drop a leading archive-root component.
74+
/// Whether an installed file is a desktop-integration resource rather than an
75+
/// executable.
7476
///
75-
/// Install paths are written against the archive as published, but soar
76-
/// promotes a single top-level directory away before locating binaries. A
77-
/// path with no directory component is already at the root.
78-
fn strip_archive_root(path: &str) -> String {
79-
match path.split_once('/') {
80-
Some((_, rest)) if !rest.is_empty() => rest.to_string(),
81-
_ => path.to_string(),
82-
}
77+
/// soar treats a non-empty `binaries` as the complete list of things to link,
78+
/// so one icon or desktop entry in there stops the actual binary being found.
79+
fn is_resource(name: &str) -> bool {
80+
let ext = name.rsplit_once('.').map(|(_, e)| e.to_ascii_lowercase());
81+
matches!(ext.as_deref(), Some("desktop" | "png" | "svg" | "xpm" | "ico"))
8382
}
8483

8584
/// Expand the two template variables an install path may carry.
8685
fn expand_arch(s: &str, version: &str, arch: &str) -> String {
8786
s.replace("${version}", version).replace("${arch}", arch)
8887
}
8988

90-
/// Rebuild the user-facing note list from the structured fields.
89+
/// Notes a user needs told, and nothing else.
9190
///
92-
/// Notes are presentation, so they are derived here rather than stored once
93-
/// per package in the tree.
94-
fn render_notes(p: &PkgToml, src: &str) -> Vec<String> {
95-
let explicit = &p.pkg.note;
96-
let is_prov = |n: &String| n.starts_with("Official binary from") || n.starts_with("Fetched from");
97-
98-
// A package may carry its own provenance wording; it wins over the
99-
// derived line and keeps the leading position.
100-
let mut out: Vec<String> = explicit.iter().filter(|n| is_prov(n)).cloned().collect();
101-
if out.is_empty() {
102-
out.push(if p.pkg.kind.as_deref() == Some("appimage") {
103-
format!("Fetched from Pre Built Community Created AppImage. Check/Report @ {src}")
104-
} else {
105-
format!("Official binary from {src}")
106-
});
107-
}
108-
109-
if p.pkg.portable {
110-
let suffix = if p.pkg.kind.as_deref() == Some("appimage") {
111-
"Works on AnyLinux"
112-
} else {
113-
"Portable Static Binary"
114-
};
115-
out.push(format!("[PORTABLE] ({suffix})"));
116-
} else {
91+
/// Provenance and portability restate `src_url` and `type`, which the entry
92+
/// already carries, so they are not repeated here as prose. Needing something
93+
/// from the host is the exception: it is a limitation rather than a property,
94+
/// and there is no other field carrying it.
95+
fn render_notes(p: &PkgToml) -> Vec<String> {
96+
let mut out = Vec::new();
97+
if !p.pkg.portable {
11798
out.push(match &p.pkg.portable_reason {
11899
Some(why) => format!("[NOT PORTABLE] {why}"),
119100
None => "[NOT PORTABLE]".to_string(),
120101
});
121102
}
122-
123-
out.extend(explicit.iter().filter(|n| !is_prov(n)).cloned());
103+
out.extend(p.pkg.note.iter().cloned());
124104
out
125105
}
126106

@@ -136,7 +116,6 @@ pub fn generate(root: &Path, host: &str) -> (Vec<Entry>, Vec<String>) {
136116
}
137117
let fam = p.pkg.family.clone();
138118
let srcs = p.src_urls();
139-
let src0 = srcs.first().cloned().unwrap_or_default();
140119

141120
for v in &pkg.versions {
142121
let Some(url) = v.url.get(host) else { continue };
@@ -178,20 +157,23 @@ pub fn generate(root: &Path, host: &str) -> (Vec<Entry>, Vec<String>) {
178157
let nested = from.trim_start_matches("*/").contains('/');
179158
(base != *to || nested)
180159
&& !to.eq_ignore_ascii_case("LICENSE")
160+
&& !is_resource(to)
181161
&& *from != "*"
182162
})
183163
.map(|(from, to)| Binary {
184164
// The index is generated per host, so templates
185165
// are expanded here rather than shipped for the
186166
// client to resolve.
187-
// The archive root is promoted away before
188-
// binaries are resolved, so publish the path
189-
// relative to what remains.
190-
source: strip_archive_root(&expand_arch(
191-
from,
192-
&v.version,
193-
&arch_for_host,
194-
)),
167+
// Published as written against the archive. An
168+
// archive with one top-level directory has it
169+
// promoted away before binaries are resolved, so
170+
// the client retries without the leading
171+
// component; stripping it here instead would
172+
// discard the only thing telling two
173+
// architectures apart in a multi-arch archive.
174+
source: expand_arch(from, &v.version, &arch_for_host)
175+
.trim_start_matches("*/")
176+
.to_string(),
195177
// Strip soar's provides markers; link_as is a
196178
// plain filename.
197179
link_as: Some(
@@ -211,6 +193,9 @@ pub fn generate(root: &Path, host: &str) -> (Vec<Entry>, Vec<String>) {
211193
.extra
212194
.iter()
213195
.filter(|e| e.blake3.is_some() || e.sha256.is_some())
196+
// A side file pinned per host belongs only to that host's
197+
// index; one without a host applies to all of them.
198+
.filter(|e| e.host.as_deref().is_none_or(|h| h == host))
214199
.map(|e| ExtraFile {
215200
url: e.url.clone(),
216201
to: e.to.clone(),
@@ -221,7 +206,7 @@ pub fn generate(root: &Path, host: &str) -> (Vec<Entry>, Vec<String>) {
221206

222207
{
223208
let prov = provides.clone();
224-
let mut note = render_notes(p, &src0);
209+
let mut note = render_notes(p);
225210
if let Some(n) = &note_src {
226211
note = n.clone();
227212
}

‎src/port/model.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ pub struct Extra {
142142
pub struct PinnedExtra {
143143
pub url: String,
144144
pub to: String,
145+
/// Set when the file differs per host, as an upstream's per-arch binary
146+
/// does. Absent means it applies to every host, which is the case for a
147+
/// licence.
148+
#[serde(default)]
149+
pub host: Option<String>,
145150
#[serde(default)]
146151
pub blake3: Option<String>,
147152
#[serde(default)]

‎src/port/validate.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,24 @@ pub fn run(root: &Path) -> Report {
7373
errors.push(format!("{tag}: hash for {host} with no url"));
7474
}
7575
}
76+
77+
// One URL serving several hosts is only right when the artifact
78+
// holds every architecture and the install map picks between them.
79+
// Without that, one architecture is being handed another's binary.
80+
let distinct: std::collections::BTreeSet<&String> = v.url.values().collect();
81+
if v.url.len() > 1 && distinct.len() == 1 {
82+
let selects_arch = p
83+
.pkg
84+
.source
85+
.as_ref()
86+
.is_some_and(|s| s.install.keys().any(|k| k.contains("${arch}")));
87+
if !selects_arch {
88+
errors.push(format!(
89+
"{tag}: one url for {} hosts and no ${{arch}} in the install map",
90+
v.url.len()
91+
));
92+
}
93+
}
7694
}
7795
}
7896

0 commit comments

Comments
 (0)