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

core: Compare rpm file coloring within layered packages #3161

Merged
merged 5 commits into from
Oct 14, 2021

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Oct 6, 2021

Before, we were only comparing rpm file coloring between layered
packages and base packages. But we also need to compare layered packages
among themselves. A concrete example is unixODBC which isn't in e.g.
Fedora Silverblue, but which some packages pull in both the i686 and
x86_64 versions. In that case, we need to skip the checkout of files in
the i686 version which are also in the x86_64 one.

Closes: #1937

Prep for specializing the `TR_ADDED` branch further.
@jlebon
Copy link
Member Author

jlebon commented Oct 6, 2021

Running pre scripts...done
�[0m�[31merror: �[0mRunning %prein for libutempter: bwrap(/bin/sh): Child process killed by signal 1
libutempter.prein: bwrap: execvp /bin/sh: No such file or directory

Hmm, not sure what's going on there. Digging.
Edit: OK fixed!

This converts `files_added` from a `map{path -> color}` to a
`map{nevra -> map{path -> color}` and similarly for `files_skip_add`.
Prep for next patch which makes use of that extra information.
@jlebon jlebon force-pushed the pr/unixODBC-issue-1937 branch from d0f75c0 to efaa0bc Compare October 6, 2021 19:52
lucab
lucab previously approved these changes Oct 7, 2021
Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM, though my knowledge of librpm API/usage is quite shallow.

cgwalters
cgwalters previously approved these changes Oct 7, 2021
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Some optional bits, feel free to merge as is. Thanks for doing this!

jlebon added 3 commits October 8, 2021 16:23
Before, we were only comparing rpm file coloring between layered
packages and base packages. But we also need to compare layered packages
among themselves. A concrete example is `unixODBC` which isn't in e.g.
Fedora Silverblue, but which some packages pull in both the i686 and
x86_64 versions. In that case, we need to skip the checkout of files in
the i686 version which are also in the x86_64 one.

Closes: coreos#1937
This matches the pattern we use for `files_skip_delete` and makes it
clear it won't be used later.
We only ever need to handle RPM file colors on architectures that have
them. And even on those that do, we only need to decide between
differently colored RPM files, and not e.g. documentation files. So we
can skip adding those to the map of files added.
@jlebon jlebon dismissed stale reviews from cgwalters and lucab via 19ccb7d October 8, 2021 20:29
@jlebon jlebon force-pushed the pr/unixODBC-issue-1937 branch from efaa0bc to 19ccb7d Compare October 8, 2021 20:29
@jlebon
Copy link
Member Author

jlebon commented Oct 8, 2021

Updated for comments!

@Mershl
Copy link
Contributor

Mershl commented Oct 13, 2021

@cgwalters This fixes an install blocker for some Fedora packages (most noticably lutris (described in #1937)). Can this PR be included in a dot release of 2021.12?

EDIT: I'm a bit very confused. Just gave the old reproducer a try and the issue was no longer reproduciable (already with 2021.11). I have a guess why: May this issue only show up if steam (from rpm-fusion) and lutris are requested at the same time - or the underlying issue has been fixed before this PR?

Worked for:
rpm-ostree-2021.11-3.fc35.x86_64
&
lutris-0.5.8.3-7.fc35.x86_64

EDIT2: Issue #1937 appears on any update afterwards. Do not remember if this was originally the case. I'll wait for !3161 to land and test again.

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Nice!

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.

error: Checkout unixODBC-2.3.7-5.fc31.i686: Hardlinking
4 participants