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.

Sign up for GitHub

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

Jump to bottom

[CI:BUILD] copr: fix el8 builds #4439

Merged
merged 1 commit into from Feb 16, 2023
Merged

[CI:BUILD] copr: fix el8 builds #4439

merged 1 commit into from Feb 16, 2023

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Nov 29, 2022

Signed-off-by: Lokesh Mandvekar lsm5@fedoraproject.org

What type of PR is this?

/kind other

What this PR does / why we need it:

Fixes copr build

How to verify it

successful el8 build at: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/build/5072942/

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

None

@flouthoc @nalind @rhatdan PTAL

@lsm5
Copy link
Member Author

lsm5 commented Nov 29, 2022

older builds were failing for el8: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/build/5072893/

@nalind
Copy link
Member

nalind commented Nov 29, 2022 •
edited

Why doesn't the next %if 0%{?rhel} block not accomplish the same thing? (Aside: the use of single quotes in that next block appears to set BUILDTAGS to a value that literally includes "$BUILDTAGS", rather than expanding it.)
From the build log, it looks like $BUILDTAGS is either empty or not being referenced when go build is run, which looks like a bigger problem than the current patch would work around.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Dec 31, 2022

@lsm5 you still working on this?

@rhatdan rhatdan removed the stale-pr label Dec 31, 2022
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@lsm5 lsm5 removed the stale-pr label Jan 31, 2023
@lsm5
Copy link
Member Author

lsm5 commented Jan 31, 2023

@lsm5 you still working on this?

I'll resume this, been working on a similar one for skopeo.

@rhatdan
Copy link
Member

rhatdan commented Jan 31, 2023

LGTM
@flouthoc @nalind @giuseppe @vrothberg @ashley-cui @umohnani8 PTAL

@vrothberg
Copy link
Member

Why doesn't the next %if 0%{?rhel} block not accomplish the same thing? (Aside: the use of single quotes in that next block appears to set BUILDTAGS to a value that literally includes "$BUILDTAGS", rather than expanding it.) From the build log, it looks like $BUILDTAGS is either empty or not being referenced when go build is run, which looks like a bigger problem than the current patch would work around.

This looks still open

@lsm5
Copy link
Member Author

lsm5 commented Jan 31, 2023

err yes, I still gotta work on this :) . Let me try to get skopeo out of the way first. If we prefer closing this and reopening when I'm ready to update, fine by me.

@lsm5 lsm5 marked this pull request as draft January 31, 2023 14:43
@lsm5 lsm5 marked this pull request as ready for review February 1, 2023 16:20
@lsm5
Copy link
Member Author

lsm5 commented Feb 1, 2023

RHEL8's default %gobuild didn't account for $BUILDTAGS. Adjusted that in buildah.spec.rpkg . Successful build at https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/build/5390214/ .

@lsm5 lsm5 marked this pull request as draft February 2, 2023 10:20
@lsm5
Copy link
Member Author

lsm5 commented Feb 2, 2023

let me fix debuginfo in this PR itself.

@lsm5 lsm5 marked this pull request as ready for review February 2, 2023 10:59
@lsm5
Copy link
Member Author

lsm5 commented Feb 2, 2023

ready for review. successful build at https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/build/5406349/

flouthoc
flouthoc approved these changes Feb 2, 2023
View reviewed changes
Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM

@lsm5 lsm5 marked this pull request as draft February 2, 2023 12:56
@lsm5
Copy link
Member Author

lsm5 commented Feb 2, 2023

i'll update this PR further with making the spec as close to podman and buildah wherever possible.

@lsm5 lsm5 marked this pull request as ready for review February 15, 2023 14:40
@lsm5
Copy link
Member Author

lsm5 commented Feb 15, 2023

good to go. PTAL

@lsm5
[CI:BUILD] copr: fix el8 builds
f9e51e7 
Bump containers-common dependency to match with that in
podman.spec.rpkg.

qemu-user-static dependency is now handled in containers-common-extra
itself.

Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>
giuseppe
giuseppe approved these changes Feb 16, 2023
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 16, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, giuseppe, lsm5

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [flouthoc,giuseppe,lsm5]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 16, 2023
@openshift-merge-robot openshift-merge-robot merged commit f2ff894 into containers:main Feb 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@giuseppe giuseppe giuseppe approved these changes

@flouthoc flouthoc flouthoc approved these changes

Assignees

@TomSweeneyRedHat TomSweeneyRedHat

Labels
approved kind/other lgtm locked - please file new issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@lsm5 @nalind @rhatdan @vrothberg @TomSweeneyRedHat @giuseppe @flouthoc @openshift-merge-robot