Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions mise.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

125 changes: 104 additions & 21 deletions src/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,28 +249,50 @@ impl PlatformInfo {
/// - Prefers sha256 checksums over blake3 (more portable/verifiable)
/// - Preserves URL if missing in self
/// - Preserves url_api if missing in self
/// - Drops the other side's checksum/size/url_api when URLs disagree, since
/// those fields describe a specific artifact and become stale if the URL
/// changes.
pub fn merge_with(&self, other: &PlatformInfo) -> PlatformInfo {
let url_changed = self.url.is_some() && other.url.is_some() && self.url != other.url;

// For checksums, prefer sha256 over blake3 since sha256 comes from
// official releases and is more portable/verifiable
let checksum = match (&self.checksum, &other.checksum) {
(Some(self_cs), Some(other_cs)) => {
let self_is_sha256 = self_cs.starts_with("sha256:");
let other_is_sha256 = other_cs.starts_with("sha256:");
match (self_is_sha256, other_is_sha256) {
(true, _) => Some(self_cs.clone()),
(false, true) => Some(other_cs.clone()),
(false, false) => Some(self_cs.clone()), // both blake3, use self
// official releases and is more portable/verifiable. If URLs disagree,
// ignore the other side's artifact-bound fields entirely.
let checksum = if url_changed {
self.checksum.clone()
} else {
match (&self.checksum, &other.checksum) {
(Some(self_cs), Some(other_cs)) => {
let self_is_sha256 = self_cs.starts_with("sha256:");
let other_is_sha256 = other_cs.starts_with("sha256:");
match (self_is_sha256, other_is_sha256) {
(true, _) => Some(self_cs.clone()),
(false, true) => Some(other_cs.clone()),
(false, false) => Some(self_cs.clone()), // both blake3, use self
}
}
(Some(cs), None) | (None, Some(cs)) => Some(cs.clone()),
(None, None) => None,
}
(Some(cs), None) | (None, Some(cs)) => Some(cs.clone()),
(None, None) => None,
};

let size = if url_changed {
self.size
} else {
self.size.or(other.size)
};

let url_api = if url_changed {
self.url_api.clone()
} else {
self.url_api.clone().or_else(|| other.url_api.clone())
};

PlatformInfo {
checksum,
size: self.size.or(other.size),
size,
url: self.url.clone().or_else(|| other.url.clone()),
url_api: self.url_api.clone().or_else(|| other.url_api.clone()),
url_api,
conda_deps: self.conda_deps.clone().or_else(|| other.conda_deps.clone()),
provenance: match (self.provenance.clone(), other.provenance.clone()) {
(Some(a), Some(b)) => Some(a.merge(b)),
Expand Down Expand Up @@ -726,13 +748,35 @@ impl Lockfile {
.iter_mut()
.find(|t| t.version == version && &t.options == options)
{
// Merge with existing platform info, preferring new values when present
// Merge with existing platform info, preferring new values when present.
// When the URL changes, drop existing checksum/size/url_api — those fields
// describe a specific artifact and are stale once the URL points elsewhere.
let merged = if let Some(existing) = tool.platforms.get(platform_key) {
let url_changed = platform_info.url.is_some()
&& existing.url.is_some()
&& platform_info.url != existing.url;
let preserve_artifact_fields = !url_changed;
PlatformInfo {
checksum: platform_info.checksum.or_else(|| existing.checksum.clone()),
size: platform_info.size.or(existing.size),
checksum: platform_info.checksum.or_else(|| {
if preserve_artifact_fields {
existing.checksum.clone()
} else {
None
}
}),
size: platform_info.size.or(if preserve_artifact_fields {
existing.size
} else {
None
}),
url: platform_info.url.or_else(|| existing.url.clone()),
url_api: platform_info.url_api.or_else(|| existing.url_api.clone()),
url_api: platform_info.url_api.or_else(|| {
if preserve_artifact_fields {
existing.url_api.clone()
} else {
None
}
}),
// For conda_deps, always use the new value - None means "no dependencies"
// rather than "not computed", so we shouldn't preserve stale deps
conda_deps: platform_info.conda_deps,
Expand Down Expand Up @@ -2438,15 +2482,17 @@ backend = "conda:jq"

#[test]
fn test_platform_info_merge_prefers_sha256() {
// Test that merge_with prefers sha256 over blake3
// The sha256-vs-blake3 preference only matters when both checksums
// describe the same artifact, so use a shared URL here.
let url = Some("https://example.com/a".to_string());
let sha256_info = PlatformInfo {
checksum: Some("sha256:abc123".to_string()),
url: Some("https://example.com/a".to_string()),
url: url.clone(),
..Default::default()
};
let blake3_info = PlatformInfo {
checksum: Some("blake3:def456".to_string()),
url: Some("https://example.com/b".to_string()),
url: url.clone(),
..Default::default()
};

Expand All @@ -2461,6 +2507,7 @@ backend = "conda:jq"
// blake3 + blake3 -> self (first)
let another_blake3 = PlatformInfo {
checksum: Some("blake3:ghi789".to_string()),
url: url.clone(),
..Default::default()
};
let merged = blake3_info.merge_with(&another_blake3);
Expand All @@ -2473,7 +2520,43 @@ backend = "conda:jq"
..Default::default()
};
let merged = no_url.merge_with(&blake3_info);
assert_eq!(merged.url, Some("https://example.com/b".to_string()));
assert_eq!(merged.url, url);
}

#[test]
fn test_platform_info_merge_drops_stale_checksum_on_url_change() {
// When URLs disagree, the other side's checksum/size/url_api describe
// a different artifact and must not be carried over (e.g. node musl
// URL bumped but old glibc checksum still on disk).
let new_info = PlatformInfo {
checksum: None,
size: None,
url: Some("https://example.com/v2.tar.gz".to_string()),
url_api: None,
..Default::default()
};
let stale_info = PlatformInfo {
checksum: Some("sha256:OLD".to_string()),
size: Some(123),
url: Some("https://example.com/v1.tar.gz".to_string()),
url_api: Some("https://api.example.com/v1".to_string()),
..Default::default()
};

let merged = new_info.merge_with(&stale_info);
assert_eq!(merged.url.as_deref(), Some("https://example.com/v2.tar.gz"));
assert_eq!(merged.checksum, None);
assert_eq!(merged.size, None);
assert_eq!(merged.url_api, None);

// Same artifact (matching URLs) preserves the missing fields.
let same_url = PlatformInfo {
url: Some("https://example.com/v1.tar.gz".to_string()),
..Default::default()
};
let merged = same_url.merge_with(&stale_info);
assert_eq!(merged.checksum.as_deref(), Some("sha256:OLD"));
assert_eq!(merged.size, Some(123));
}

#[test]
Expand Down
88 changes: 65 additions & 23 deletions src/plugins/core/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,14 @@ impl NodePlugin {
let tarball_name = tarball.file_name().unwrap().to_string_lossy().to_string();
let shasums_file = tarball.parent().unwrap().join("SHASUMS256.txt");
HTTP.download_file(
self.shasums_url(version)?,
self.shasums_url(version, &tarball_name)?,
&shasums_file,
Some(ctx.pr.as_ref()),
)
.await?;
if Settings::get().node.gpg_verify != Some(false) && version.starts_with("2") {
self.verify_with_gpg(ctx, &shasums_file, version).await?;
self.verify_with_gpg(ctx, &shasums_file, version, &tarball_name)
.await?;
}
let shasums = file::read_to_string(&shasums_file)?;
let shasums = hash::parse_shasums(&shasums);
Expand All @@ -279,13 +280,14 @@ impl NodePlugin {
ctx: &InstallContext,
shasums_file: &Path,
v: &str,
tarball_name: &str,
) -> Result<()> {
if file::which_non_pristine("gpg").is_none() && Settings::get().node.gpg_verify.is_none() {
warn!("gpg not found, skipping verification");
return Ok(());
}
let sig_file = shasums_file.with_extension("asc");
let sig_url = format!("{}.sig", self.shasums_url(v)?);
let sig_url = format!("{}.sig", self.shasums_url(v, tarball_name)?);
if let Err(e) = HTTP
.download_file(sig_url, &sig_file, Some(ctx.pr.as_ref()))
.await
Expand Down Expand Up @@ -408,13 +410,11 @@ impl NodePlugin {
.execute()
}

fn shasums_url(&self, v: &str) -> Result<Url> {
fn shasums_url(&self, v: &str, tarball_name: &str) -> Result<Url> {
// let url = MISE_NODE_MIRROR_URL.join(&format!("v{v}/SHASUMS256.txt.asc"))?;
let settings = Settings::get();
let url = settings
.node
.mirror_url()
.join(&format!("v{v}/SHASUMS256.txt"))?;
let url =
mirror_url_for(&settings.node, tarball_name).join(&format!("v{v}/SHASUMS256.txt"))?;
Ok(url)
}

Expand Down Expand Up @@ -671,10 +671,9 @@ impl Backend for NodePlugin {
format!("{slug}.tar.gz")
};

// Use Node.js mirror URL to construct download URL
let url = settings
.node
.mirror_url()
// Use Node.js mirror URL to construct download URL.
// Musl tarballs live on unofficial-builds, not nodejs.org/dist.
let url = mirror_url_for(&settings.node, &filename)
.join(&format!("v{version}/{filename}"))
.map_err(|e| eyre::eyre!("Failed to construct Node.js download URL: {e}"))?;

Expand Down Expand Up @@ -725,18 +724,16 @@ impl Backend for NodePlugin {
format!("{slug}.tar.gz")
};

// Build download URL
let url = settings
.node
.mirror_url()
// Build download URL. Musl tarballs live on unofficial-builds; pick the
// mirror once and use it for both the tarball URL and SHASUMS so the
// recorded checksum matches the recorded URL.
let mirror = mirror_url_for(&settings.node, &filename);
let url = mirror
.join(&format!("v{version}/{filename}"))
.map_err(|e| eyre::eyre!("Failed to construct Node.js download URL: {e}"))?;

// Fetch SHASUMS256.txt to get checksum without downloading the tarball
let shasums_url = settings
.node
.mirror_url()
.join(&format!("v{version}/SHASUMS256.txt"))?;
let shasums_url = mirror.join(&format!("v{version}/SHASUMS256.txt"))?;
let checksum = fetch_checksum_from_shasums(shasums_url.as_str(), &filename).await;

Ok(PlatformInfo {
Expand Down Expand Up @@ -845,16 +842,27 @@ impl BuildOpts {
.join(&format!("v{v}/{source_tarball_name}"))?,
source_tarball_name,
binary_tarball_path: tv.download_path().join(&binary_tarball_name),
binary_tarball_url: settings
.node
.mirror_url()
binary_tarball_url: mirror_url_for(&settings.node, &binary_tarball_name)
.join(&format!("v{v}/{binary_tarball_name}"))?,
binary_tarball_name,
install_path,
})
}
}

/// `nodejs.org/dist` does not host musl tarballs; they live at unofficial-builds.
/// When a filename references a musl artifact and the user has not explicitly set
/// `node.mirror_url`, route URL construction (and the matching `SHASUMS256.txt`)
/// to the unofficial-builds host so the URL and checksum stay consistent.
const UNOFFICIAL_NODE_MIRROR_URL: &str = "https://unofficial-builds.nodejs.org/download/release/";

fn mirror_url_for(node: &crate::config::settings::SettingsNode, filename: &str) -> Url {
if node.mirror_url.is_none() && filename.contains("-musl") {
return Url::parse(UNOFFICIAL_NODE_MIRROR_URL).unwrap();
}
node.mirror_url()
}
Comment on lines +859 to +865

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of mirror_url_for only checks the node.mirror_url configuration field. Following the principle of using provided parameters and their methods (like node.mirror_url()) ensures that environment variables and tool-specific configurations are correctly respected without re-deriving them from a general context.

Suggested change
fn mirror_url_for(node: &crate::config::settings::SettingsNode, filename: &str) -> Url {
if node.mirror_url.is_none() && filename.contains("-musl") {
return Url::parse(UNOFFICIAL_NODE_MIRROR_URL).unwrap();
}
node.mirror_url()
}
fn mirror_url_for(node: &crate::config::settings::SettingsNode, filename: &str) -> Url {
let mirror = node.mirror_url();
if filename.contains("-musl") && mirror.as_str() == DEFAULT_NODE_MIRROR_URL {
return Url::parse(UNOFFICIAL_NODE_MIRROR_URL).unwrap();
}
mirror
}
References
  1. When using provided parameters, use their methods to access values instead of re-deriving them from a more general context to ensure specific configurations are preserved.


fn os() -> &'static str {
NodePlugin::map_os(built_info::CFG_OS)
}
Expand Down Expand Up @@ -893,3 +901,37 @@ struct NodeVersion {
date: Option<String>,
files: Vec<String>,
}

#[cfg(test)]
mod tests {
use super::*;
use crate::config::settings::SettingsNode;

#[test]
fn test_mirror_url_for_routes_musl_to_unofficial_builds() {
let node = SettingsNode::default();
let url = mirror_url_for(&node, "node-v24.14.0-linux-x64-musl.tar.gz");
assert_eq!(url.as_str(), UNOFFICIAL_NODE_MIRROR_URL);
}

#[test]
fn test_mirror_url_for_keeps_default_for_glibc() {
let node = SettingsNode::default();
let url = mirror_url_for(&node, "node-v24.14.0-linux-x64.tar.gz");
assert_eq!(url.as_str(), DEFAULT_NODE_MIRROR_URL);
}

#[test]
fn test_mirror_url_for_respects_explicit_mirror() {
// If the user has a custom mirror, route everything through it —
// they may be using a corporate mirror that does host musl builds.
let node = SettingsNode {
mirror_url: Some("https://corp.example/node/".to_string()),
..Default::default()
};
let glibc = mirror_url_for(&node, "node-v24.14.0-linux-x64.tar.gz");
let musl = mirror_url_for(&node, "node-v24.14.0-linux-x64-musl.tar.gz");
assert_eq!(glibc.as_str(), "https://corp.example/node/");
assert_eq!(musl.as_str(), "https://corp.example/node/");
}
}
Loading