Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: extra build artifacts #613

Merged
merged 3 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
*.snap.new
/book/book/
cargo-dist/wix
/dist-manifest-schema.json

# Oranda
oranda-debug.log
Expand Down
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ targets = ["x86_64-unknown-linux-gnu", "aarch64-apple-darwin", "x86_64-apple-dar
# Publish jobs to run in CI
pr-run-mode = "plan"

[[workspace.metadata.dist.extra-artifacts]]
artifacts = ["dist-manifest-schema.json"]
build = ["cargo", "run", "--", "dist", "manifest-schema", "--output=dist-manifest-schema.json"]

# The profile that 'cargo dist' will build with
[profile.dist]
inherits = "release"
Expand Down
3 changes: 3 additions & 0 deletions cargo-dist-schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ pub enum ArtifactKind {
/// A tarball containing the source code
#[serde(rename = "source-tarball")]
SourceTarball,
/// Some form of extra artifact produced by a sidecar build
#[serde(rename = "extra-artifact")]
ExtraArtifact,
/// Unknown to this version of cargo-dist-schema
///
/// This is a fallback for forward/backward-compat
Expand Down
15 changes: 15 additions & 0 deletions cargo-dist-schema/src/snapshots/cargo_dist_schema__emit.snap
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,21 @@ expression: json_schema
}
}
},
{
"description": "Some form of extra artifact produced by a sidecar build",
"type": "object",
"required": [
"kind"
],
"properties": {
"kind": {
"type": "string",
"enum": [
"extra-artifact"
]
}
}
},
{
"description": "Unknown to this version of cargo-dist-schema\n\nThis is a fallback for forward/backward-compat",
"type": "object",
Expand Down
6 changes: 5 additions & 1 deletion cargo-dist/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,11 @@ pub enum OutputFormat {
}

#[derive(Args, Clone, Debug)]
pub struct ManifestSchemaArgs {}
pub struct ManifestSchemaArgs {
/// Write the manifest schema to the named file instead of stdout
#[clap(long)]
pub output: Option<String>,
mistydemeo marked this conversation as resolved.
Show resolved Hide resolved
}

#[derive(Args, Clone, Debug)]
pub struct HostArgs {
Expand Down
19 changes: 19 additions & 0 deletions cargo-dist/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ pub struct DistMetadata {
/// Hosting provider
#[serde(skip_serializing_if = "Option::is_none")]
pub hosting: Option<Vec<HostingStyle>>,

/// Any extra artifacts and their buildscripts
#[serde(skip_serializing_if = "Option::is_none")]
pub extra_artifacts: Option<Vec<ExtraArtifact>>,
}

impl DistMetadata {
Expand Down Expand Up @@ -308,6 +312,7 @@ impl DistMetadata {
ssldotcom_windows_sign: _,
msvc_crt_static: _,
hosting: _,
extra_artifacts: _,
} = self;
if let Some(include) = include {
for include in include {
Expand Down Expand Up @@ -353,6 +358,7 @@ impl DistMetadata {
ssldotcom_windows_sign,
msvc_crt_static,
hosting,
extra_artifacts,
} = self;

// Check for global settings on local packages
Expand Down Expand Up @@ -444,6 +450,9 @@ impl DistMetadata {
if publish_jobs.is_none() {
*publish_jobs = workspace_config.publish_jobs.clone();
}
if extra_artifacts.is_none() {
*extra_artifacts = workspace_config.extra_artifacts.clone();
}

// This was historically implemented as extend, but I'm not convinced the
// inconsistency is worth the inconvenience...
Expand Down Expand Up @@ -1034,6 +1043,16 @@ pub enum ProductionMode {
Prod,
}

/// An extra artifact to upload alongside the release tarballs,
/// and the build command which produces it.
#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct ExtraArtifact {
/// The build command to invoke
pub build: Vec<String>,
/// The artifact(s) produced via this build script
pub artifacts: Vec<String>,
}

impl std::fmt::Display for ProductionMode {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down
90 changes: 69 additions & 21 deletions cargo-dist/src/generic_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::{
env,
io::{stderr, Write},
process::Command,
process::{Command, Output},
};

use camino::Utf8Path;
Expand All @@ -14,7 +14,8 @@ use crate::{
copy_file,
env::{calculate_cflags, calculate_ldflags, fetch_brew_env, parse_env, select_brew_env},
errors::Result,
BinaryIdx, BuildStep, DistGraph, DistGraphBuilder, GenericBuildStep, SortedMap, TargetTriple,
BinaryIdx, BuildStep, DistGraph, DistGraphBuilder, ExtraBuildStep, GenericBuildStep, SortedMap,
TargetTriple,
};

impl<'a> DistGraphBuilder<'a> {
Expand Down Expand Up @@ -72,14 +73,12 @@ fn platform_appropriate_cxx(target: &str) -> &str {
}
}

/// Build a generic target
pub fn build_generic_target(dist_graph: &DistGraph, target: &GenericBuildStep) -> Result<()> {
let mut command_string = target.build_command.clone();
eprintln!(
"building generic target ({} via {})",
target.target_triple,
command_string.join(" ")
);
fn run_build(
dist_graph: &DistGraph,
command_string: &[String],
target: Option<&str>,
) -> Result<Output> {
let mut command_string = command_string.to_owned();

let mut desired_extra_env = vec![];
let mut cflags = None;
Expand Down Expand Up @@ -109,16 +108,16 @@ pub fn build_generic_target(dist_graph: &DistGraph, target: &GenericBuildStep) -
// inject into the environment, apply them now.
command.envs(desired_extra_env);

// Ensure we inform the build what architecture and platform
// it's building for.
command.env("CARGO_DIST_TARGET", &target.target_triple);
if let Some(target) = target {
// Ensure we inform the build what architecture and platform
// it's building for.
command.env("CARGO_DIST_TARGET", target);

let cc =
std::env::var("CC").unwrap_or(platform_appropriate_cc(&target.target_triple).to_owned());
command.env("CC", cc);
let cxx =
std::env::var("CXX").unwrap_or(platform_appropriate_cxx(&target.target_triple).to_owned());
command.env("CXX", cxx);
let cc = std::env::var("CC").unwrap_or(platform_appropriate_cc(target).to_owned());
command.env("CC", cc);
let cxx = std::env::var("CXX").unwrap_or(platform_appropriate_cxx(target).to_owned());
command.env("CXX", cxx);
}

// Pass CFLAGS/LDFLAGS for C builds
if let Some(cflags) = cflags {
Expand All @@ -133,10 +132,25 @@ pub fn build_generic_target(dist_graph: &DistGraph, target: &GenericBuildStep) -
}

info!("exec: {:?}", command);
let result = command
command
.output()
.into_diagnostic()
.wrap_err_with(|| format!("failed to exec generic build: {command:?}"))?;
.wrap_err_with(|| format!("failed to exec generic build: {command:?}"))
}

/// Build a generic target
pub fn build_generic_target(dist_graph: &DistGraph, target: &GenericBuildStep) -> Result<()> {
eprintln!(
"building generic target ({} via {})",
target.target_triple,
target.build_command.join(" ")
);

let result = run_build(
dist_graph,
&target.build_command,
Some(&target.target_triple),
)?;

if !result.status.success() {
println!("Build exited non-zero: {}", result.status);
Expand All @@ -163,3 +177,37 @@ pub fn build_generic_target(dist_graph: &DistGraph, target: &GenericBuildStep) -

Ok(())
}

/// Similar to the above, but with slightly different signatures since
/// it's not based around axoproject-identified binaries
pub fn run_extra_artifacts_build(dist_graph: &DistGraph, target: &ExtraBuildStep) -> Result<()> {
eprintln!(
"building extra artifacts target (via {})",
target.build_command.join(" ")
);

let result = run_build(dist_graph, &target.build_command, None)?;
let dest = dist_graph.dist_dir.to_owned();

if !result.status.success() {
println!("Build exited non-zero: {}", result.status);
}
eprintln!();
eprintln!("stdout:");
stderr().write_all(&result.stdout).into_diagnostic()?;
Comment on lines +195 to +197
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a weird juxtaposition... but ok i think i see why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for consistency with the cargo builds, where we have things which consume the underlying build's stdout. I'd run into cases where allowing the output to go to stdout caused other parts of our build process to fail. (This code is actually from the original generic builds code, just duplicated here.)


// Check that we got everything we expected, and copy into the distribution path
for artifact in &target.expected_artifacts {
let binary_path = Utf8Path::new(artifact);
if binary_path.exists() {
copy_file(binary_path, &dest.join(artifact))?;
} else {
return Err(miette!(
"failed to find bin {} -- did the build above have errors?",
binary_path
));
}
}

Ok(())
}
2 changes: 2 additions & 0 deletions cargo-dist/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ fn get_new_dist_metadata(
ssldotcom_windows_sign: None,
msvc_crt_static: None,
hosting: None,
extra_artifacts: None,
}
};

Expand Down Expand Up @@ -770,6 +771,7 @@ fn apply_dist_to_metadata(metadata: &mut toml_edit::Item, meta: &DistMetadata) {
ssldotcom_windows_sign,
msvc_crt_static,
hosting,
extra_artifacts: _,
} = &meta;

apply_optional_value(
Expand Down
3 changes: 2 additions & 1 deletion cargo-dist/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use config::{
ArtifactMode, ChecksumStyle, CompressionImpl, Config, DirtyMode, GenerateMode, ZipStyle,
};
use console::Term;
use generic_build::build_generic_target;
use generic_build::{build_generic_target, run_extra_artifacts_build};
use semver::Version;
use tracing::info;

Expand Down Expand Up @@ -138,6 +138,7 @@ fn run_build_step(
prefix,
target,
}) => Ok(generate_source_tarball(committish, prefix, target)?),
BuildStep::Extra(target) => run_extra_artifacts_build(dist_graph, target),
}
}

Expand Down
11 changes: 9 additions & 2 deletions cargo-dist/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use std::io::Write;

use axoasset::LocalAsset;
use camino::Utf8PathBuf;
// Import everything from the lib version of ourselves
use cargo_dist::{linkage::Linkage, *};
Expand Down Expand Up @@ -492,10 +493,16 @@ fn print_help_markdown(out: &mut dyn Write) -> std::io::Result<()> {

fn cmd_manifest_schema(
_config: &Cli,
_args: &cli::ManifestSchemaArgs,
args: &cli::ManifestSchemaArgs,
) -> Result<(), miette::ErrReport> {
let schema = cargo_dist_schema::DistManifest::json_schema();
let json_schema = serde_json::to_string_pretty(&schema).expect("failed to stringify schema!?");
println!("{json_schema}");

if let Some(destination) = args.output.to_owned() {
let contents = json_schema + "\n";
LocalAsset::write_new(&contents, destination)?;
} else {
println!("{json_schema}");
}
Ok(())
}
5 changes: 5 additions & 0 deletions cargo-dist/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ fn manifest_artifact(
description = None;
kind = cargo_dist_schema::ArtifactKind::SourceTarball;
}
ArtifactKind::ExtraArtifact(_) => {
install_hint = None;
description = None;
kind = cargo_dist_schema::ArtifactKind::ExtraArtifact;
}
};

let checksum = artifact.checksum.map(|idx| dist.artifact(idx).id.clone());
Expand Down
Loading
Loading