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

installation.md: simplify build-from-source for rpm distros #174

Merged
merged 1 commit into from
Aug 25, 2023
Merged

installation.md: simplify build-from-source for rpm distros #174

merged 1 commit into from
Aug 25, 2023

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Jul 12, 2023 •
edited
Loading

RPM distros can simply run dnf builddep on rpm/podman.spec. That
specfile already handles dependencies for all supported versions of
Fedora and CentOS Stream so it's a lot simpler and easier to maintain
than duplicating dependency specification in the documentation.

Resolves: #157

@lsm5
Copy link
Member Author

lsm5 commented Jul 12, 2023

This should go in only after containers/podman#19214 is merged.

@lsm5 lsm5 force-pushed the makefile-rpm branch 3 times, most recently from 95fba4a to f2fffb1 Compare July 12, 2023 19:42
@lsm5
Copy link
Member Author

lsm5 commented Jul 12, 2023

hmm, git-archive-all would be a problem on centos and rhel. Guess I should just replace it with git archive...

@lsm5 lsm5 mentioned this pull request Jul 13, 2023
Closed
@lsm5 lsm5 marked this pull request as ready for review July 13, 2023 15:30
@lsm5
Copy link
Member Author

lsm5 commented Jul 13, 2023 •
edited
Loading

@TomSweeneyRedHat @Luap99 @rhatdan PTAL

@Ricky-Tigg the podman PR is merged. Please feel free to try the updated steps in this PR if you'd like.

@lsm5
Copy link
Member Author

lsm5 commented Jul 13, 2023

@cgiradkar PTAL

TomSweeneyRedHat
TomSweeneyRedHat reviewed Jul 13, 2023
View reviewed changes
docs/installation.md Show resolved Hide resolved
docs/installation.md Show resolved Hide resolved
@lsm5
Copy link
Member Author

lsm5 commented Jul 13, 2023

err ... looking at the title Building from scratch I wonder if I should revise this PR to NOT mention the rpmbuild steps but only get build and install dependencies. Sorry I just lost track.

@lsm5
Copy link
Member Author

lsm5 commented Jul 13, 2023

do we expect the user to want to run make or will the user be cool with make rpm ? I'm happy to go either way.

@lsm5
Copy link
Member Author

lsm5 commented Jul 13, 2023

fetching install time dependencies directly from the spec file and avoiding rpmbuild will likely be not so trivial, and might also end up needing frequent maintenance. Just that dnf install $RPM very easily handles all install dependencies.

@TomSweeneyRedHat
Copy link
Member

I think most folks will be looking to do 'make'. Instead of "Building from Scratch", I'd be OK with "Building from Source" if you think that's better.

@Luap99
Copy link
Member

Luap99 commented Jul 14, 2023

If I just look at the current list I see several packages that are missing and/or should not be there. Thus I like the dnf builddep and dnf install podman because well we know that the rpm spec is always correct otherwise it would not build.

But then again we have the same problem on debian/ubuntu so maybe we should allocate someone to keep this in sync.

Also the normal make commands are listed under ### Get Source Code, this section here should really only list deps to be consistent with the other distros.

@lsm5
Copy link
Member Author

lsm5 commented Jul 14, 2023 •
edited
Loading

ack, I'll revise it this way then:

# build deps
dnf -y builddep rpm/podman.spec

# runtime deps
dnf -y install catatonit conmon containers-common-extra

I can leave a note in rpm/podman.spec saying if any dependency needs to be updated then podman.io building from source should be updated as well.

@lsm5
Copy link
Member Author

lsm5 commented Jul 14, 2023 •
edited
Loading

So, go-md2man is not shipped on rhel8 / c8s. I could provide a copr to install it but that might bring in some more ugliness by polluting the user's env.

Are we ok with limiting the RHEL steps to rhel 9 / c9s ?

I have separated Fedora out from RHEL and CentOS Stream already as containers-common-extra is only on Fedora and RHEL/centos needs additional repos enabled.

@lsm5
Copy link
Member Author

lsm5 commented Jul 17, 2023

So, go-md2man is not shipped on rhel8 / c8s. I could provide a copr to install it but that might bring in some more ugliness by polluting the user's env.

Are we ok with limiting the RHEL steps to rhel 9 / c9s ?

I have separated Fedora out from RHEL and CentOS Stream already as containers-common-extra is only on Fedora and RHEL/centos needs additional repos enabled.

@Luap99 @TomSweeneyRedHat ping, wdyt ?

@Luap99
Copy link
Member

Luap99 commented Jul 17, 2023

Well the current instructions (listed packages) already do not work for RHEL so this would not be a new problem.

I would love to have some basic instruction on how to build on RHEL 8 as I had to test some of my patches there in the past and it was difficult to figure out how to get the right deps. However I won't block over it. If RHEL 9 works than I see this already as good improvement.

@TomSweeneyRedHat
Copy link
Member

