[OS-BUILD PATCHv2 3/4] redhat/Makefile: fix setup-source and document its caveat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Ondrej Mosnacek <omosnace@xxxxxxxxxx>

redhat/Makefile: fix setup-source and document its caveat

The current definition of setup-source is racy, since it may cause
dist-clean-sources and _setup-source to be "built" in parallel and the
former caould then remove the results of the latter. Fix it to ensure
the right ordering and document the condition that must be preserved to
keep dist-clean-sources from racing with other targets.

_setup-source (and this race) has been introduced in commit 0cce4209d1cc
("redhat/Makefile: Split up setup-source target") in order to be called
from redhat/self-test/data/create-data.sh, where multiple source preps
can run in parallel. Make create-data.sh call the full setup-source and
use the existing RHSELFTESTDATA variable as an indication that
dist-clean-sources should be skipped.

Fixes: 0cce4209d1cc ("redhat/Makefile: Split up setup-source target")
Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -574,10 +574,11 @@ dist-clean-configs:
 	@cd $(REDHAT)/configs; rm -f kernel-*.config kernel-*.config.orig kernel-*.config.tmp partial-*-snip.config
 
 dist-clean-sources:
+# skip dist-clean-sources when generating test data to prevent race condition
+ifndef RHSELFTESTDATA
 	@rm -f $(RPM)/SPECS/*
-	@for i in $(SOURCES)/*; do \
-		rm -f $$i; \
-	done;
+	@rm -f $(SOURCES)/*
+endif
 
 dist-clean-rpmdirs:
 	@for i in $(RPM)/{BUILD,SRPMS,RPMS,SPECS}/*; do \
@@ -638,7 +639,11 @@ dist-get-buildreqs: setup-source
 		echo "PASS: All build dependencies found."; \
 	fi
 
-_setup-source: dist-git-version-check
+# IMPORTANT: All targets whose recipes read/modify files under $(SOURCES)
+# MUST depend on setup-source (at least indirectly). Otherwise such
+# recipes may be ordered before/in parallel with dist-clean-sources
+# (prerequisite of setup-source), which would then delete their results.
+setup-source: dist-clean-sources dist-git-version-check
 	@if [ ! -e $(REDHAT)/$(SPECFILE).template ]; then \
 		echo "Creating $(REDHAT)/$(SPECFILE).template as a copy of $(REDHAT)/kernel.spec.template"; \
 		cp $(REDHAT)/kernel.spec.template $(REDHAT)/$(SPECFILE).template; \
@@ -660,8 +665,6 @@ _setup-source: dist-git-version-check
 	@$(REDHAT)/scripts/genspec/genspec.sh
 	@cp $(SOURCES)/$(SPECFILE) $(SOURCES)/../SPECS/
 
-setup-source: dist-clean-sources _setup-source
-
 generate-testpatch-tmp:
 	@$(GIT) diff --no-renames HEAD ":(exclude,top).get_maintainer.conf" \
 		":(exclude,top).gitattributes" \
diff --git a/redhat/self-test/data/create-data.sh b/redhat/self-test/data/create-data.sh
index blahblah..blahblah 100755
--- a/redhat/self-test/data/create-data.sh
+++ b/redhat/self-test/data/create-data.sh
@@ -28,7 +28,7 @@ specfile_helper () {
 
 	specfilename=$1
 	cp ./kernel.spec.template "${varfilename}.spec.template"
-	make RHSELFTESTDATA=1 SPECFILE="${specfilename}.spec" DIST="${DIST}" DISTRO="${DISTRO}" HEAD="${commit}" _setup-source
+	make RHSELFTESTDATA=1 SPECFILE="${specfilename}.spec" DIST="${DIST}" DISTRO="${DISTRO}" HEAD="${commit}" setup-source
 	grep -Fvx -f "${specfilename}.spec.template" "${sources}/${specfilename}.spec" > "${destdir}"/"${specfilename}".spec
 	# Ignore bpftoolversion definition as it may change.
 	sed -i '/^%define bpftoolversion /d' "${destdir}"/"${specfilename}".spec

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2913
--
_______________________________________________
kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Index of Archives]     [Fedora General Discussion]     [Older Fedora Users Archive]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Legacy]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Announce]     [Fedora Package Review]     [Fedora Music]     [Fedora Packaging]     [Centos]     [Fedora SELinux]     [Coolkey]     [Yum Users]     [Tux]     [Yosemite News]     [KDE Users]     [Fedora Art]     [Fedora Docs]     [USB]     [Asterisk PBX]

  Powered by Linux