Skip to content

Commit

Permalink
chore: remove last vestiges of old linkage
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gankra committed Mar 18, 2024
1 parent 367059f commit 212dd58
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 67 deletions.
16 changes: 6 additions & 10 deletions cargo-dist-schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// the linkage of this Asset
pub linkage: Option<Linkage>,
}
Expand Down Expand Up @@ -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<String>,
/// The target triple for which the binary was built
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none")]
pub target: Option<String>,
/// Libraries included with the operating system
#[serde(default)]
#[serde(skip_serializing_if = "SortedSet::is_empty")]
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions cargo-dist/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl BuildExpectations {
name: bin.name.clone(),
system: dist.system_id.clone(),
linkage: Some(linkage),
target_triples: vec![target.clone()],
},
);
Ok(())
Expand Down
4 changes: 0 additions & 4 deletions cargo-dist/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ pub fn do_build(cfg: &Config) -> Result<DistManifest> {
}
}

// 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 {
Expand Down
103 changes: 56 additions & 47 deletions cargo-dist/src/linkage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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<Linkage> = 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<String>,
artifacts: Vec<Artifact>,
dist_dir: Utf8PathBuf,
) -> DistResult<Vec<Linkage>> {
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<Artifact> = artifacts
.clone()
.into_iter()
.filter(|r| r.target_triples.contains(&target))
.filter(|r| r.target_triples.contains(target))
.collect();

if artifacts.is_empty() {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -379,8 +390,6 @@ pub fn determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult<Linkage
};

let mut linkage = Linkage {
binary: Some(path.file_name().unwrap().to_owned()),
target: Some(target.to_owned()),
system: Default::default(),
homebrew: Default::default(),
public_unmanaged: Default::default(),
Expand Down
8 changes: 2 additions & 6 deletions cargo-dist/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::io::Write;
use axoasset::LocalAsset;
use camino::Utf8PathBuf;
// Import everything from the lib version of ourselves
use cargo_dist::*;
use cargo_dist::{linkage::LinkageDisplay, *};
use cargo_dist_schema::{AssetKind, DistManifest};
use clap::Parser;
use cli::{
Expand Down Expand Up @@ -183,11 +183,7 @@ fn print_json(out: &mut Term, report: &DistManifest) -> 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> {
Expand Down
12 changes: 12 additions & 0 deletions cargo-dist/tests/snapshots/axolotlsay_basic_lies.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand All @@ -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": [
{
Expand All @@ -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": [
{
Expand All @@ -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": [
{
Expand Down

0 comments on commit 212dd58

Please sign in to comment.