[OS-BUILD PATCH 0/3] Implement better SRPM BuildRequires check

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
Merge Request: https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786

Developers want to build a srpm locally and submit it to a build server like
Koji.  The current process checks the BuildRequires in the kernel.spec file to
ensure all the right packages are there.  However, that list includes too many
packages.  Reduce the list to the bare minimum.

In addition cleanup the Makefile and utilize the original buildreqs-check
target better.

Signed-off-by: Don Zickus

---
 redhat/Makefile             |  33 ++++++++++++++++++++++++---------
 redhat/genspec.sh           |   1 +
 redhat/kernel.spec.template |   7 ++++---
 3 files changed, 29 insertions(+), 12 deletions(-)

[OS-BUILD PATCH 1/3] [redhat] Explicitly set srpm requirements

[Don Zickus (via Email Bridge)]

From: Don Zickus

[redhat] Explicitly set srpm requirements

It isn't clear what packages are necessary to be installed to generate
the srpm for the kernel.  Use another spec macro to call out the exact
packages needed. This allows a makefile target to be used to verify the
right packages have been installed.

Signed-off-by: Don Zickus

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -84,6 +84,7 @@ TESTPATCH:=$(REDHAT)/linux-kernel-test.patch
 SPECCHANGELOG:=$(PACKAGE_NAME).changelog-$(RHEL_MAJOR).$(RHEL_MINOR)
 CHANGELOG_PREV:=$(PACKAGE_NAME).changelog-$(RHEL_MAJOR).$(shell expr $(RHEL_MINOR) - 1)
 ARCH_LIST=aarch64 ppc64le s390x x86_64
+SPECSRPMBUILDREQS := git-core make gcc flex bison bzip2

ifndef DISTRO
   ifneq ($(findstring .fc,$(DIST)),)
@@ -391,7 +392,7 @@ rh-configs: dist-rhel-configs
 dist-configs-check: dist-configs-prep
    +cd $(REDHAT)/configs; ./process_configs.sh $(PROCESS_CONFIGS_CHECK_OPTS) "" ""

-dist-configs-prep: dist-clean-configs dist-buildreq-check
+dist-configs-prep: dist-clean-configs dist-srpmbuildreq-check
    +cd $(REDHAT)/configs; ./build_configs.sh "partial" "snip"
    +cd $(REDHAT)/configs; ./build_configs.sh "$(PACKAGE_NAME)" "$(FLAVOR)"

@@ -469,6 +470,17 @@ dist-git-version-check:
    	exit 1; \
    fi

+dist-srpmbuildreq-check: setup-source
+	@PKGLIST="rpm-build $(SPECSRPMBUILDREQS)"; \
+	MISSING=""; \
+	for pkg in $$PKGLIST; do \
+		rpm -q --whatprovides $$pkg >/dev/null || MISSING="$$MISSING $$pkg"; \
+	done; \
+	if [ -n "$$MISSING" ]; then \
+		echo "Error: please 'dnf install $$MISSING'"; \
+		exit 1; \
+	fi
+
 dist-buildreq-check: setup-source
    @PKGLIST="rpm-build $$(rpmspec -q --buildrequires $(SOURCES)/$(SPECFILE) | cut -d ' ' -f 1)"; \
    MISSING=""; \
@@ -812,6 +824,7 @@ dist-full-help:

@echo  ''
    @echo  'Misc targets:'
+	@echo  '  dist-srpmbuildreq-check - Checks for presence of packages required for the srpm'
    @echo  '  dist-buildreq-check - Checks for presence of packages required for build'
    @echo  '  dist-get-latest - Returns the latest "known good" kernel from brew. This'
    @echo  '                    should not be confused with the latest top-of-tree'
diff --git a/redhat/genspec.sh b/redhat/genspec.sh
index blahblah..blahblah 100755
--- a/redhat/genspec.sh
+++ b/redhat/genspec.sh
@@ -70,6 +70,7 @@ test -f "$SOURCES/$SPECFILE" &&
    s/%%SPECINCLUDE_RHEL_FILES%%/$SPECINCLUDE_RHEL_FILES/
    s/%%SPECPATCHLIST_CHANGELOG%%/$SPECPATCHLIST_CHANGELOG/
    s/%%SPECVERSION%%/$SPECVERSION/
