Skip to content
Open
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
65 changes: 65 additions & 0 deletions contrib/packaging/bcvk.spec
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
%bcond_without check

Name: bcvk
Version: 0.5.3
Release: 1%{?dist}
Summary: Bootable container VM toolkit

# Apache-2.0 OR MIT
License: Apache-2.0 OR MIT
URL: https://github.com/bootc-dev/bcvk.rpm
Source0: %{url}/releases/download/v%{version}/bcvk-%{version}.tar.zstd
Source1: %{url}/releases/download/v%{version}/bcvk-%{version}-vendor.tar.zstd

# https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval
ExcludeArch: %{ix86}

BuildRequires: make
BuildRequires: openssl-devel
%if 0%{?rhel}
BuildRequires: rust-toolset
%else
BuildRequires: cargo-rpm-macros >= 25
%endif

%description
%{summary}

%prep
%autosetup -p1 -a1
# Default -v vendor config doesn't support non-crates.io deps (i.e. git)
cp .cargo/vendor-config.toml .
%cargo_prep -N
cat vendor-config.toml >> .cargo/config.toml
rm vendor-config.toml

%build
%cargo_build

make manpages

%cargo_vendor_manifest
# https://pagure.io/fedora-rust/rust-packaging/issue/33
sed -i -e '/https:\/\//d' cargo-vendor.txt
%cargo_license_summary
%{cargo_license} > LICENSE.dependencies

%install
%make_install INSTALL="install -p -c"

%if %{with check}
%check
%cargo_test
%endif

