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

cmdlib: use repo-packages for overrides/rpm #2954

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jun 28, 2022

In addition to a lockfile, we can be even stronger here by using
repo-packages to tell rpm-ostree that the overridden packages must come
from the generated overrides repo.

This also allows one to override RPMs which themselves are specified as
repo-packages in the manifests without in most cases having to edit
them.

Related: coreos/rpm-ostree#3789

In addition to a lockfile, we can be even stronger here by using
repo-packages to tell rpm-ostree that the overridden packages must come
from the generated overrides repo.

This also allows one to override RPMs which themselves are specified as
`repo-packages` in the manifests without in most cases having to edit
them.

Related: coreos/rpm-ostree#3789
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.

Seems sane to me, though it's worth noting that (AFAIK) CI doesn't cover any of the "local dev" flows like adding to overrides/rpm.

}
]
}
for line in sys.stdin:
Copy link
Member

Choose a reason for hiding this comment

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

I know this isn't a new issue, but hopefully in the future we can drop this python-in-shell-script in favor of Go

@jlebon
Copy link
Member Author

jlebon commented Jun 28, 2022

Indeed re. CI (though rpm-ostree's CI at least uses overrides/rpm, but we don't use repo pinning in FCOS). I did test it locally with RHCOS and a kernel override, which I think is what motivated the original issue:

$ ls -l overrides/rpm
total 92136
-rw-r--r--. 1 jlebon jlebon  8859320 Jun 28 12:52 kernel-4.18.0-405.el8.x86_64.rpm
-rw-r--r--. 1 jlebon jlebon 41828532 Jun 28 12:52 kernel-core-4.18.0-405.el8.x86_64.rpm
-rw-r--r--. 1 jlebon jlebon 33798412 Jun 28 12:53 kernel-modules-4.18.0-405.el8.x86_64.rpm
-rw-r--r--. 1 jlebon jlebon  9564576 Jun 28 12:53 kernel-modules-extra-4.18.0-405.el8.x86_64.rpm
drwxr-xr-x. 2 jlebon jlebon     4096 Jun 28 13:24 repodata
$ cosa build
...
Enabled rpm-md repositories: coreos-assembler-local-overrides rhel-8-server-ose rhel-8-fast-datapath rhel-8-baseos rhel-8-appstream
Importing rpm-md... done
rpm-md repo 'coreos-assembler-local-overrides' (cached); generated: 2022-06-28T19:39:48Z solvables: 5
rpm-md repo 'rhel-8-server-ose' (cached); generated: 2022-06-28T14:57:24Z solvables: 475
rpm-md repo 'rhel-8-fast-datapath' (cached); generated: 2022-05-27T18:05:26Z solvables: 793
rpm-md repo 'rhel-8-baseos' (cached); generated: 2022-06-28T12:31:21Z solvables: 12476
rpm-md repo 'rhel-8-appstream' (cached); generated: 2022-06-28T16:34:38Z solvables: 27785
Resolving dependencies... done
Installing 500 packages:
  ...
  kernel-4.18.0-405.el8.x86_64 (coreos-assembler-local-overrides)
  kernel-core-4.18.0-405.el8.x86_64 (coreos-assembler-local-overrides)
  kernel-modules-4.18.0-405.el8.x86_64 (coreos-assembler-local-overrides)
  kernel-modules-extra-4.18.0-405.el8.x86_64 (coreos-assembler-local-overrides)
  ...

@jlebon jlebon enabled auto-merge (rebase) June 28, 2022 19:42
@cgwalters
Copy link
Member

Yep I assumed you did, I personally am totally fine with local-only testing for now for this type of stuff.

@jlebon jlebon merged commit 8c8b733 into coreos:main Jun 28, 2022
cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request Jun 29, 2022
The recent changes to coreos-assembler in
coreos/coreos-assembler#2954
broke this, and I think what we were trying to do here should
be fixed.
cgwalters added a commit to coreos/rpm-ostree that referenced this pull request Jun 29, 2022
The recent changes to coreos-assembler in
coreos/coreos-assembler#2954
broke this, and I think what we were trying to do here should
be fixed.
@jlebon
Copy link
Member Author

jlebon commented Jul 7, 2022

As mentioned in coreos/rpm-ostree#3802, this actually changes the semantics of overrides/rpm so that now all RPMs in there try to be installed. Before with the lockfile in non-strict mode, it would only be a "preferred" source of packages (i.e. if foo happened to get pulled in, then any foo in overrides/rpm has priority).

So this breaks workflows where people just dump the output of rpmbuild or e.g. koji download-build in overrides/rpm which also includes e.g. debuginfo and devel packages.

I think I actually prefer the new semantic because it's conceptually cleaner and easier to understand, but I'm cool with reverting it if it's too confusing.

@dustymabe
Copy link
Member

As mentioned in coreos/rpm-ostree#3802, this actually changes the semantics of overrides/rpm so that now all RPMs in there try to be installed. Before with the lockfile in non-strict mode, it would only be a "preferred" source of packages (i.e. if foo happened to get pulled in, then any foo in overrides/rpm has priority).

So this breaks workflows where people just dump the output of rpmbuild or e.g. koji download-build in overrides/rpm which also includes e.g. debuginfo and devel packages.

I think I actually prefer the new semantic because it's conceptually cleaner and easier to understand, but I'm cool with reverting it if it's too confusing.

Ehh. I'm not a big fan of the new semantic. For example I'm trying to test a development SELinux build. I download the rpms.zip file and extract it to the overrides/rpm dir. In the past this was all I had to do. Now I get an error:

Resolving dependencies... done
error: Could not depsolve transaction; 2 problems detected:
 Problem 1: package selinux-policy-devel-37.6-1.20220708_074447.51fddf5.fc37.noarch requires policycoreutils-devel >= 3.4-1, but none of the providers can be installed
  - package policycoreutils-devel-3.4-3.fc37.i686 requires /usr/bin/python3, but none of the providers can be installed
  - package policycoreutils-devel-3.4-3.fc37.x86_64 requires /usr/bin/python3, but none of the providers can be installed
  - conflicting requests
  - package python3-3.11.0~b3-6.fc37.i686 is filtered out by exclude filtering
  - package python3-3.11.0~b3-6.fc37.x86_64 is filtered out by exclude filtering
 Problem 2: package selinux-policy-minimum-37.6-1.20220708_074447.51fddf5.fc37.noarch requires policycoreutils-python-utils >= 3.4-1, but none of the providers can be installed
  - package policycoreutils-python-utils-3.4-3.fc37.noarch requires /usr/bin/python3, but none of the providers can be installed
  - conflicting requests
  - package python3-3.11.0~b3-6.fc37.i686 is filtered out by exclude filtering
  - package python3-3.11.0~b3-6.fc37.x86_64 is filtered out by exclude filtering

But it's not very obvious to me what's going on. I need to manually inspect the downloaded rpms and filter through them to find the ones I don't want.

What problem was this solving? Could we solve it a different way or add a new "mode" for the new behavior?

@jlebon
Copy link
Member Author

jlebon commented Jul 8, 2022

What problem was this solving?

That's in the commit message. :)

Could we solve it a different way or add a new "mode" for the new behavior?

Yup, follow-up in #2969!

@jlebon jlebon deleted the pr/override-manifest branch April 22, 2023 23:35
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.

None yet

3 participants