+	s/%%SPECSRPMBUILDREQS%%/$SPECSRPMBUILDREQS/
    s/%%SPECTARFILE_RELEASE%%/$SPECTARFILE_RELEASE/" "$SOURCES/$SPECFILE"
 test -n "$RHSELFTESTDATA" && test -f "$SOURCES/$SPECFILE" && sed -i -e "
    /%%SPECCHANGELOG%%/r $SOURCES/$SPECCHANGELOG
diff --git a/redhat/kernel.spec.template b/redhat/kernel.spec.template
index blahblah..blahblah 100755
--- a/redhat/kernel.spec.template
+++ b/redhat/kernel.spec.template
@@ -570,9 +570,10 @@ Requires: kernel-modules-uname-r = %{KVERREL}
 #
 # List the packages used during the kernel build
 #
-BuildRequires: kmod, patch, bash, coreutils, tar, git-core, which
-BuildRequires: bzip2, xz, findutils, gzip, m4, perl-interpreter, perl-Carp, perl-devel, perl-generators, make, diffutils, gawk
-BuildRequires: gcc, binutils, redhat-rpm-config, hmaccalc, bison, flex, gcc-c++
+BuildRequires: %%SPECSRPMBUILDREQS%%
+BuildRequires: kmod, patch, bash, coreutils, tar, which
+BuildRequires: xz, findutils, gzip, m4, perl-interpreter, perl-Carp, perl-devel, perl-generators, diffutils, gawk
+BuildRequires: binutils, redhat-rpm-config, hmaccalc, gcc-c++
 BuildRequires: net-tools, hostname, bc, elfutils-devel
 BuildRequires: dwarves
 BuildRequires: python3-devel

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786

[OS-BUILD PATCH 1/3] [redhat] Explicitly set srpm requirements

[Prarit Bhargava (via Email Bridge)]

From: Prarit Bhargava on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516403...

OOC what happens if the SRPM builds correctly, but something like
'dist-configs-check' fails because there are missing BuildRequires?

IOW, is SPECSRPMBUILDREQS really complete?

[OS-BUILD PATCH 1/3] [redhat] Explicitly set srpm requirements

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9517051...

I can't prove it mathematically, but in order to build the SRPM, you need to
build the configs.  What does dist-configs-check do that regular dist-configs
does not?  And if it is missing something, then we add it in there.

[OS-BUILD PATCH 2/3] [redhat] Makefile: drop duplicate dist-sources deps

[Don Zickus (via Email Bridge)]

From: Ondrej Mosnacek

[redhat] Makefile: drop duplicate dist-sources deps

do-rpmbuild itself already depends on dist-sources, thus no need to
specify when we depend on do-rpmbuild already.

Signed-off-by: Ondrej Mosnacek

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -574,29 +574,29 @@ do-rpmbuild: dist-sources
    $(RPMBUILD) --define "_sourcedir $(SOURCES)" --define "_builddir $(RPM)/BUILD" --define "_srcrpmdir $(RPM)/SRPMS" --define "_rpmdir $(RPM)/RPMS" --define "_specdir $(RPM)/SPECS" --define "dist $(DIST)" $(RPMBUILDOPTS) $(RPM)/SOURCES/$(PACKAGE_NAME).spec

dist-all-rpms: RPMBUILDOPTS=--target $(MACH) -ba
-dist-all-rpms: dist-sources do-rpmbuild
+dist-all-rpms: do-rpmbuild

dist-srpm: RPMBUILDOPTS=--nodeps -bs
-dist-srpm: dist-sources do-rpmbuild
+dist-srpm: do-rpmbuild

dist-srpm-gcov: BUILDID=.gcov
 dist-srpm-gcov: BUILDOPTS+=+gcov
 dist-srpm-gcov: dist-srpm

dist-rpms: RPMBUILDOPTS=--target $(MACH) -bb
-dist-rpms: dist-sources do-rpmbuild
+dist-rpms: do-rpmbuild

-dist-kernel-%: dist-sources
+dist-kernel-%:
    RPMBUILDOPTS="--target $(MACH) --with $* -bb" make do-rpmbuild

