Skip to content

Commit

Permalink
[antlir2][rpm] remove unnecessary details from repomd.xml
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vmagro authored and facebook-github-bot committed Jan 13, 2025
1 parent 9398343 commit 646f32f
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 135 deletions.
1 change: 0 additions & 1 deletion antlir/antlir2/features/rpm/tests/repo/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -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/...",
Expand Down
1 change: 0 additions & 1 deletion antlir/antlir2/features/rpm/tests/sig/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ repo(
":signed-with-wrong-key",
":signed-sha512",
],
timestamp = 0,
visibility = [
],
)
Expand Down
2 changes: 0 additions & 2 deletions antlir/antlir2/package_managers/dnf/rules/makerepo/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ rust_binary(
"anyhow",
"clap",
"flate2",
"hex",
"quick-xml",
"serde",
"serde_json",
"sha2",
],
)

Expand Down
148 changes: 21 additions & 127 deletions antlir/antlir2/package_managers/dnf/rules/makerepo/src/makerepo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<u64>,
#[clap(long)]
xml_dir: PathBuf,
#[clap(long, value_enum, default_value_t)]
compress: Compress,
Expand All @@ -55,58 +47,25 @@ enum Compress {
Gzip,
}

struct DigestWriter<W: Write, D: Digest> {
inner: W,
hasher: D,
len: u64,
}

impl<W: Write, D: Digest> DigestWriter<W, D> {
fn new(inner: W) -> Self {
Self {
inner,
hasher: D::new(),
len: 0,
}
}

fn finish(self) -> (W, digest::Output<D>, u64) {
(self.inner, self.hasher.finalize(), self.len)
}
}

impl<W: Write, D: Digest> Write for DigestWriter<W, D> {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
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,
filelists: String,
other: String,
}

struct XmlFile<W: Write, D: Digest> {
struct XmlFile<W: Write> {
filename: String,
element: &'static str,
inner: XmlFileInner<W, D>,
inner: XmlFileInner<W>,
}

enum XmlFileInner<W: Write, D: Digest> {
Gzipped(DigestWriter<GzEncoder<DigestWriter<W, D>>, D>),
Uncompressed(DigestWriter<W, D>),
enum XmlFileInner<W: Write> {
Gzipped(GzEncoder<W>),
Uncompressed(W),
}

impl XmlFile<BufWriter<File>, Sha256> {
impl XmlFile<BufWriter<File>> {
fn new(
basename: &str,
num_packages: usize,
Expand All @@ -122,12 +81,12 @@ impl XmlFile<BufWriter<File>, 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"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")?;
let element = match basename {
Expand Down Expand Up @@ -163,7 +122,7 @@ impl XmlFile<BufWriter<File>, Sha256> {
}
}

impl<W: Write, D: Digest> XmlFile<W, D> {
impl<W: Write> XmlFile<W> {
fn write_package(&mut self, package: &str) -> std::io::Result<()> {
match &mut self.inner {
XmlFileInner::Gzipped(w) => w.write_all(package.as_bytes()),
Expand All @@ -177,37 +136,19 @@ impl<W: Write, D: Digest> XmlFile<W, D> {
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<W: Write, D: Digest> Write for XmlFileInner<W, D> {
impl<W: Write> Write for XmlFileInner<W> {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
match self {
Self::Gzipped(w) => w.write(buf),
Expand All @@ -225,33 +166,13 @@ impl<W: Write, D: Digest> Write for XmlFileInner<W, D> {

struct RepomdRecord {
location: String,
checksum: String,
open_checksum: Option<String>,
size: u64,
open_size: Option<u64>,
}

impl RepomdRecord {
fn write<W: Write>(&self, w: &mut XmlWriter<W>, 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<W: Write>(&self, w: &mut XmlWriter<W>) -> 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(&timestamp.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(())
}
}
Expand Down Expand Up @@ -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/{}",
Expand All @@ -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")?,
);
Expand All @@ -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(())
})?;
Expand Down
3 changes: 0 additions & 3 deletions antlir/antlir2/package_managers/dnf/rules/repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
1 change: 0 additions & 1 deletion antlir/antlir2/test_images/cfg/os/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ repo(
name = "repo",
compress = "none",
rpms = all_rpms,
timestamp = 0,
visibility = [
],
)

0 comments on commit 646f32f

Please sign in to comment.