From 646f32f70d9c1393ac424312bc81e6e99689b3e9 Mon Sep 17 00:00:00 2001 From: Vinnie Magro Date: Mon, 13 Jan 2025 08:46:10 -0800 Subject: [PATCH] [antlir2][rpm] remove unnecessary details from repomd.xml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Since these repos are materialized to local directories by buck2 as a part of image builds, `dnf` is perfectly happy without `revision`, `checksum`, `size`, `open-checksum` and `open-size`. Removing this is a small simplification, but it simplifies the snapshot process such that a repo with pre-built and stored xml blobs can have `repomd.xml` written directly by the buck2 rule without actually needing to checksum/inspect the xml files at all. Test Plan: ```name="Force usage of repo-built version" ❯ hg diff diff --git a/fbcode/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK b/fbcode/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK --- a/fbcode/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK +++ b/fbcode/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK @@ -1,8 +1,10 @@ load("//antlir/bzl:build_defs.bzl", "buck_sh_binary", "rust_binary") -load("//antlir/bzl:internal_external.bzl", "is_facebook") +# load("//antlir/bzl:internal_external.bzl", "is_facebook") oncall("antlir") +is_facebook = False + rust_binary( name = "makerepo.rc" if is_facebook else "makerepo", srcs = glob(["src/**/*.rs"]), ``` ```name="Unit tests" ❯ buck2 test fbcode//antlir/antlir2/features/rpm/tests/... Buck UI: https://www.internalfb.com/buck2/e2cb2559-21c6-4488-a28d-9dff1f6a3d40 Test UI: https://www.internalfb.com/intern/testinfra/testrun/6755399687630739 Tests finished: Pass 104. Fail 0. Fatal 0. Skip 0. Build failure 0 ``` Reviewed By: epilatow Differential Revision: D67994895 fbshipit-source-id: 41245ad5d909505ced42aefd47623f4a77fc0a8f --- antlir/antlir2/features/rpm/tests/repo/BUCK | 1 - antlir/antlir2/features/rpm/tests/sig/BUCK | 1 - .../package_managers/dnf/rules/makerepo/BUCK | 2 - .../dnf/rules/makerepo/src/makerepo.rs | 148 +++--------------- .../package_managers/dnf/rules/repo.bzl | 3 - antlir/antlir2/test_images/cfg/os/BUCK | 1 - 6 files changed, 21 insertions(+), 135 deletions(-) diff --git a/antlir/antlir2/features/rpm/tests/repo/BUCK b/antlir/antlir2/features/rpm/tests/repo/BUCK index 5697fffbc53..c6d7d8cc228 100644 --- a/antlir/antlir2/features/rpm/tests/repo/BUCK +++ b/antlir/antlir2/features/rpm/tests/repo/BUCK @@ -227,7 +227,6 @@ repo( name = "test-repo-impl", compress = "none", rpms = all_rpms, - timestamp = 0, visibility = [ "//antlir/antlir2/antlir2_depgraph/tests/...", "//antlir/antlir2/antlir2_facts/tests/...", diff --git a/antlir/antlir2/features/rpm/tests/sig/BUCK b/antlir/antlir2/features/rpm/tests/sig/BUCK index 9890c184b0e..66e4d5815dd 100644 --- a/antlir/antlir2/features/rpm/tests/sig/BUCK +++ b/antlir/antlir2/features/rpm/tests/sig/BUCK @@ -112,7 +112,6 @@ repo( ":signed-with-wrong-key", ":signed-sha512", ], - timestamp = 0, visibility = [ ], ) diff --git a/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK b/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK index 7568283e3b6..3d3075c92e4 100644 --- a/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK +++ b/antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK @@ -12,11 +12,9 @@ rust_binary( "anyhow", "clap", "flate2", - "hex", "quick-xml", "serde", "serde_json", - "sha2", ], ) diff --git a/antlir/antlir2/package_managers/dnf/rules/makerepo/src/makerepo.rs b/antlir/antlir2/package_managers/dnf/rules/makerepo/src/makerepo.rs index 2ee8d500a84..c3c00ec50d5 100644 --- a/antlir/antlir2/package_managers/dnf/rules/makerepo/src/makerepo.rs +++ b/antlir/antlir2/package_managers/dnf/rules/makerepo/src/makerepo.rs @@ -6,9 +6,7 @@ */ use std::fs::File; -use std::io::BufReader; use std::io::BufWriter; -use std::io::Read; use std::io::Write; use std::path::Path; use std::path::PathBuf; @@ -18,25 +16,19 @@ use anyhow::Context; use anyhow::Result; use clap::Parser; use clap::ValueEnum; -use digest::Digest; use flate2::write::GzEncoder; use flate2::GzBuilder; use quick_xml::events::BytesEnd; use quick_xml::events::BytesStart; -use quick_xml::events::BytesText; use quick_xml::events::Event; use quick_xml::Writer as XmlWriter; use serde::Deserialize; -use sha2::digest; -use sha2::Sha256; #[derive(Debug, Parser)] struct Args { #[clap(long)] repo_id: String, #[clap(long)] - timestamp: Option, - #[clap(long)] xml_dir: PathBuf, #[clap(long, value_enum, default_value_t)] compress: Compress, @@ -55,39 +47,6 @@ enum Compress { Gzip, } -struct DigestWriter { - inner: W, - hasher: D, - len: u64, -} - -impl DigestWriter { - fn new(inner: W) -> Self { - Self { - inner, - hasher: D::new(), - len: 0, - } - } - - fn finish(self) -> (W, digest::Output, u64) { - (self.inner, self.hasher.finalize(), self.len) - } -} - -impl Write for DigestWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let written = self.inner.write(buf)?; - self.hasher.update(&buf[..written]); - self.len += written as u64; - Ok(written) - } - - fn flush(&mut self) -> std::io::Result<()> { - self.inner.flush() - } -} - #[derive(Debug, Deserialize)] struct PackageXmlBlobs { primary: String, @@ -95,18 +54,18 @@ struct PackageXmlBlobs { other: String, } -struct XmlFile { +struct XmlFile { filename: String, element: &'static str, - inner: XmlFileInner, + inner: XmlFileInner, } -enum XmlFileInner { - Gzipped(DigestWriter>, D>), - Uncompressed(DigestWriter), +enum XmlFileInner { + Gzipped(GzEncoder), + Uncompressed(W), } -impl XmlFile, Sha256> { +impl XmlFile> { fn new( basename: &str, num_packages: usize, @@ -122,12 +81,12 @@ impl XmlFile, Sha256> { File::create(&path).with_context(|| format!("while creating {}", path.display()))?; let w = BufWriter::new(f); let mut inner = match compress { - Compress::None => XmlFileInner::Uncompressed(DigestWriter::new(w)), - Compress::Gzip => XmlFileInner::Gzipped(DigestWriter::new( + Compress::None => XmlFileInner::Uncompressed(w), + Compress::Gzip => XmlFileInner::Gzipped( GzBuilder::new() .mtime(0) // deterministic output - .write(DigestWriter::new(w), flate2::Compression::default()), - )), + .write(w, flate2::Compression::default()), + ), }; inner.write_all(b"\n")?; let element = match basename { @@ -163,7 +122,7 @@ impl XmlFile, Sha256> { } } -impl XmlFile { +impl XmlFile { fn write_package(&mut self, package: &str) -> std::io::Result<()> { match &mut self.inner { XmlFileInner::Gzipped(w) => w.write_all(package.as_bytes()), @@ -177,37 +136,19 @@ impl XmlFile { let inner = xml.into_inner(); match inner { XmlFileInner::Gzipped(w) => { - // The outer layer (and the first finish()) is the uncompressed - // stream, so contains open-size and open-checksum - let (w, open_checksum, open_len) = w.finish(); - // The next layer is the GzEncoder, which when finish()ed will - // give us back the bottom DigestWriter that has the checksum - // and size of the compressed data - let (_, compressed_checksum, compressed_len) = - w.finish().context("while finishing compression")?.finish(); + w.finish()?; Ok(RepomdRecord { location: format!("repodata/{}", self.filename), - checksum: hex::encode(compressed_checksum), - size: compressed_len, - open_checksum: Some(hex::encode(open_checksum)), - open_size: Some(open_len), - }) - } - XmlFileInner::Uncompressed(w) => { - let (_, checksum, len) = w.finish(); - Ok(RepomdRecord { - location: format!("repodata/{}", self.filename), - checksum: hex::encode(checksum), - size: len, - open_checksum: None, - open_size: None, }) } + XmlFileInner::Uncompressed(_) => Ok(RepomdRecord { + location: format!("repodata/{}", self.filename), + }), } } } -impl Write for XmlFileInner { +impl Write for XmlFileInner { fn write(&mut self, buf: &[u8]) -> std::io::Result { match self { Self::Gzipped(w) => w.write(buf), @@ -225,33 +166,13 @@ impl Write for XmlFileInner { struct RepomdRecord { location: String, - checksum: String, - open_checksum: Option, - size: u64, - open_size: Option, } impl RepomdRecord { - fn write(&self, w: &mut XmlWriter, timestamp: u64) -> quick_xml::Result<()> { - w.create_element("checksum") - .with_attribute(("type", "sha256")) - .write_text_content(BytesText::from_plain_str(&self.checksum))?; - if let Some(open_checksum) = &self.open_checksum { - w.create_element("open-checksum") - .with_attribute(("type", "sha256")) - .write_text_content(BytesText::from_plain_str(open_checksum))?; - } + fn write(&self, w: &mut XmlWriter) -> quick_xml::Result<()> { w.create_element("location") .with_attribute(("href", self.location.as_str())) .write_empty()?; - w.create_element("timestamp") - .write_text_content(BytesText::from_plain_str(×tamp.to_string()))?; - w.create_element("size") - .write_text_content(BytesText::from_plain_str(&self.size.to_string()))?; - if let Some(open_size) = self.open_size { - w.create_element("open-size") - .write_text_content(BytesText::from_plain_str(&open_size.to_string()))?; - } Ok(()) } } @@ -304,17 +225,6 @@ fn main() -> Result<()> { .join(modulemd.file_name().expect("must have filename")), ) .context("while copying modulemd")?; - let mut reader = BufReader::new(File::open(modulemd).context("while opening modulemd")?); - let mut hasher = Sha256::new(); - let mut buffer = [0; 4096]; - loop { - let count = reader.read(&mut buffer)?; - if count == 0 { - break; - } - hasher.update(&buffer[..count]); - } - let checksum = hex::encode(hasher.finalize()); Some(RepomdRecord { location: format!( "repodata/{}", @@ -324,25 +234,11 @@ fn main() -> Result<()> { .to_str() .expect("must be utf8") ), - checksum, - size: modulemd - .metadata() - .context("while statting modulemd")? - .len(), - open_checksum: None, - open_size: None, }) } else { None }; - let timestamp = args.timestamp.unwrap_or_else(|| { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .expect("no time travel pls") - .as_secs() - }); - let mut inner = BufWriter::new( File::create(args.out.join("repomd.xml")).context("while creating repomd.xml")?, ); @@ -353,21 +249,19 @@ fn main() -> Result<()> { .with_attribute(("xmlns", "http://linux.duke.edu/metadata/repo")) .with_attribute(("xmlns:rpm", "http://linux.duke.edu/metadata/rpm")) .write_inner_content(|w| { - w.create_element("revision") - .write_text_content(BytesText::from_plain_str(timestamp.to_string().as_str()))?; w.create_element("data") .with_attribute(("type", "primary")) - .write_inner_content(|w| primary.write(w, timestamp))?; + .write_inner_content(|w| primary.write(w))?; w.create_element("data") .with_attribute(("type", "filelists")) - .write_inner_content(|w| filelists.write(w, timestamp))?; + .write_inner_content(|w| filelists.write(w))?; w.create_element("data") .with_attribute(("type", "other")) - .write_inner_content(|w| other.write(w, timestamp))?; + .write_inner_content(|w| other.write(w))?; if let Some(modulemd) = &modulemd { w.create_element("data") .with_attribute(("type", "modules")) - .write_inner_content(|w| modulemd.write(w, timestamp))?; + .write_inner_content(|w| modulemd.write(w))?; } Ok(()) })?; diff --git a/antlir/antlir2/package_managers/dnf/rules/repo.bzl b/antlir/antlir2/package_managers/dnf/rules/repo.bzl index 66593c51bea..ec28e09bebf 100644 --- a/antlir/antlir2/package_managers/dnf/rules/repo.bzl +++ b/antlir/antlir2/package_managers/dnf/rules/repo.bzl @@ -26,8 +26,6 @@ def _impl(ctx: AnalysisContext) -> list[Provider]: xml_dir = ctx.actions.declare_output("xml", dir = True) ctx.actions.copied_dir(xml_dir, {rpm.nevra: rpm.xml for rpm in rpm_infos}) optional_args = [] - if ctx.attrs.timestamp != None: - optional_args += ["--timestamp={}".format(ctx.attrs.timestamp)] # First build a repodata directory that just contains repodata (this would # be suitable as a baseurl for dnf) @@ -130,7 +128,6 @@ repo_attrs = { doc = "base key in manifold", default = None, ), - "timestamp": attrs.option(attrs.int(doc = "repomd.xml revision"), default = None), } _repo = rule( diff --git a/antlir/antlir2/test_images/cfg/os/BUCK b/antlir/antlir2/test_images/cfg/os/BUCK index 087a6ddc088..0494ab43b01 100644 --- a/antlir/antlir2/test_images/cfg/os/BUCK +++ b/antlir/antlir2/test_images/cfg/os/BUCK @@ -198,7 +198,6 @@ repo( name = "repo", compress = "none", rpms = all_rpms, - timestamp = 0, visibility = [ ], )