dist-prep: RPMBUILDOPTS=--nodeps --target noarch -bp
-dist-prep: dist-sources do-rpmbuild
+dist-prep: do-rpmbuild

dist-perf: RPMBUILDOPTS=--without up --without smp --without zfcpdump --without debug --without doc --without headers --without  --without doc --without debuginfo --target $(MACH) -bb
-dist-perf: dist-sources do-rpmbuild
+dist-perf: do-rpmbuild

dist-rpm-baseonly: RPMBUILDOPTS=--target $(MACH) --without debug --without debuginfo --without vdso_install --without bpftool --without perf --without tools -bb
-dist-rpm-baseonly: dist-sources do-rpmbuild
+dist-rpm-baseonly: do-rpmbuild

# unless you know what you're doing, you don't want to use the next four ones
 dist-release-finish: setup-source

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786

[OS-BUILD PATCH 3/3] [redhat] Add dist-buildreq-check for binary rpm builds

[Don Zickus (via Email Bridge)]

From: Don Zickus

[redhat] Add dist-buildreq-check for binary rpm builds

By simplifying the package check to srpm only, we neglect to check
the packaging for local binary rpm building.  Add that Makefile
dependency for those cases.

Signed-off-by: Don Zickus

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -573,8 +573,10 @@ dist-test-patch: generate-testpatch-tmp
 do-rpmbuild: dist-sources
    $(RPMBUILD) --define "_sourcedir $(SOURCES)" --define "_builddir $(RPM)/BUILD" --define "_srcrpmdir $(RPM)/SRPMS" --define "_rpmdir $(RPM)/RPMS" --define "_specdir $(RPM)/SPECS" --define "dist $(DIST)" $(RPMBUILDOPTS) $(RPM)/SOURCES/$(PACKAGE_NAME).spec

+do-rpmbuild-binary: dist-buildreq-check do-rpmbuild
+
 dist-all-rpms: RPMBUILDOPTS=--target $(MACH) -ba
-dist-all-rpms: do-rpmbuild
+dist-all-rpms: do-rpmbuild-binary

dist-srpm: RPMBUILDOPTS=--nodeps -bs
 dist-srpm: do-rpmbuild
@@ -584,19 +586,19 @@ dist-srpm-gcov: BUILDOPTS+=+gcov
 dist-srpm-gcov: dist-srpm

dist-rpms: RPMBUILDOPTS=--target $(MACH) -bb
-dist-rpms: do-rpmbuild
+dist-rpms: do-rpmbuild-binary

dist-kernel-%:
-	RPMBUILDOPTS="--target $(MACH) --with $* -bb" make do-rpmbuild
+	RPMBUILDOPTS="--target $(MACH) --with $* -bb" make do-rpmbuild-binary

dist-prep: RPMBUILDOPTS=--nodeps --target noarch -bp
 dist-prep: do-rpmbuild

dist-perf: RPMBUILDOPTS=--without up --without smp --without zfcpdump --without debug --without doc --without headers --without  --without doc --without debuginfo --target $(MACH) -bb
-dist-perf: do-rpmbuild
+dist-perf: do-rpmbuild-binary

dist-rpm-baseonly: RPMBUILDOPTS=--target $(MACH) --without debug --without debuginfo --without vdso_install --without bpftool --without perf --without tools -bb
-dist-rpm-baseonly: do-rpmbuild
+dist-rpm-baseonly: do-rpmbuild-binary

# unless you know what you're doing, you don't want to use the next four ones
 dist-release-finish: setup-source

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786

[OS-BUILD PATCH 3/3] [redhat] Add dist-buildreq-check for binary rpm builds

[Prarit Bhargava (via Email Bridge)]

From: Prarit Bhargava on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516328...

I understand why you've named this 'do-rpmbuild-binary' but I think that
will be confusing to a casual reader.  At first glance I thought it
implied that do-rpmbuild is for generating SRPMs only.  Perhaps a better
name here would be 'do-rpmbuild-buildreq-check'?  I realize that is an
annoyingly long name but it does describe what the target actually does.

[OS-BUILD PATCH 3/3] [redhat] Add dist-buildreq-check for binary rpm builds

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9517233...