%files
%license LICENSE-MIT
%license LICENSE-APACHE
%license LICENSE.dependencies
%license cargo-vendor.txt
%doc README.md
%{_bindir}/bcvk
%{_mandir}/man*/*bcvk*

%changelog
%autochangelog
256 changes: 214 additions & 42 deletions crates/xtask/src/xtask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const TASKS: &[(&str, fn(&Shell) -> Result<()>)] = &[
("update-manpages", update_manpages),
("sync-manpages", sync_manpages),
("package", package),
("package-srpm", package_srpm),
("spec", spec),
];

fn install_tracing() {
Expand Down Expand Up @@ -86,65 +88,235 @@ fn sync_manpages(sh: &Shell) -> Result<()> {
man::sync_all_man_pages(sh)
}

fn package(sh: &Shell) -> Result<()> {
use std::env;
const NAME: &str = "bcvk";
const TAR_REPRODUCIBLE_OPTS: &[&str] = &[
"--sort=name",
"--owner=0",
"--group=0",
"--numeric-owner",
"--pax-option=exthdr.name=%d/PaxHeaders/%f,delete=atime,delete=ctime",
];

fn gitrev_to_version(v: &str) -> String {
let v = v.trim().trim_start_matches('v');
v.replace('-', ".")
}

fn gitrev(sh: &Shell) -> Result<String> {
use xshell::cmd;
if let Ok(rev) = cmd!(sh, "git describe --tags --exact-match")
.ignore_stderr()
.read()
{
Ok(gitrev_to_version(&rev))
} else {
// Just use the version from Cargo.toml
man::get_raw_package_version()
}
}

// Get version from Cargo.toml
let version = man::get_raw_package_version()?;
/// Return the timestamp of the latest git commit in seconds since the Unix epoch.
fn git_source_date_epoch(dir: &str) -> Result<u64> {
let o = Command::new("git")
.args(["log", "-1", "--pretty=%ct"])
.current_dir(dir)
.output()?;
if !o.status.success() {
return Err(eyre!("git exited with an error: {:?}", o));
}
let buf = String::from_utf8(o.stdout).context("Failed to parse git log output")?;
let r = buf.trim().parse()?;
Ok(r)
}

println!("Creating release archives for version {}", version);
/// When using cargo-vendor-filterer --format=tar, the config generated has a bogus source
/// directory. This edits it to refer to vendor/ as a stable relative reference.
fn edit_vendor_config(config: &str) -> Result<String> {
let mut config: toml::Value = toml::from_str(config)?;
let config = config.as_table_mut().unwrap();
let source_table = config.get_mut("source").unwrap();
let source_table = source_table.as_table_mut().unwrap();
let vendored_sources = source_table.get_mut("vendored-sources").unwrap();
let vendored_sources = vendored_sources.as_table_mut().unwrap();
Comment on lines +136 to +140

Choose a reason for hiding this comment

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

medium

This chain of unwrap() calls is brittle and will cause a panic if the structure of the cargo-vendor-filterer output TOML changes. While this is an internal build script, it's better to handle this gracefully with proper error handling. Consider using and_then or ? with ok_or_else to provide more informative error messages.

Suggested change
let config = config.as_table_mut().unwrap();
let source_table = config.get_mut("source").unwrap();
let source_table = source_table.as_table_mut().unwrap();
let vendored_sources = source_table.get_mut("vendored-sources").unwrap();
let vendored_sources = vendored_sources.as_table_mut().unwrap();
let vendored_sources = config
.as_table_mut()
.and_then(|c| c.get_mut("source"))
.and_then(|s| s.as_table_mut())
.and_then(|s| s.get_mut("vendored-sources"))
.and_then(|vs| vs.as_table_mut())
.ok_or_else(|| eyre!("malformed vendor config: missing [source.vendored-sources] table"))?;

let previous =
vendored_sources.insert("directory".into(), toml::Value::String("vendor".into()));
assert!(previous.is_some());

// Get the git commit timestamp for reproducible builds
let source_date_epoch = cmd!(sh, "git log -1 --format=%ct").read()?;
env::set_var("SOURCE_DATE_EPOCH", source_date_epoch.trim());
Ok(config.to_string())
}

// Create target directory if it doesn't exist
sh.create_dir("target")?;
struct Package {
version: String,
srcpath: camino::Utf8PathBuf,
vendorpath: camino::Utf8PathBuf,
}

// Create temporary directory for intermediate files
let tempdir = tempfile::tempdir()?;
let temp_tar = tempdir.path().join(format!("bcvk-{}.tar", version));
fn impl_package(sh: &Shell) -> Result<Package> {
use camino::Utf8Path;
use xshell::cmd;

// Create source archive using git archive (uncompressed initially)
let source_archive = format!("target/bcvk-{}.tar.zstd", version);
cmd!(
sh,
"git archive --format=tar --prefix=bcvk-{version}/ HEAD -o {temp_tar}"
)
.run()?;
let source_date_epoch = git_source_date_epoch(".")?;
let v = gitrev(sh)?;

// Create vendor archive
let vendor_archive = format!("target/bcvk-{}-vendor.tar.zstd", version);
cmd!(
let namev = format!("{NAME}-{v}");
let p = Utf8Path::new("target").join(format!("{namev}.tar"));
let prefix = format!("{namev}/");
cmd!(sh, "git archive --format=tar --prefix={prefix} -o {p} HEAD").run()?;
// Generate the vendor directory now, as we want to embed the generated config to use
// it in our source.
let vendorpath = Utf8Path::new("target").join(format!("{namev}-vendor.tar.zstd"));
let vendor_config = cmd!(
sh,
"cargo vendor-filterer --format=tar.zstd {vendor_archive}"
"cargo vendor-filterer --prefix=vendor --format=tar.zstd {vendorpath}"
)
.run()?;
.read()?;
let vendor_config = edit_vendor_config(&vendor_config)?;
// Append .cargo/vendor-config.toml (a made up filename) into the tar archive.
{
let tmpdir = tempfile::tempdir_in("target")?;
let tmpdir_path = tmpdir.path();
let path = tmpdir_path.join("vendor-config.toml");
std::fs::write(&path, vendor_config)?;
let source_date_epoch = format!("{source_date_epoch}");
cmd!(
sh,
"tar -r -C {tmpdir_path} {TAR_REPRODUCIBLE_OPTS...} --mtime=@{source_date_epoch} --transform=s,^,{prefix}.cargo/, -f {p} vendor-config.toml"
)
.run()?;
}
// Compress with zstd
let srcpath: camino::Utf8PathBuf = format!("{p}.zstd").into();
cmd!(sh, "zstd --rm -f {p} -o {srcpath}").run()?;

println!("Created vendor archive: {}", vendor_archive);
Ok(Package {
version: v,
srcpath,
vendorpath,
})
}

// Create vendor config for the source archive
let vendor_config_content = r#"[source.crates-io]
replace-with = "vendored-sources"
fn package(sh: &Shell) -> Result<()> {
let p = impl_package(sh)?.srcpath;
println!("Generated: {p}");
Ok(())
}

[source.vendored-sources]
directory = "vendor"
"#;
let vendor_config_path = tempdir.path().join(".cargo-vendor-config.toml");
std::fs::write(&vendor_config_path, vendor_config_content)?;
fn update_spec(sh: &Shell) -> Result<camino::Utf8PathBuf> {
use camino::Utf8Path;
use std::fs::File;
use std::io::{BufRead, BufReader, BufWriter, Write};

// Add vendor config to source archive
cmd!(sh, "tar --owner=0 --group=0 --numeric-owner --sort=name --mtime=@{source_date_epoch} -rf {temp_tar} --transform='s|.*/.cargo-vendor-config.toml|bcvk-{version}/.cargo/vendor-config.toml|' {vendor_config_path}").run()?;
let p = Utf8Path::new("target");
let pkg = impl_package(sh)?;

