-
Notifications
You must be signed in to change notification settings - Fork 781
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
packit: Build PRs into default packit COPRs #2073
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Building all PRs of all container projects into the same COPR does not properly isolate PRs from each other. To avoid that, change the copr_build configuration to use the packit default COPRs, which are specific to the particular PR, and disappear after a few weeks. Depending projects should only run against what landed in skopeo/main i.e. the podman-next COPR. Signed-off-by: Martin Pitt <[email protected]>
ELN build failed with
The problem is that the podman-next COPR does not have a build of golang-github-cpuguy83-md2man for ELN. @lsm5 Is that something that you can fix on the COPR side, or should I take out the ELN builds here for the time being? Note that I most probably won't be able to finish this as my PTO starts pretty much "now". But you can force-push over my branch/PR of course. |
Sure, no worries. Enjoy PTO. I'll try to force-push or create a new PR and list you as co-author. Podman recently switched to using a vendored go-md2man because of this issue, so that's why we didn't see this in podman's copr switch. skopeo doesn't seem to vendor go-md2man yet, so I could either look at vendoring it, or switch to pandoc to build manpages, based on what @mtrmac and @vrothberg are ok with. |
I really don’t have an opinion. Aesthetically, on every Skopeo build having to build a tool that is available as a pre-built RPM seems wasteful to me, OTOH if it is not available, that argument falls apart. (Or would we continue to use an externally-built one in upstream and CI, but a vendored one in RPM?) As for go-md2man vs. Pandoc, I have no opinion at all. go-md2man was used from the very start of the project, and I never had a reason to look for alternatives, so I have no idea how the two compare. IIRC go-md2man has always somehow been available in EL as an RPM, and that includes RHEL 8. I wouldn’t think a RPM would be dropped without replacement during a single major version of the product. Is it possible that this could be resolved just by enabling some repo? (I didn’t check.) |
You need to enable a Fedora ELN build for the golang-github-cpuguy83-md2man package in the podman-next COPR. ELN is already enabled in podman-next in general, but either that was done after that build, or the build restricts its architectures to exclude Fedora ELN somehow. Or of course, not build skopeo in ELN, that's also an option. |
closing in favour of #2084 . There was an additional commit added to fix an eln target. |
Building all PRs of all container projects into the same COPR does not properly isolate PRs from each other.
To avoid that, change the copr_build configuration to use the packit default COPRs, which are specific to the particular PR, and disappear after a few weeks. Depending projects should only run against what landed in skopeo/main i.e. the podman-next COPR.
This is exactly the same as containers/crun#1260 ; please see the discussion there, this change needs to be applied to all container projects, and then all land together. Let's keep all the relevant discussions and tracking in the crun PR please.