From 212dd582d7a2e59d54449bdc80db753dd7071f8d Mon Sep 17 00:00:00 2001 From: Aria Beingessner Date: Mon, 18 Mar 2024 17:50:02 -0400 Subject: [PATCH] chore: remove last vestiges of old linkage note that the linkage field remains on dist-manifest to avoid breaking old users of cargo-dist-schema who do not think the linkage field is ever optional. this can be migrated away over time. --- cargo-dist-schema/src/lib.rs | 16 +-- cargo-dist/src/build/mod.rs | 1 + cargo-dist/src/lib.rs | 4 - cargo-dist/src/linkage.rs | 103 ++++++++++-------- cargo-dist/src/main.rs | 8 +- .../snapshots/axolotlsay_basic_lies.snap | 12 ++ 6 files changed, 77 insertions(+), 67 deletions(-) diff --git a/cargo-dist-schema/src/lib.rs b/cargo-dist-schema/src/lib.rs index c1df5c1e7..c4354c212 100644 --- a/cargo-dist-schema/src/lib.rs +++ b/cargo-dist-schema/src/lib.rs @@ -116,6 +116,12 @@ pub struct AssetInfo { pub name: String, /// the system it was built on pub system: SystemId, + /// rust-style target triples the Asset natively supports + /// + /// * length 0: not a meaningful question, maybe some static file + /// * length 1: typical of binaries + /// * length 2+: some kind of universal binary + pub target_triples: Vec, /// the linkage of this Asset pub linkage: Option, } @@ -590,14 +596,6 @@ impl Hosting { /// Information about dynamic libraries used by a binary #[derive(Clone, Default, Debug, Deserialize, Serialize, JsonSchema)] pub struct Linkage { - /// The filename of the binary - #[serde(default)] - #[serde(skip_serializing_if = "Option::is_none")] - pub binary: Option, - /// The target triple for which the binary was built - #[serde(default)] - #[serde(skip_serializing_if = "Option::is_none")] - pub target: Option, /// Libraries included with the operating system #[serde(default)] #[serde(skip_serializing_if = "SortedSet::is_empty")] @@ -634,8 +632,6 @@ impl Linkage { /// merge another linkage into this one pub fn extend(&mut self, val: &Linkage) { let Linkage { - binary: _, - target: _, system, homebrew, public_unmanaged, diff --git a/cargo-dist/src/build/mod.rs b/cargo-dist/src/build/mod.rs index 08b6d22f0..ccd0fb1f9 100644 --- a/cargo-dist/src/build/mod.rs +++ b/cargo-dist/src/build/mod.rs @@ -197,6 +197,7 @@ impl BuildExpectations { name: bin.name.clone(), system: dist.system_id.clone(), linkage: Some(linkage), + target_triples: vec![target.clone()], }, ); Ok(()) diff --git a/cargo-dist/src/lib.rs b/cargo-dist/src/lib.rs index c07e80340..ad5a60ce5 100644 --- a/cargo-dist/src/lib.rs +++ b/cargo-dist/src/lib.rs @@ -85,10 +85,6 @@ pub fn do_build(cfg: &Config) -> Result { } } - // Compute linkage data now that we've done all builds - // TODO(#848): delete this line to remove the old linkage system - linkage::add_linkage_to_manifest(cfg, &dist, &mut manifest)?; - // Next the global steps for step in &dist.global_build_steps { if dist.local_builds_are_lies { diff --git a/cargo-dist/src/linkage.rs b/cargo-dist/src/linkage.rs index a6ddcc668..989a6b91d 100644 --- a/cargo-dist/src/linkage.rs +++ b/cargo-dist/src/linkage.rs @@ -8,7 +8,7 @@ use std::{ use axoasset::SourceFile; use axoprocess::Cmd; use camino::Utf8PathBuf; -use cargo_dist_schema::{DistManifest, Library, Linkage}; +use cargo_dist_schema::{AssetInfo, DistManifest, Library, Linkage}; use comfy_table::{presets::UTF8_FULL, Table}; use goblin::Object; use mach_object::{LoadCommand, OFile}; @@ -28,56 +28,41 @@ pub struct LinkageArgs { /// Determinage dynamic linkage of built artifacts (impl of `cargo dist linkage`) pub fn do_linkage(cfg: &Config, args: &LinkageArgs) -> Result<()> { - let (dist, _manifest) = gather_work(cfg)?; - - let reports: Vec = if let Some(target) = args.from_json.clone() { + let manifest = if let Some(target) = args.from_json.clone() { let file = SourceFile::load_local(target)?; file.deserialize_json()? } else { - fetch_linkage(cfg.targets.clone(), dist.artifacts, dist.dist_dir)? + let (dist, mut manifest) = gather_work(cfg)?; + compute_linkage_assuming_local_build(&dist, &mut manifest, cfg)?; + manifest }; if args.print_output { - for report in &reports { - eprintln!("{}", report_linkage(report)); - } + eprintln!("{}", LinkageDisplay(&manifest)); } if args.print_json { - let j = serde_json::to_string(&reports).unwrap(); - println!("{}", j); + let string = serde_json::to_string_pretty(&manifest).unwrap(); + println!("{string}"); } - Ok(()) } -/// Compute the linkage of local builds and add them to the DistManifest -pub fn add_linkage_to_manifest( - cfg: &Config, +/// Assuming someone just ran `cargo dist build` on the current machine, +/// compute the linkage by checking binaries in the temp to-be-zipped dirs. +fn compute_linkage_assuming_local_build( dist: &DistGraph, manifest: &mut DistManifest, -) -> Result<()> { - let linkage = fetch_linkage( - cfg.targets.clone(), - dist.artifacts.clone(), - dist.dist_dir.clone(), - )?; - - manifest.linkage.extend(linkage); - Ok(()) -} - -fn fetch_linkage( - targets: Vec, - artifacts: Vec, - dist_dir: Utf8PathBuf, -) -> DistResult> { - let mut reports = vec![]; + cfg: &Config, +) -> DistResult<()> { + let targets = &cfg.targets; + let artifacts = &dist.artifacts; + let dist_dir = &dist.dist_dir; for target in targets { let artifacts: Vec = artifacts .clone() .into_iter() - .filter(|r| r.target_triples.contains(&target)) + .filter(|r| r.target_triples.contains(target)) .collect(); if artifacts.is_empty() { @@ -88,22 +73,55 @@ fn fetch_linkage( for artifact in artifacts { let path = Utf8PathBuf::from(&dist_dir).join(format!("{}-{target}", artifact.id)); - for (_, binary) in artifact.required_binaries { - let bin_path = path.join(binary); + for (bin_idx, binary_relpath) in artifact.required_binaries { + let bin = dist.binary(bin_idx); + let bin_path = path.join(binary_relpath); if !bin_path.exists() { eprintln!("Binary {bin_path} missing; skipping check"); } else { - reports.push(determine_linkage(&bin_path, &target)?); + let linkage = determine_linkage(&bin_path, target)?; + manifest.assets.insert( + bin.id.clone(), + AssetInfo { + id: bin.id.clone(), + name: bin.name.clone(), + system: dist.system_id.clone(), + linkage: Some(linkage), + target_triples: vec![target.clone()], + }, + ); } } } } - Ok(reports) + Ok(()) +} + +/// Formatter for a DistManifest that prints the linkage human-readably +pub struct LinkageDisplay<'a>(pub &'a DistManifest); + +impl std::fmt::Display for LinkageDisplay<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for asset in self.0.assets.values() { + let Some(linkage) = &asset.linkage else { + continue; + }; + let name = &asset.name; + let targets = asset.target_triples.join(", "); + write!(f, "{name}")?; + if !targets.is_empty() { + write!(f, " ({targets})")?; + } + writeln!(f, "\n")?; + format_linkage_table(f, linkage)?; + } + Ok(()) + } } /// Formatted human-readable output -pub fn report_linkage(linkage: &Linkage) -> String { +fn format_linkage_table(f: &mut std::fmt::Formatter<'_>, linkage: &Linkage) -> std::fmt::Result { let mut table = Table::new(); table .load_preset(UTF8_FULL) @@ -163,14 +181,7 @@ pub fn report_linkage(linkage: &Linkage) -> String { .join("\n") .as_str(), ]); - - use std::fmt::Write; - let mut output = String::new(); - if let (Some(bin), Some(target)) = (&linkage.binary, &linkage.target) { - writeln!(&mut output, "{} ({}):\n", bin, target).unwrap(); - } - write!(&mut output, "{table}").unwrap(); - output + write!(f, "{table}") } /// Create a homebrew library for the given path @@ -379,8 +390,6 @@ pub fn determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult Result<(), std::io::Erro } fn print_human_linkage(out: &mut Term, report: &DistManifest) -> Result<(), std::io::Error> { - for linkage in &report.linkage { - writeln!(out, "{}", linkage::report_linkage(linkage))?; - } - - Ok(()) + writeln!(out, "{}", LinkageDisplay(report)) } fn cmd_build(cli: &Cli, args: &BuildArgs) -> Result<(), miette::Report> { diff --git a/cargo-dist/tests/snapshots/axolotlsay_basic_lies.snap b/cargo-dist/tests/snapshots/axolotlsay_basic_lies.snap index f2b30dba1..4d45f628f 100644 --- a/cargo-dist/tests/snapshots/axolotlsay_basic_lies.snap +++ b/cargo-dist/tests/snapshots/axolotlsay_basic_lies.snap @@ -2407,6 +2407,9 @@ maybeInstall(true).then(run); "id": "axolotlsay-aarch64-apple-darwin-axolotlsay", "name": "axolotlsay", "system": "lies:", + "target_triples": [ + "aarch64-apple-darwin" + ], "linkage": { "other": [ { @@ -2419,6 +2422,9 @@ maybeInstall(true).then(run); "id": "axolotlsay-x86_64-apple-darwin-axolotlsay", "name": "axolotlsay", "system": "lies:", + "target_triples": [ + "x86_64-apple-darwin" + ], "linkage": { "other": [ { @@ -2431,6 +2437,9 @@ maybeInstall(true).then(run); "id": "axolotlsay-x86_64-pc-windows-msvc-axolotlsay", "name": "axolotlsay", "system": "lies:", + "target_triples": [ + "x86_64-pc-windows-msvc" + ], "linkage": { "other": [ { @@ -2443,6 +2452,9 @@ maybeInstall(true).then(run); "id": "axolotlsay-x86_64-unknown-linux-gnu-axolotlsay", "name": "axolotlsay", "system": "lies:", + "target_triples": [ + "x86_64-unknown-linux-gnu" + ], "linkage": { "other": [ {