Choose a reason for hiding this comment

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

medium

The impl_package() function, which can be time-consuming as it creates tarballs, is called inside both update_spec() (here) and impl_srpm() (line 259). This means that running separate xtasks like spec and package-srpm will cause the packaging process to run multiple times unnecessarily. Consider refactoring to call impl_package() once per high-level task and pass the resulting Package struct to the helper functions. This would improve efficiency and make the data flow clearer.

let srcpath = pkg.srcpath.file_name().unwrap();
let v = pkg.version;
let src_vendorpath = pkg.vendorpath.file_name().unwrap();
{
let specin = File::open(format!("contrib/packaging/{NAME}.spec"))
.map(BufReader::new)
.context("Opening spec")?;
let mut o = File::create(p.join(format!("{NAME}.spec"))).map(BufWriter::new)?;
for line in specin.lines() {
let line = line?;
if line.starts_with("Version:") {
writeln!(o, "# Replaced by cargo xtask spec")?;
writeln!(o, "Version: {v}")?;
} else if line.starts_with("Source0") {
writeln!(o, "Source0: {srcpath}")?;
} else if line.starts_with("Source1") {
writeln!(o, "Source1: {src_vendorpath}")?;
} else {
writeln!(o, "{line}")?;
}
}
}
let spec_path = p.join(format!("{NAME}.spec"));
Ok(spec_path)
}

// Compress the final source archive
cmd!(sh, "zstd {temp_tar} -f -o {source_archive}").run()?;
fn spec(sh: &Shell) -> Result<()> {
let s = update_spec(sh)?;
println!("Generated: {s}");
Ok(())
}

