Skip to content

Commit 1d7f494

Browse files
nbafrankclaude
andcommitted
Release v0.3.3: hotfix v0.3.2 Linux PPM end-to-end (BLOCK + HIGH)
Code review on v0.3.2 caught two silent-failure bugs that made the new Linux PPM binaries feature broken end-to-end. Both produce broken R libraries with no error — packages "install" but fail at library() time because the wrong tarball got extracted to the lib dir. BLOCK — Bioc on Linux installs source as binary P3MBinaryIndex::fetch was unconditionally fetching bioconductor.org/packages/<release>/bioc/src/contrib/PACKAGES.gz on Linux. That index lists SOURCE tarballs (Bioc doesn't serve Linux binaries), but they got registered in the binary index and routed through install_binary_package — which extracts them as if they had pre-built libs/*.so. Result: any Bioc package on a Linux project was silently broken. Fix: gate bioc_fut with `!info.is_linux`. HIGH — Per-package downloads didn't carry the R-shaped User-Agent PPM serves source vs. binary at the same URL based on UA. v0.3.2 set the UA on the index fetch (correctly got binary URLs in the listing) but the per-package downloads in download_one used a bare client.get(), so PPM served source for every actual tarball. We extracted the source trees as if they were binaries → unloadable installs. Fix: threaded a `user_agent: Option<&str>` field through DownloadSpec and download_one; sync.rs sets it for any URL containing /__linux__/. MED — r_minor wired into the UA (drops hardcoded "4.5.3") Future-proof against PPM tightening its UA sniffing rules. The actual project R minor was already in scope at the call site. Tests added: - ppm_codename_rhel_centos_naming_discontinuity: pins the rhel-7/8 → centos7/8 / rhel-9 → rhel9 mapping (the asymmetric naming is the highest-value regression anchor in the table). - linux_user_agent_uses_r_minor_not_hardcoded: asserts the UA reflects the wired r_minor for both x86_64 and aarch64. 163 uvr-core tests, 31 CLI tests, 45 unit tests. Clippy clean (added a single allow for too_many_arguments on download_one — 8 params is past clippy's 7-arg threshold but the alternative of bundling into a struct adds more noise than it saves). Per cadence rule: install-blocking-bug exception applies — v0.3.2 was a feature release whose feature is broken; v0.3.3 is the right fix. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent c3fd1ab commit 1d7f494

6 files changed

Lines changed: 158 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,41 @@ release page on GitHub. Issue numbers reference https://github.com/nbafrank/uvr/
77

88
Pure tracking section — fixes and small features land here between tags.
99

10+
## v0.3.3 (2026-04-30)
11+
12+
Hotfix for v0.3.2's Linux PPM binaries — the feature was end-to-end
13+
broken in two ways found by post-release code review. Both produced
14+
silent install corruption (no error, just unloadable packages at
15+
`library()` time). Allowed under the cadence rule's install-blocking
16+
exception.
17+
18+
### Fixes
19+
- **Linux PPM package downloads now carry the R-shaped User-Agent.**
20+
v0.3.2 set the UA on the index fetch (which correctly returned binary
21+
URLs) but not on the per-package tarball downloads. PPM serves source
22+
vs. binary at the same URL based on UA — without it on the download,
23+
every "binary" package was actually a source tarball that
24+
`install_binary_package` extracted as if it had pre-compiled `.so`
25+
files. Threaded a `user_agent` field through `DownloadSpec` and
26+
`download_one`; sync sets it for any URL containing `/__linux__/`.
27+
- **Bioconductor packages on Linux no longer enter the binary path.**
28+
v0.3.2 fetched `bioconductor.org/packages/<release>/bioc/src/contrib/PACKAGES.gz`
29+
on Linux and registered those URLs in the binary index — but
30+
Bioconductor doesn't serve Linux binaries, so the tarballs at those
31+
URLs are sources. They got installed as binaries → unloadable. Now
32+
guarded with `!info.is_linux` so Bioc on Linux falls through to
33+
source install (same path as before #55).
34+
- **Linux UA uses the project's actual R minor instead of a hardcoded
35+
`4.5.3`.** Future-proof against PPM tightening its UA sniffing.
36+
37+
### Tests
38+
- New `ppm_codename_rhel_centos_naming_discontinuity` pins the
39+
rhel-7/8 → centos7/8 / rhel-9 → rhel9 mapping (the asymmetry was an
40+
unsigned cliff in the table).
41+
- New `linux_user_agent_uses_r_minor_not_hardcoded` asserts both arch
42+
variants (x86_64, aarch64) get the right UA from the wired-through
43+
`r_minor`.
44+
1045
## v0.3.2 (2026-04-30)
1146

1247
Linux gets pre-built binary packages. The platform-support table no

Cargo.lock

Lines changed: 2 additions & 2 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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ members = ["crates/uvr", "crates/uvr-core"]
33
resolver = "2"
44

55
[workspace.package]
6-
version = "0.3.2"
6+
version = "0.3.3"
77
edition = "2021"
88
authors = ["uvr contributors"]
99
license = "MIT"

crates/uvr-core/src/installer/download.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ impl Downloader {
4949
let url = spec.url.to_string();
5050
let fallback_url = spec.fallback_url.map(str::to_string);
5151
let is_binary = spec.is_binary;
52+
let user_agent = spec.user_agent.map(str::to_string);
5253
// Binary packages: lockfile checksum is for the source tarball, not the
5354
// P3M binary. Skip verification on binary downloads, but keep the
5455
// checksum for the fallback path which downloads the source tarball.
@@ -62,14 +63,16 @@ impl Downloader {
6263
tokio::spawn(async move {
6364
let _permit = sem.acquire().await.unwrap();
6465

65-
// Try primary URL
66+
// Try primary URL. The UA override only applies to the
67+
// primary path — fallbacks (CRAN source) don't need it.
6668
let primary_result = download_one(
6769
&client,
6870
&cache_dir,
6971
&pkg_name,
7072
&pkg_version,
7173
&url,
7274
primary_checksum.as_deref(),
75+
user_agent.as_deref(),
7376
&mp,
7477
)
7578
.await;
@@ -100,6 +103,7 @@ impl Downloader {
100103
&pkg_version,
101104
fallback,
102105
source_checksum.as_deref(),
106+
None, // fallback URL is plain CRAN source — no UA override needed
103107
&mp,
104108
)
105109
.await?;
@@ -129,6 +133,11 @@ pub struct DownloadSpec<'a> {
129133
/// Fallback URL to try if primary fails (e.g. source tarball when P3M binary 500s).
130134
pub fallback_url: Option<&'a str>,
131135
pub is_binary: bool,
136+
/// Per-request `User-Agent` override. Set this for Linux PPM binary
137+
/// URLs — PPM uses the UA to choose between binary and source builds
138+
/// served at the same URL, and the default `uvr/x.y.z` UA gets you
139+
/// source. None = use the client's default UA.
140+
pub user_agent: Option<&'a str>,
132141
}
133142

134143
/// Result of downloading a single package.
@@ -158,13 +167,15 @@ fn cran_archive_url(url: &str) -> Option<String> {
158167
Some(format!("{base}/src/contrib/Archive/{pkg_name}/{filename}"))
159168
}
160169

170+
#[allow(clippy::too_many_arguments)]
161171
async fn download_one(
162172
client: &reqwest::Client,
163173
cache_dir: &Path,
164174
name: &str,
165175
version: &str,
166176
url: &str,
167177
expected_checksum: Option<&str>,
178+
user_agent: Option<&str>,
168179
mp: &MultiProgress,
169180
) -> Result<PathBuf> {
170181
// Derive the cache filename from the URL so that source tarballs (.tar.gz)
@@ -230,14 +241,23 @@ async fn download_one(
230241

231242
// Stream response to a temp file to avoid buffering entire packages in RAM.
232243
// Compute checksums on-the-fly during the stream.
233-
let mut resp_result = match client.get(url).send().await {
244+
let request = |target: &str| {
245+
let mut req = client.get(target);
246+
if let Some(ua) = user_agent {
247+
req = req.header(reqwest::header::USER_AGENT, ua);
248+
}
249+
req
250+
};
251+
let mut resp_result = match request(url).send().await {
234252
Ok(r) => r.error_for_status(),
235253
Err(e) => Err(e),
236254
};
237255
if resp_result.is_err() {
238256
if let Some(archive_url) = cran_archive_url(url) {
239257
debug!("{name}: {url} failed, retrying via CRAN Archive: {archive_url}");
240-
resp_result = match client.get(&archive_url).send().await {
258+
// CRAN Archive doesn't require the R-shaped UA, but plumbing the
259+
// override here is harmless and keeps requests symmetric.
260+
resp_result = match request(&archive_url).send().await {
241261
Ok(r) => r.error_for_status(),
242262
Err(e) => Err(e),
243263
};

crates/uvr-core/src/registry/p3m.rs

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl P3MBinaryIndex {
4545
bioc_release: Option<&str>,
4646
posit_distro_slug: Option<&str>,
4747
) -> Self {
48-
let Some(info) = platform_info(platform, posit_distro_slug) else {
48+
let Some(info) = platform_info(platform, posit_distro_slug, r_minor) else {
4949
// Unsupported platform combination: macOS x86 / Windows always
5050
// OK; Linux only when we recognise the distro. Falls through
5151
// to source — same behavior as before #55.
@@ -55,10 +55,19 @@ impl P3MBinaryIndex {
5555
let cran_fut = fetch_repo_index(client, r_minor, &info, P3MRepo::Cran);
5656
let bioc_fut = async {
5757
match bioc_release {
58-
Some(release) => fetch_repo_index(client, r_minor, &info, P3MRepo::Bioc(release))
59-
.await
60-
.ok(),
61-
None => None,
58+
// BLOCK fix: Bioconductor doesn't serve Linux binaries — its
59+
// /packages/<release>/bioc/src/contrib/PACKAGES.gz is the
60+
// SOURCE index. Without this guard, Linux projects with Bioc
61+
// deps would register source tarball URLs in the binary index,
62+
// and `install_binary_package` would extract a source tree
63+
// (R/, src/, no libs/*.so) into the library — silently broken
64+
// installs that fail at `library()` time.
65+
Some(release) if !info.is_linux => {
66+
fetch_repo_index(client, r_minor, &info, P3MRepo::Bioc(release))
67+
.await
68+
.ok()
69+
}
70+
_ => None,
6271
}
6372
};
6473
let (cran_result, bioc_result) = tokio::join!(cran_fut, bioc_fut);
@@ -280,7 +289,11 @@ struct PlatformInfo {
280289
/// `posit_distro_slug` should be the same slug uvr uses for the R install
281290
/// URL (`ubuntu-2204`, `debian-12`, `rhel-9`, …); we translate it to PPM's
282291
/// codename system (`jammy`, `bookworm`, `rhel9`) here.
283-
fn platform_info(platform: Platform, posit_distro_slug: Option<&str>) -> Option<PlatformInfo> {
292+
fn platform_info(
293+
platform: Platform,
294+
posit_distro_slug: Option<&str>,
295+
r_minor: &str,
296+
) -> Option<PlatformInfo> {
284297
match platform {
285298
Platform::MacOsArm64 => Some(PlatformInfo {
286299
url_segment: "macosx/big-sur-arm64".to_string(),
@@ -317,10 +330,11 @@ fn platform_info(platform: Platform, posit_distro_slug: Option<&str>) -> Option<
317330
} else {
318331
"aarch64"
319332
};
320-
// R prints UA as `R (<ver> <triple> <arch> <os>-gnu)`. We use a
321-
// current R minor; PPM only checks for "R " prefix and a Linux
322-
// arch in the triple — exact ver isn't material.
323-
let user_agent = format!("R (4.5.3 {arch}-pc-linux-gnu {arch} linux-gnu)");
333+
// R prints UA as `R (<ver> <triple> <arch> <os>-gnu)`. PPM
334+
// currently sniffs the "R " prefix and the linux-gnu marker;
335+
// pass the actual r_minor so the UA stays plausibly current
336+
// if PPM tightens its sniffing rules.
337+
let user_agent = format!("R ({r_minor}.0 {arch}-pc-linux-gnu {arch} linux-gnu)");
324338
Some(PlatformInfo {
325339
url_segment: String::new(),
326340
cache_key: format!("linux-{codename}-{arch}"),
@@ -473,15 +487,15 @@ Version: 1.1.4
473487

474488
#[test]
475489
fn platform_info_macos_arm64() {
476-
let info = platform_info(Platform::MacOsArm64, None).unwrap();
490+
let info = platform_info(Platform::MacOsArm64, None, "4.5").unwrap();
477491
assert_eq!(info.url_segment, "macosx/big-sur-arm64");
478492
assert_eq!(info.pkg_ext, "tgz");
479493
assert!(!info.is_linux);
480494
}
481495

482496
#[test]
483497
fn platform_info_windows() {
484-
let info = platform_info(Platform::WindowsX86_64, None).unwrap();
498+
let info = platform_info(Platform::WindowsX86_64, None, "4.5").unwrap();
485499
assert_eq!(info.url_segment, "windows");
486500
assert_eq!(info.pkg_ext, "zip");
487501
assert!(!info.is_linux);
@@ -490,8 +504,8 @@ Version: 1.1.4
490504
#[test]
491505
fn platform_info_linux_supported_distro() {
492506
// #55: Linux gets a binary index when the distro maps to a PPM codename.
493-
let info =
494-
platform_info(Platform::LinuxX86_64, Some("ubuntu-2204")).expect("ubuntu-2204 covered");
507+
let info = platform_info(Platform::LinuxX86_64, Some("ubuntu-2204"), "4.5")
508+
.expect("ubuntu-2204 covered");
495509
assert!(info.is_linux);
496510
assert_eq!(info.linux_codename.as_deref(), Some("jammy"));
497511
assert_eq!(info.pkg_ext, "tar.gz");
@@ -504,10 +518,10 @@ Version: 1.1.4
504518
#[test]
505519
fn platform_info_linux_unknown_distro_none() {
506520
// Slackware isn't on PPM — falls back to None (source-only).
507-
assert!(platform_info(Platform::LinuxX86_64, Some("slackware-15")).is_none());
508-
assert!(platform_info(Platform::LinuxArm64, Some("nixos-2411")).is_none());
521+
assert!(platform_info(Platform::LinuxX86_64, Some("slackware-15"), "4.5").is_none());
522+
assert!(platform_info(Platform::LinuxArm64, Some("nixos-2411"), "4.5").is_none());
509523
// Without a slug, we can't translate.
510-
assert!(platform_info(Platform::LinuxX86_64, None).is_none());
524+
assert!(platform_info(Platform::LinuxX86_64, None, "4.5").is_none());
511525
}
512526

513527
#[test]
@@ -552,6 +566,37 @@ Version: 1.1.4
552566
assert!(ppm_linux_codename("alpine-3.21").is_none());
553567
}
554568

569+
#[test]
570+
fn ppm_codename_rhel_centos_naming_discontinuity() {
571+
// RHEL 9 broke the centosN naming convention because CentOS Stream
572+
// replaced CentOS Linux. Pin both the symmetric and the asymmetric
573+
// cases since they're the highest-value regression anchors when
574+
// someone edits the match table.
575+
assert_eq!(ppm_linux_codename("rhel-7"), Some("centos7"));
576+
assert_eq!(ppm_linux_codename("centos-7"), Some("centos7"));
577+
assert_eq!(ppm_linux_codename("rhel-8"), Some("centos8"));
578+
assert_eq!(ppm_linux_codename("centos-8"), Some("centos8"));
579+
assert_eq!(ppm_linux_codename("rhel-9"), Some("rhel9"));
580+
assert_eq!(ppm_linux_codename("centos-9"), Some("rhel9"));
581+
}
582+
583+
#[test]
584+
fn linux_user_agent_uses_r_minor_not_hardcoded() {
585+
// Reviewer flagged the prior hardcoded "4.5.3" UA as a future
586+
// staleness risk if PPM tightens its UA matching. r_minor flows
587+
// through and shows up in the UA — verify both arch variants.
588+
let info_x86 =
589+
platform_info(Platform::LinuxX86_64, Some("ubuntu-2204"), "4.6").expect("supported");
590+
let ua = info_x86.user_agent.expect("Linux gets a UA");
591+
assert!(ua.starts_with("R (4.6.0 x86_64-pc-linux-gnu"), "got {ua}");
592+
assert!(ua.contains("linux-gnu"));
593+
594+
let info_arm =
595+
platform_info(Platform::LinuxArm64, Some("debian-12"), "4.5").expect("supported");
596+
let ua = info_arm.user_agent.expect("Linux gets a UA");
597+
assert!(ua.starts_with("R (4.5.0 aarch64-pc-linux-gnu"), "got {ua}");
598+
}
599+
555600
#[test]
556601
fn empty_index() {
557602
let idx = P3MBinaryIndex::empty();

crates/uvr/src/commands/sync.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,34 @@ pub async fn install_from_lockfile(
421421
is_binary: bool,
422422
}
423423

424+
// Linux-specific UA. PPM serves source vs. binary at the same URL, gated
425+
// by the User-Agent. The index fetch in `P3MBinaryIndex::fetch` sets this
426+
// UA on its own request; we need to set the same UA on the per-package
427+
// tarball downloads or PPM serves source for those even though the index
428+
// told us they were binary (would extract a source tree as if it were a
429+
// binary package — silent breakage). Built once and attached per-spec
430+
// below.
431+
let detected_platform = Platform::detect();
432+
let linux_ppm_user_agent: Option<String> = match detected_platform {
433+
Ok(p) if matches!(p, Platform::LinuxX86_64 | Platform::LinuxArm64) => {
434+
let slug = uvr_core::r_version::downloader::detect_posit_distro_slug();
435+
uvr_core::registry::p3m::ppm_linux_codename(&slug).map(|_| {
436+
let arch = if matches!(p, Platform::LinuxX86_64) {
437+
"x86_64"
438+
} else {
439+
"aarch64"
440+
};
441+
// R version is the project's actual minor, not a hardcoded
442+
// string — future-proofs the UA against PPM tightening its
443+
// sniffing rules.
444+
format!("R ({r_minor_str}.0 {arch}-pc-linux-gnu {arch} linux-gnu)")
445+
})
446+
}
447+
_ => None,
448+
};
449+
424450
let plans: Vec<PkgPlan> = if !cache_misses.is_empty() {
425-
let p3m = match Platform::detect() {
451+
let p3m = match detected_platform {
426452
Ok(platform) => {
427453
// #55: Linux gets binaries via PPM's `__linux__/<codename>` URL
428454
// space, gated by a User-Agent the registry sets internally.
@@ -516,6 +542,14 @@ pub async fn install_from_lockfile(
516542
url: &p.url,
517543
fallback_url: p.fallback_url.as_deref(),
518544
is_binary: p.is_binary,
545+
// Attach the R-shaped UA only for Linux PPM binary URLs so
546+
// PPM serves the binary tarball, not source. macOS / Windows
547+
// / source-fallback paths leave it None (default uvr UA).
548+
user_agent: if p.is_binary && p.url.contains("/__linux__/") {
549+
linux_ppm_user_agent.as_deref()
550+
} else {
551+
None
552+
},
519553
})
520554
.collect();
521555

0 commit comments

Comments
 (0)