Skip to content

Commit

Permalink
ostree-ext/tar/export: Fix reexport of var/tmp
Browse files Browse the repository at this point in the history
A lot of technical debt here. A long time ago I added this hacky bit
to inject var/tmp is the container stream even if it wasn't in the
ostree commit.

Today things shipped by `rpm-ostree compose image` like FCOS
don't have `var/tmp` in the commit.

But then more recently we started shipping `/var/tmp`
in base images directly.

Now I'm working on coreos/rpm-ostree#5221
where we're rechunking from a rootfs that does have var/tmp
and that ends up in the ostree commit.

The path comparison here was wrong because the tar stream we
generate has the paths start with `./` and a literal comparison
doesn't match `./var/tmp` != `var/tmp`.

Add a canonicalization helper and use it for this.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
cgwalters committed Jan 14, 2025
1 parent 8dff320 commit f187208
Showing 1 changed file with 25 additions and 1 deletion.
26 changes: 25 additions & 1 deletion ostree-ext/src/tar/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ fn map_path_v1(p: &Utf8Path) -> &Utf8Path {
}
}

/// Given two paths, which may be absolute (starting with /) or
/// start with `./`, return true if they are equal after removing
/// those prefixes. This is effectively "would these paths be equal"
/// when processed as a tar entry.
pub(crate) fn path_equivalent_for_tar(a: impl AsRef<Utf8Path>, b: impl AsRef<Utf8Path>) -> bool {
fn strip_prefix(p: &Utf8Path) -> &Utf8Path {
if let Ok(p) = p.strip_prefix("/") {
return p;
} else if let Ok(p) = p.strip_prefix("./") {
return p;
}
return p;
}
strip_prefix(a.as_ref()) == strip_prefix(b.as_ref())
}

struct OstreeTarWriter<'a, W: std::io::Write> {
repo: &'a ostree::Repo,
commit_checksum: &'a str,
Expand Down Expand Up @@ -496,7 +512,7 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {

// Record if the ostree commit includes /var/tmp; if so we don't need to synthesize
// it in `append_standard_var()`.
if dirpath == "var/tmp" {
if path_equivalent_for_tar(dirpath, "var/tmp") {
self.wrote_vartmp = true;
}

Expand Down Expand Up @@ -724,6 +740,14 @@ pub fn update_detached_metadata<D: std::io::Write, C: IsA<gio::Cancellable>>(
mod tests {
use super::*;

#[test]
fn test_path_equivalent() {
assert!(path_equivalent_for_tar("var/tmp", "./var/tmp"));
assert!(path_equivalent_for_tar("./var/tmp", "var/tmp"));
assert!(path_equivalent_for_tar("/var/tmp", "var/tmp"));
assert!(!path_equivalent_for_tar("var/tmp", "var"));
}

#[test]
fn test_map_path() {
assert_eq!(map_path("/".into()), Utf8Path::new("/"));
Expand Down

0 comments on commit f187208

Please sign in to comment.