Sure, but it doesn't describe why some rpmbuild commands get it, while others
do not.  Hence the -binary part.  If you are building binaries you needed the
extra suffix.  This makes it clear for future targets.  While the rule is the
same for your name, it doesn't appear to be as clear to me.

[Herton R. Krzesinski (via Email Bridge)]

From: Herton R. Krzesinski on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516277...

I believe you need to add a new dist-configs-prep-rpm and dist-configs-prep-
srpm: srpm generation does not process configs right? But other targets that
may process it will need full build requires because some package missing can
change the config generation.

[Herton R. Krzesinski (via Email Bridge)]

From: Herton R. Krzesinski on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516291...

or change the dist-config-check dependencies to add dist-buildreq-check, I
supposed it can work.

[Herton R. Krzesinski (via Email Bridge)]

From: Herton R. Krzesinski on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516315...

However, what about the case srpm builds correctly, but configs fail on build,
due an extra buildrequires that is now present? In that case, I believe
SPECSRPMBUILDREQS needs some packages added, like glibc-static.

[Justin M. Forbes (via Email Bridge)]

From: Justin M. Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516337...

srpm generation does process configs.  While you may not need full deps for a
proper config check, there are some which are required. That was what I meant
about this being a new list we will have to maintain.  Which buildreq's are
critical? I am somewhat curious as to what the driver is for this though. Is
it the space required, or just having to manually install all of the deps? If
it is the later, 'sudo dnf builddep kernel' makes it pretty easy.

[Prarit Bhargava (via Email Bridge)]

From: Prarit Bhargava on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9516416...

@hertonrk-rh @jmflinuxtx : I think we just all asked the same question of
@dzickusrh, but in slightly different ways :smile:

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9517209...

@hertonrk-rh @prarit @jmflinuxtx

To answer a variety of questions.  Yes srpm processes the configs.  It kinda
has to otherwise what is rpmbuild going to use to build the kernel? :-)

Why is this an issue now? It popped up with my MR last week actually fixing
how this works (otherwise the dep was only rpmbuild). Some folks do not want
to install the full list of buildreqs just to generate a srpm and upload it.
Especially for users on Fedora trying to build a RHEL-X srpm and don't want to
have to install obsoleted or missing RHEL-X buildreqs (system-sb-certs, llvm-
tools, etc).

As for 'dnf builddep kernel' that could work too in some cases.  One quirk
with that approach is it needs to match the kernel in development, so a RHEL
kernel on a Fedora distro may not work.  But we can advertise this too if we
want.

I am also looking at the docs here: https://cki-project.gitlab.io/kernel-
ark/#cloning-the-repository should either be auto-generated or told to use
'dnf builddep kernel'.  I am working with COPR and they didn't have an easy
way to install the packages necessary to build the srpm other than guessing.

I am trying to improve the user experience
git clone <kernel repo>
make dist-srpmbuildreqs-check | sudo dnf install  (??)
make dist-srpm
koji build

or something like that.  IOW there is a gap here that I am trying to address
with a consistent story.  Something that isn't complicated (yeah, this MR
leans toward complicated).

Thoughts?

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9517322...

Oh and @omos filed this https://gitlab.com/cki-project/kernel-
ark/-/merge_requests/1781 because he got hit with my buildreqs change from
last week.  That MR kinda undoes the check and defeats the spirit of it.

[Ondrej Mosnáček (via Email Bridge)]

From: Ondrej Mosnáček on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9526255...

This needs to include also `glibc-static`, which is known to affect config
processing. Also possibly `gcc-c++` (for getting compiler capabilities during
config generation) and `xz` (for compressing the tarball) might bee needed,
too. I guess also `bash` and `coreutils`, but without those you wouldn't even
get to the check, so not sure if it makes sense to add them :smile:

[Ondrej Mosnáček (via Email Bridge)]

From: Ondrej Mosnáček on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_9526255...

This feels like it should be deduped somehow... maybe by putting it into its
own `check_deps.sh` script, which would get PKGLIST via argv?

[Patrick Talbert (via Email Bridge)]

From: Patrick Talbert on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_1022391...

@dzickusrh where does this stand? If nothing else it needs to be rebased...

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1786#note_1065198...

Closing in favor of https://gitlab.com/cki-project/kernel-
ark/-/merge_requests/1900