@lsm5 I'd say no copr. I'm OK with providing instructions for RHEL9 only, but maybe we can add "try at your own risk" kind of verbiage for RHEL 8.

@lsm5
Copy link
Member Author

lsm5 commented Jul 18, 2023

ack, I'll do Fedora, RHEL 9, CentOS 9, followed by RHEL 8, CentOS 8, so 3 sections.

@TomSweeneyRedHat
Copy link
Member

just a quick friendly @lsm5 ping to put this back on the radar

@lsm5
Copy link
Member Author

lsm5 commented Aug 17, 2023

just a quick friendly @lsm5 ping to put this back on the radar

oops, yup. Let me get on this one. The go-md2man issue has gone away so things are a little simpler now.

@lsm5 lsm5 marked this pull request as draft August 17, 2023 13:22
lsm5 added a commit to lsm5/podman that referenced this pull request Aug 17, 2023
@lsm5
[CI:BUILD] rpm: depend on man-db
50136ea 
Include dependencies in rpm/podman.spec to make it easy for end users to
build podman from source. This way users can install all build
dependencies by running `dnf -y builddep rpm/podman.spec`.

This is not usually noticeable except on container environments where
man-db often isn't installed by default.

Refs:
containers/podman.io#157
containers/podman.io#174

[NO NEW TESTS NEEDED]

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5 lsm5 mentioned this pull request Aug 17, 2023
Merged
@lsm5
Copy link
Member Author

lsm5 commented Aug 17, 2023

blocking on containers/podman#19670

@lsm5
Copy link
Member Author

lsm5 commented Aug 17, 2023

Updated PR. Have separate instructions for CentOS Stream, RHEL 8 and 9. RE: RHEL 8 and 9, I have yet to verify that on RHEL machines. The repos on rhel containers don't find the shadow-utils-subid-devel and ostree-devel packages. I'll get back to this Monday.

openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/podman that referenced this pull request Aug 21, 2023
@lsm5
rpm: depend on man-db
2fe604a 
Include dependencies in rpm/podman.spec to make it easy for end users to
build podman from source. This way users can install all build
dependencies by running `dnf -y builddep rpm/podman.spec`.

This is not usually noticeable except on container environments where
man-db often isn't installed by default.

Refs:
containers/podman.io#157
containers/podman.io#174

[NO NEW TESTS NEEDED]

Signed-off-by: Lokesh Mandvekar <[email protected]>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/podman that referenced this pull request Aug 21, 2023
@lsm5
rpm: depend on man-db
2388d96 
Include dependencies in rpm/podman.spec to make it easy for end users to
build podman from source. This way users can install all build
dependencies by running `dnf -y builddep rpm/podman.spec`.

This is not usually noticeable except on container environments where
man-db often isn't installed by default.

Refs:
containers/podman.io#157
containers/podman.io#174

[NO NEW TESTS NEEDED]

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5 lsm5 marked this pull request as ready for review August 22, 2023 19:02
@lsm5
Copy link
Member Author

lsm5 commented Aug 22, 2023

Updated PR. Have separate instructions for CentOS Stream, RHEL 8 and 9. RE: RHEL 8 and 9, I have yet to verify that on RHEL machines. The repos on rhel containers don't find the shadow-utils-subid-devel and ostree-devel packages. I'll get back to this Monday.

tried this on an internal rhel instance and the CRB and appstream repos were able to find all required packages. I'm not sure if a public RHEL instance with developer subscription or such will have access to the exact same repos, but we can cross that bridge later.

@TomSweeneyRedHat @Luap99 Good for another review.

@TomSweeneyRedHat
Copy link
Member

One small suggestion for consideration, otherwise

LGTM

@lsm5
installation.md: simplify build-from-source for rpm distros
63fa0bd 
RPM distros can simply run `dnf builddep` on `rpm/podman.spec` to fetch
build dependencies. That specfile already handles dependencies
for all supported versions of Fedora and CentOS Stream so it's
a lot simpler and easier to maintain than duplicating dependency
specification in the documentation.

Resolves: containers#157

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5
Copy link
Member Author

lsm5 commented Aug 24, 2023

@TomSweeneyRedHat @Luap99 PTAL again. I simplified the additional repo enablement step by a lot. The builddep step for rhel should work with a simple copy paste.

@baude
Copy link
Member

baude commented Aug 25, 2023

lgtm

Luap99
Luap99 reviewed Aug 25, 2023
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99 Luap99 merged commit db32d27 into containers:main Aug 25, 2023
@TomSweeneyRedHat
Copy link
Member

@lsm5 very nice rework btb, TYVM!

@lsm5 lsm5 deleted the makefile-rpm branch August 25, 2023 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Luap99 Luap99 Luap99 left review comments

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Building from scratch | section "Fedora"
4 participants
@lsm5 @TomSweeneyRedHat @Luap99 @baude