println!("Created source archive: {}", source_archive);
fn impl_srpm(sh: &Shell) -> Result<camino::Utf8PathBuf> {
use camino::Utf8Path;
use std::fs::File;
use std::io::{BufRead, BufReader, BufWriter, Write};
use xshell::cmd;

println!("Release archives created successfully:");
println!(" Source: {}", source_archive);
println!(" Vendor: {}", vendor_archive);
{
let _g = sh.push_dir("target");
for name in sh.read_dir(".")? {
if let Some(name) = name.to_str() {
if name.ends_with(".src.rpm") {
sh.remove_path(name)?;
}
}
}
}
let pkg = impl_package(sh)?;
let td = tempfile::tempdir_in("target").context("Allocating tmpdir")?;
let td = td.keep();
let td: &Utf8Path = td.as_path().try_into().unwrap();

Choose a reason for hiding this comment

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

medium

The use of unwrap() here can cause a panic if the temporary directory path created by tempfile is not valid UTF-8. While unlikely on most systems, it's safer to handle this potential error explicitly by using ? and providing an error context.

Suggested change
let td: &Utf8Path = td.as_path().try_into().unwrap();
let td: &Utf8Path = td.as_path().try_into().context("temporary directory path is not valid UTF-8")?;

let srcpath = &pkg.srcpath;
cmd!(sh, "mv {srcpath} {td}").run()?;
let v = pkg.version;
let src_vendorpath = &pkg.vendorpath;
cmd!(sh, "mv {src_vendorpath} {td}").run()?;
{
let specin = File::open(format!("contrib/packaging/{NAME}.spec"))
.map(BufReader::new)
.context("Opening spec")?;
let mut o = File::create(td.join(format!("{NAME}.spec"))).map(BufWriter::new)?;
for line in specin.lines() {
let line = line?;
if line.starts_with("Version:") {
writeln!(o, "# Replaced by cargo xtask package-srpm")?;
writeln!(o, "Version: {v}")?;
} else {
writeln!(o, "{line}")?;
}
}
}
Comment on lines +268 to +282

Choose a reason for hiding this comment

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

critical

The spec file generation logic within impl_srpm is incomplete. It only replaces the Version tag, but not Source0 and Source1. The template spec file contains URLs for these sources. Without replacing them with the local tarball filenames, rpmbuild will likely try to download them from the internet, which is not the intended behavior, or fail if it cannot resolve the macros. This logic should be updated to also replace Source0 and Source1 with the filenames from the pkg variable, similar to how update_spec does it.

    {
        let specin = File::open(format!("contrib/packaging/{NAME}.spec"))
            .map(BufReader::new)
            .context("Opening spec")?;
        let mut o = File::create(td.join(format!("{NAME}.spec"))).map(BufWriter::new)?;
        let srcpath_fname = pkg.srcpath.file_name().unwrap();
        let vendorpath_fname = pkg.vendorpath.file_name().unwrap();
        for line in specin.lines() {
            let line = line?;
            if line.starts_with("Version:") {
                writeln!(o, "# Replaced by cargo xtask package-srpm")?;
                writeln!(o, "Version: {v}")?;
            } else if line.starts_with("Source0") {
                writeln!(o, "Source0:        {}", srcpath_fname)?;
            } else if line.starts_with("Source1") {
                writeln!(o, "Source1:        {}", vendorpath_fname)?;
            } else {
                writeln!(o, "{line}")?;
            }
        }
    }

let d = sh.push_dir(td);
let mut cmd = cmd!(sh, "rpmbuild");
for k in [
"_sourcedir",
"_specdir",
"_builddir",
"_srcrpmdir",
"_rpmdir",
] {
cmd = cmd.arg("--define");
cmd = cmd.arg(format!("{k} {td}"));
}
cmd.arg("--define")
.arg(format!("_buildrootdir {td}/.build"))
.args(["-bs", &format!("{NAME}.spec")])
.run()?;
drop(d);
let mut srpm = None;
for e in std::fs::read_dir(td)? {
let e = e?;
let n = e.file_name();
let Some(n) = n.to_str() else {
continue;
};
if n.ends_with(".src.rpm") {
srpm = Some(Utf8Path::new(td).join(n));
break;
}
}
let srpm = srpm.ok_or_else(|| eyre!("Failed to find generated .src.rpm"))?;
let dest = Utf8Path::new("target").join(srpm.file_name().unwrap());
std::fs::rename(&srpm, &dest)?;
Ok(dest)
}

fn package_srpm(sh: &Shell) -> Result<()> {
let srpm = impl_srpm(sh)?;
println!("Generated: {srpm}");
Ok(())
}
Loading