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

Introduce packit support #6649

Closed
wants to merge 1 commit into from
Closed

Conversation

matejak
Copy link
Member

@matejak matejak commented Mar 1, 2021

Packit enables integration with downstream Fedora packaging, and potentially with packaging of derived systems.

The setup uses the Rawhide spec file to create the srpm, so the upstream project doesn't require any adjustments.

@matejak matejak added this to the 0.1.55 milestone Mar 1, 2021
@matejak matejak changed the title Introduced packit support. Introduce packit support Mar 1, 2021
Copy link

@TomasTomecek TomasTomecek left a comment

Choose a reason for hiding this comment

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

I can see the SRPM build failed due to infra problem - I'll investigate tomorrow what the problem was.

Comment on lines +6 to +9
# add or remove files that should be synced
synced_files:
- scap-security-guide.spec
- .packit.yaml

Choose a reason for hiding this comment

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

these are actually defaults, so you can drop these 4 lines if you want

downstream_package_name: scap-security-guide

actions:
post-upstream-clone: "wget https://src.fedoraproject.org/rpms/scap-security-guide/raw/rawhide/f/scap-security-guide.spec -O scap-security-guide.spec"

Choose a reason for hiding this comment

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

happy to see this! I hope that you don't change dependencies often so that you don't run into "I need to fix the spec in downstream in order to have passing builds in the upstream"

Copy link
Member Author

Choose a reason for hiding this comment

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

This won't be a problem here - required dependencies don't change often.

@JAORMX
Copy link
Contributor

JAORMX commented Mar 1, 2021

/retest

@TomasTomecek
Copy link

okay, I can see the problem now, we failed to pull image from docker.io due to quota; sorry about that

@TomasTomecek
Copy link

/packit build

@TomasTomecek
Copy link

I can see the RPM failed to be built due to cmake - is there some special way to create archives or doing git archive is good enough?

@jan-cerny
Copy link
Collaborator

@TomasTomecek Only the F32 build fails. For some reason it tries to use the Rawhide spec file on F32. Do you have any ideas why it didn't use the F32 spec file on F32?

@jan-cerny
Copy link
Collaborator

Aha I can see what can be the problem, it's in the pull request description:

The setup uses the Rawhide spec file to create the srpm, so the upstream project doesn't require any adjustments.

You can't use the Rawhide spec file to build RPM on F32 because we use CMake and the RPM macro %cmake behaves differently starting from F33.

If you are curious you can read details about this F33 change: https://fedoraproject.org/wiki/Changes/CMake_to_do_out-of-source_builds

@matejak Please change your "setup" so that a correct spec according to the Fedora version is used.

@matejak
Copy link
Member Author

matejak commented Mar 2, 2021

@matejak Please change your "setup" so that a correct spec according to the Fedora version is used.

That's not a bug, it's a feature - the point is to use the Rawhide spec file for everything. You are right that it doesn't work for Fedora 32, and it also doesn't work for RHEL8 due to the cmake build issue as well, but it will work for anything newer than Fedora 33 and derived.

@TomasTomecek could you please suggest the most elegant way how to disable Packit builds for F32?

@jan-cerny
Copy link
Collaborator

@matejak Please change your "setup" so that a correct spec according to the Fedora version is used.

That's not a bug, it's a feature - the point is to use the Rawhide spec file for everything. You are right that it doesn't work for Fedora 32, and it also doesn't work for RHEL8 due to the cmake build issue as well, but it will work for anything newer than Fedora 33 and derived.

I think that maybe you can add an if else construction that depend on the system version to the rawhide spec

@matejak
Copy link
Member Author

matejak commented Mar 2, 2021

I think that maybe you can add an if else construction that depend on the system version to the rawhide spec

I don't think that a product that will be EOLed in two months deserves to have an if statement in the rawhide spec file. The goal is to have the rawhide spec file as simple as possible as long as it can be used on RHEL, CentOS Stream, and Fedora at the same time. If there is a conflict between simplicity and product support, one has to pick sides, and F32 alone is not a convincing side.

@TomasTomecek
Copy link

TomasTomecek commented Mar 2, 2021

The easiest solution is to configure jobs and precisely pick chroots:

jobs:
  - job: copr_build
    trigger: pull_request
    metadata:
      targets:
        - fedora-rawhide
        - fedora-34
        - fedora-33
  - job: tests
    trigger: pull_request
    metadata:
      targets:
        - fedora-rawhide
        - fedora-34
        - fedora-33

we also have aliases for development and stable versions of Fedora Linux: https://packit.dev/docs/configuration/#available-copr-build-targets

Let me know if this is acceptable for you.

.packit.yaml Outdated
post-upstream-clone: "wget https://src.fedoraproject.org/rpms/scap-security-guide/raw/rawhide/f/scap-security-guide.spec -O scap-security-guide.spec"


# TODO: Remove the "jobs" key as soon as F34 is out
Copy link
Collaborator

Choose a reason for hiding this comment

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

My understanding was that this key disables F32 builds so I would say that it should be disabled after F32 is removed from the infrastructure. Am I correct or are you correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, is F33 under fedora-development?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that both of us are correct - when F34 is released, F32 will disappear, and we can return to default jobs.

F33 is clearly not a development version, as you can see when you check out test results. I have tried to avoid mentioning explicit version numbers, so things work even if nobody removes the jobs key.

@vojtapolasek vojtapolasek modified the milestones: 0.1.55, 0.1.56 Mar 8, 2021
@vojtapolasek vojtapolasek modified the milestones: 0.1.56, 0.1.57 May 11, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jun 21, 2021

@matejak: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-ocp4-e8 9b9eaee link /test e2e-aws-ocp4-e8
ci/prow/e2e-aws-ocp4-cis 9b9eaee link /test e2e-aws-ocp4-cis
ci/prow/e2e-aws-rhcos4-e8 9b9eaee link /test e2e-aws-rhcos4-e8
ci/prow/e2e-aws-ocp4-cis-node 9b9eaee link /test e2e-aws-ocp4-cis-node
ci/prow/e2e-aws-rhcos4-moderate 9b9eaee link /test e2e-aws-rhcos4-moderate
ci/prow/e2e-aws-ocp4-moderate-node 9b9eaee link /test e2e-aws-ocp4-moderate-node

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@matejak matejak modified the milestones: 0.1.57, 0.1.58 Aug 17, 2021
@jan-cerny
Copy link
Collaborator

@matejak Any updates?

@yuumasato yuumasato modified the milestones: 0.1.58, 0.1.59 Sep 15, 2021
@matejak
Copy link
Member Author

matejak commented Sep 23, 2021

I would say that it simply works, so we can merge it. If we wait until the F34 release, we can simplify the config even further.

@jan-cerny
Copy link
Collaborator

@matejak Fedora 34 was released in April

The setup uses the Rawhide spec file to create an srpm.
@matejak
Copy link
Member Author

matejak commented Sep 23, 2021

@matejak Fedora 34 was released in April

Time flies, it's actually really F35 which is around the corner

@matejak
Copy link
Member Author

matejak commented Sep 23, 2021

And it doesn't work because of patches in the Fedora spec file, so let's close the PR.

@matejak matejak closed this Sep 23, 2021
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

6 participants