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

ostree-ext/tar/export: Fix reexport of var/tmp #1025

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

cgwalters
Copy link
Collaborator

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.

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 <walters@verbum.org>
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters merged commit 0767f7d into containers:main Jan 15, 2025
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants