7:13 p.m.
From: Prarit Bhargava on gitlab.com
Merge Request: https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
Fix the tests in redhat/self-test.
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
---
redhat/configs/build_configs.sh | 42 ++++++++++--------
redhat/configs/process_configs.sh | 53 +++++++++++++----------
redhat/self-test/0001-shellcheck.bats | 3 +-
redhat/self-test/1001-rpmlint.bats | 12 +++--
redhat/self-test/1002-basic-structural-test.bats | 16 ++++++-
redhat/self-test/1003-rpminspect.bats | 11 ++--
redhat/self-test/2001-dist-release.bats | 30 +++++++------
redhat/Makefile | 4 +-
redhat/genspec.sh | 12 ++--
9 files changed, 105 insertions(+), 78 deletions(-)
7:13 p.m.
New subject: [OS-BUILD PATCHv2 1/5] redhat/self-test: Fix shellcheck test
From: Prarit Bhargava <prarit(a)redhat.com>
redhat/self-test: Fix shellcheck test
Fix shellcheck errors and the shellcheck test.
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
diff --git a/redhat/configs/build_configs.sh b/redhat/configs/build_configs.sh
index blahblah..blahblah 100755
--- a/redhat/configs/build_configs.sh
+++ b/redhat/configs/build_configs.sh
@@ -16,6 +16,7 @@ else
echo "$3" > .flavors
fi
+# shellcheck disable=SC2015
RHJOBS="$(test -n "$4" && echo "$4" || nproc
--all)"
LANG=en_US.UTF-8
@@ -73,7 +74,7 @@ function merge_configs()
name=$OUTPUT_DIR/$PACKAGE_NAME-$archvar-$flavor.config
echo "Building $name ... "
- touch config-merging.$count config-merged.$count
+ touch config-merging."$count" config-merged."$count"
# apply based on order
skip_if_missing=""
@@ -85,31 +86,34 @@ function merge_configs()
test -n "$skip_if_missing" && test ! -e "$cfile"
&& continue
- if ! perl merge.pl "$cfile" config-merging.$count > config-merged.$count;
then
+ if ! perl merge.pl "$cfile" config-merging."$count" >
config-merged."$count"; then
die "Failed to merge $cfile"
fi
- mv config-merged.$count config-merging.$count
+ mv config-merged."$count" config-merging."$count"
done
# first configs in $order is baseline, all files should be
# there. second pass is overrides and can be missing.
skip_if_missing="1"
done
- if [ "x$arch" == "xaarch64" ]; then
- echo "# arm64" > "$name"
- elif [ "x$arch" == "xppc64le" ]; then
- echo "# powerpc" > "$name"
- elif [ "x$arch" == "xs390x" ]; then
- echo "# s390" > "$name"
- elif [ "x$arch" == "xarmv7hl" ]; then
- echo "# arm" > "$name"
- elif [ "x$arch" == "xi686" ]; then
- echo "# i386" > "$name"
- else
- echo "# $arch" > "$name"
- fi
- sort config-merging.$count >> "$name"
- rm -f config-merged.$count config-merging.$count
+
+ case "$arch" in
+ "aarch64")
+ echo "# arm64" > "$name";;
+ "ppc64le")
+ echo "# powerpc" > "$name";;
+ "s390x")
+ echo "# s390" > "$name";;
+ "armv7hl")
+ echo "# arm" > "$name";;
+ "i686")
+ echo "# i386" > "$name";;
+ *)
+ echo "# $arch" > "$name";;
+ esac
+
+ sort config-merging."$count" >> "$name"
+ rm -f config-merged."$count" config-merging."$count"
echo "Building $name complete"
}
@@ -156,7 +160,7 @@ function build_flavor()
merge_configs "$arch" "$configs" "$order"
"$flavor" "$count" &
waitpids[$count]=$!
((count++))
- while [ "$(jobs | grep Running | wc -l)" -ge $RHJOBS ]; do :; done
+ while [ "$(jobs | grep -c Running)" -ge "$RHJOBS" ]; do :; done
fi
done < "$control_file"
diff --git a/redhat/configs/process_configs.sh b/redhat/configs/process_configs.sh
index blahblah..blahblah 100755
--- a/redhat/configs/process_configs.sh
+++ b/redhat/configs/process_configs.sh
@@ -85,10 +85,10 @@ checkoptions()
print "Found "a[1]"="a[2]" after generation, had "
a[1]"="configs[a[1]]" in Source tree";
}
}
- ' "$1" "$2" > .mismatches${count}
+ ' "$1" "$2" > .mismatches"${count}"
checkoptions_error=false
- if test -s .mismatches${count}
+ if test -s .mismatches"${count}"
then
while read -r LINE
do
@@ -100,14 +100,14 @@ checkoptions()
checkoptions_error=true
break
fi
- done < .mismatches${count}
+ done < .mismatches"${count}"
! $checkoptions_error && return
- sed -i "1s/^/Error: Mismatches found in configuration files for ${arch}
${variant}\n/" .mismatches${count}
+ sed -i "1s/^/Error: Mismatches found in configuration files for ${arch}
${variant}\n/" .mismatches"${count}"
[ "$CONTINUEONERROR" ] || exit 1
else
- rm -f .mismatches${count}
+ rm -f .mismatches"${count}"
fi
}
@@ -206,11 +206,13 @@ function commit_new_configs()
fi
echo -n "Checking for new configs in $cfg ... "
- make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE=$(get_cross_compile $arch)
KCONFIG_CONFIG="$cfgorig" listnewconfig >& .listnewconfig
+ # shellcheck disable=SC2086
+ make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE="$(get_cross_compile
"$arch")" KCONFIG_CONFIG="$cfgorig" listnewconfig >&
.listnewconfig
grep -E 'CONFIG_' .listnewconfig > .newoptions
if test -s .newoptions
then
- make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE=$(get_cross_compile $arch)
KCONFIG_CONFIG="$cfgorig" helpnewconfig >& .helpnewconfig
+ # shellcheck disable=SC2086
+ make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE="$(get_cross_compile
"$arch")" KCONFIG_CONFIG="$cfgorig" helpnewconfig >&
.helpnewconfig
parsenewconfigs
fi
rm .newoptions
@@ -249,31 +251,33 @@ function process_config()
echo "Processing $cfg ... "
- make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE=$(get_cross_compile $arch)
KCONFIG_CONFIG="$cfgorig" listnewconfig >& .listnewconfig${count}
- grep -E 'CONFIG_' .listnewconfig${count} > .newoptions${count}
- if test -n "$NEWOPTIONS" && test -s .newoptions${count}
+ # shellcheck disable=SC2086
+ make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE="$(get_cross_compile
"$arch")" KCONFIG_CONFIG="$cfgorig" listnewconfig >&
.listnewconfig"${count}"
+ grep -E 'CONFIG_' .listnewconfig"${count}" >
.newoptions"${count}"
+ if test -n "$NEWOPTIONS" && test -s .newoptions"${count}"
then
- echo "Found unset config items in ${arch} ${variant}, please set them to an
appropriate value" >> .errors${count}
- cat .newoptions${count} >> .errors${count}
- rm .newoptions${count}
+ echo "Found unset config items in ${arch} ${variant}, please set them to an
appropriate value" >> .errors"${count}"
+ cat .newoptions"${count}" >> .errors"${count}"
+ rm .newoptions"${count}"
RETURNCODE=1
[ "$CONTINUEONERROR" ] || exit 1
fi
- rm .newoptions${count}
+ rm .newoptions"${count}"
- grep -E 'config.*warning' .listnewconfig${count} > .warnings${count}
- if test -n "$CHECKWARNINGS" && test -s .warnings${count}
+ grep -E 'config.*warning' .listnewconfig"${count}" >
.warnings"${count}"
+ if test -n "$CHECKWARNINGS" && test -s .warnings"${count}"
then
- echo "Found misconfigured config items in ${arch} ${variant}, please set them to
an appropriate value" >> .errors${count}
- cat .warnings${count} >> .errors${count}
- rm .warnings${count}
+ echo "Found misconfigured config items in ${arch} ${variant}, please set them to
an appropriate value" >> .errors"${count}"
+ cat .warnings"${count}" >> .errors"${count}"
+ rm .warnings"${count}"
[ "$CONTINUEONERROR" ] || exit 1
fi
- rm .warnings${count}
+ rm .warnings"${count}"
- rm .listnewconfig${count}
+ rm .listnewconfig"${count}"
- make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE=$(get_cross_compile $arch)
KCONFIG_CONFIG="$cfgorig" olddefconfig > /dev/null || exit 1
+ # shellcheck disable=SC2086
+ make ${MAKEOPTS} ARCH="$arch" CROSS_COMPILE="$(get_cross_compile
"$arch")" KCONFIG_CONFIG="$cfgorig" olddefconfig > /dev/null
|| exit 1
echo "# $arch" > "$cfgtmp"
cat "$cfgorig" >> "$cfgtmp"
if test -n "$CHECKOPTIONS"
@@ -307,7 +311,7 @@ function process_configs()
process_config "$cfg" "$count" &
waitpids[${count}]=$!
((count++))
- while [ "$(jobs | grep Running | wc -l)" -ge $RHJOBS ]; do :; done
+ while [ "$(jobs | grep -c Running)" -ge "$RHJOBS" ]; do :; done
done
for pid in ${waitpids[*]}; do
wait ${pid}
@@ -374,7 +378,7 @@ do
;;
-m)
shift
- if [ "$1" = "CC=clang" -o "$1" = "LLVM=1" ];
then
+ if [ "$1" = "CC=clang" ] || [ "$1" = "LLVM=1"
]; then
CC_IS_CLANG=1
fi
MAKEOPTS="$MAKEOPTS $1"
@@ -389,6 +393,7 @@ PACKAGE_NAME="${1:-kernel}" # defines the package name used
KVERREL="$(test -n "$2" && echo "-$2" || echo
"")"
SUBARCH="$(test -n "$3" && echo "-$3" || echo
"")"
FLAVOR="$(test -n "$4" && echo "-$4" || echo
"-ark")"
+# shellcheck disable=SC2015
RHJOBS="$(test -n "$5" && echo "$5" || nproc
--all)"
SCRIPT=$(readlink -f "$0")
SCRIPT_DIR=$(dirname "$SCRIPT")
diff --git a/redhat/genspec.sh b/redhat/genspec.sh
index blahblah..blahblah 100755
--- a/redhat/genspec.sh
+++ b/redhat/genspec.sh
@@ -1,6 +1,6 @@
#!/bin/bash
-LAST_MARKER=$(cat $REDHAT/marker)
+LAST_MARKER=$(cat "${REDHAT}"/marker)
clogf="$SOURCES/changelog"
# hide [redhat] entries from changelog
HIDE_REDHAT=1;
@@ -25,11 +25,11 @@ if [[ -z $lasttag ]]; then
fi
echo "Gathering new log entries since $lasttag"
# master is expected to track mainline.
-UPSTREAM="$(git rev-parse -q --verify origin/$UPSTREAM_BRANCH || \
- git rev-parse -q --verify $UPSTREAM_BRANCH)"
+UPSTREAM=$(git rev-parse -q --verify origin/"${UPSTREAM_BRANCH}" || \
+ git rev-parse -q --verify "${UPSTREAM_BRANCH}")
git log --topo-order --reverse --no-merges -z --format="- %s (%an)%n%b" \
- ^${UPSTREAM} "$lasttag".. -- ':!/redhat/rhdocs' | ${0%/*}/genlog.py
>> "$clogf"
+ ^"${UPSTREAM}" "$lasttag".. -- ':!/redhat/rhdocs' |
"${0%/*}"/genlog.py >> "$clogf"
grep -v "tagging $RPMVERSION" "$clogf" >
"$clogf.stripped"
cp "$clogf.stripped" "$clogf"
@@ -151,10 +151,10 @@ if [ "$SINGLE_TARBALL" = 0 ]; then
# May need to preserve word splitting in EXCLUDE_FILES
# shellcheck disable=SC2086
git diff -p --no-renames --stat "$MARKER".. $EXCLUDE_FILES \
- >
"$SOURCES"/patch-"${RPMKVERSION}.${RPMKPATCHLEVEL}"-redhat.patch
+ > ${SOURCES}/patch-${RPMKVERSION}.${RPMKPATCHLEVEL}-redhat.patch
else
# Need an empty file for dist-git compatibility
- touch
"$SOURCES"/patch-"${RPMKVERSION}.${RPMKPATCHLEVEL}"-redhat.patch
+ touch "${SOURCES}/patch-${RPMKVERSION}.${RPMKPATCHLEVEL}"-redhat.patch
fi
# We depend on work splitting of BUILDOPTS
diff --git a/redhat/self-test/0001-shellcheck.bats
b/redhat/self-test/0001-shellcheck.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/0001-shellcheck.bats
+++ b/redhat/self-test/0001-shellcheck.bats
@@ -3,6 +3,5 @@
then
skip "The ShellCheck package is not installed"
fi
- run shellcheck $(find $BATS_TEST_DIRNAME/.. -name "*.sh")
- [ "$status" = 0 ]
+ shellcheck $(find $BATS_TEST_DIRNAME/.. -name "*.sh" -not -path
"$BATS_TEST_DIRNAME/../rpm/*")
}
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
7:13 p.m.
New subject: [OS-BUILD PATCHv2 2/5] redhat/self-test: Cleanup SRPM related
self-tests
From: Prarit Bhargava <prarit(a)redhat.com>
redhat/self-test: Cleanup SRPM related self-tests
The self-tests fail messily and loudly when a single SRPM is not
present in the directory. Fix these tests to skip instead of loudly
failing.
[Aside: I could have added a call to dist-srpm in the dist-self-test
target, however, that could result in overwriting an existing SRPM on the
users' system.]
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
diff --git a/redhat/self-test/1001-rpmlint.bats b/redhat/self-test/1001-rpmlint.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/1001-rpmlint.bats
+++ b/redhat/self-test/1001-rpmlint.bats
@@ -1,9 +1,4 @@
#!/usr/bin/env bats
-@test "Exactly one SRPM exists" {
- result=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
- [ "$result" = 1 ]
-}
-
@test "rpmlint" {
if ! test -x /usr/bin/rpmlint
then
@@ -11,7 +6,14 @@
else
skip "Skip rpmlint test pending kernel.spec.template changes"
fi
+
+ numsrpms=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ if [ "$numsrpms" != "1" ]; then
+ skip "Only one SRPM should be in $BATS_TEST_DIRNAME/redhat/rpms/SRPMS."
+ fi
+
srpm=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm")
run rpmlint $srpm
+ status=$?
[ "$status" = 0 ]
}
diff --git a/redhat/self-test/1002-basic-structural-test.bats
b/redhat/self-test/1002-basic-structural-test.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/1002-basic-structural-test.bats
+++ b/redhat/self-test/1002-basic-structural-test.bats
@@ -1,6 +1,10 @@
#!/usr/bin/env bats
@test "SRPM unpacks OK" {
- result=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ numsrpms=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ if [ "$numsrpms" != "1" ]; then
+ skip "Only one SRPM should be in $BATS_TEST_DIRNAME/redhat/rpms/SRPMS."
+ fi
+
srpm=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm")
pushd "$BATS_TMPDIR"
if [ -e SRPMS ]
@@ -16,6 +20,11 @@
}
@test "Linux tree unpacks OK" {
+ numsrpms=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ if [ "$numsrpms" != "1" ]; then
+ skip "Only one SRPM should be in $BATS_TEST_DIRNAME/redhat/rpms/SRPMS."
+ fi
+
pushd "$BATS_TMPDIR"/SRPMS >& /dev/null
ls | wc
linuxname=$(ls linux*.tar.xz)
@@ -25,6 +34,11 @@
}
@test "Linux top level structural check" {
+ numsrpms=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ if [ "$numsrpms" != "1" ]; then
+ skip "Only one SRPM should be in $BATS_TEST_DIRNAME/redhat/rpms/SRPMS."
+ fi
+
pushd "$BATS_TMPDIR"/SRPMS >& /dev/null
linuxtree=$(ls linux*.tar.xz)
linuxtree=${linuxtree/.tar.xz}
diff --git a/redhat/self-test/1003-rpminspect.bats
b/redhat/self-test/1003-rpminspect.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/1003-rpminspect.bats
+++ b/redhat/self-test/1003-rpminspect.bats
@@ -1,9 +1,4 @@
#!/usr/bin/env bats
-@test "Exactly one SRPM exists" {
- result=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
- [ "$result" = 1 ]
-}
-
@test "rpminspect" {
if ! test -x /usr/bin/rpminspect
then
@@ -11,6 +6,12 @@
else
skip "Skip rpminspect test pending fixes"
fi
+
+ numsrpms=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm" | wc -l)
+ if [ "$numsrpms" != "1" ]; then
+ skip "Only one SRPM should be in $BATS_TEST_DIRNAME/redhat/rpms/SRPMS."
+ fi
+
srpm=$(find "$BATS_TEST_DIRNAME"/.. -name "*.rpm")
run rpminspect $srpm
[ "$status" = 0 ]
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
7:13 p.m.
New subject: [OS-BUILD PATCHv2 3/5] redhat/self-test: Fix BUILD verification test
From: Prarit Bhargava <prarit(a)redhat.com>
redhat/self-test: Fix BUILD verification test
This test fails because it assumes that the top commit will be a
"[redhat]: kernel-version" commit. This is true for the main tree but
isn't true for development branches.
Change the test to look for the latest "[redhat]: kernel-version" commit
and compare the value of BUILD to the value in that kernel version.
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
diff --git a/redhat/self-test/2001-dist-release.bats
b/redhat/self-test/2001-dist-release.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/2001-dist-release.bats
+++ b/redhat/self-test/2001-dist-release.bats
@@ -24,19 +24,20 @@
@test "dist-release test 2" {
# Test whether release number in commit message matches
- # release number in Makefile.rhelver, which is BUILD as
- # established in Makefile.common (BUILD:=$(RHEL_RELEASE))
+ # release number in Makefile.rhelver.
# and above in prologue.
cd $BATS_TMPDIR/distrelease
- commit="$(git log --oneline -n 1)"
- # commit = ... [redhat] kernel-5.11.0-0.rc0.20201220git467f8165a2b0.104
- # Just the commit message part AFTER "[redhat] ":
- title=${commit##*\[redhat\] }
+ title="$(git log --oneline --all --grep "\[redhat\] kernel" -n 1
--pretty="format:%s")"
+ # title = ... [redhat] kernel-5.11.0-0.rc0.20201220git467f8165a2b0.104
+ # Just the title message part AFTER "[redhat] ":
+ title=${title##*\[redhat\] }
# Strip off ...kernel-VV.PP.SS-:
pkgrelease=${title##*kernel-+([5-9]).+([0-9]).+([0-9])-}
- # BUILD = RHEL_RELEASE from Makefile.rhelver; cf. Makefile.common:
- BUILD=$(DIST=.fc33 make dist-dump-variables | grep "^BUILD=" | sed -e
's/BUILD=//')
- echo $pkgrelease | grep -q -w $BUILD
+ build=$(BUILD= DIST=.fc33 make dist-dump-variables | grep -E "^BUILD=" |
cut -d"=" -f2 | xargs)
+ ((build--))
+ echo "pkgrelease=$pkgrelease"
+ echo "build=$build"
+ echo $pkgrelease | grep -q -w "$build"
status=$?
[ "$status" = 0 ]
}
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
7:13 p.m.
New subject: [OS-BUILD PATCHv2 4/5] redhat/self-test: Fix version tag test
From: Prarit Bhargava <prarit(a)redhat.com>
redhat/self-test: Fix version tag test
The version tag test is using an incorrect changelog ID and doesn't
look for the last '[redhat] kernel-*' commit.
Fix both of these problems so the test runs correctly.
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
diff --git a/redhat/self-test/2001-dist-release.bats
b/redhat/self-test/2001-dist-release.bats
index blahblah..blahblah 100644
--- a/redhat/self-test/2001-dist-release.bats
+++ b/redhat/self-test/2001-dist-release.bats
@@ -48,13 +48,14 @@
cd $BATS_TMPDIR/distrelease
# Extract just the version part (the part between [ ]) on the first line of
# the change log:
- changelogversion=$(head -1 ./redhat/kernel.changelog-8.99 | sed -e
's/.*\[\(.*\)\].*/\1/')
- commit="$(git log --oneline -n 1)"
+ changelog=$(head -1 ./redhat/kernel.changelog-${RHEL_MAJOR}.${RHEL_MINOR} | sed -e
's/.*\[\(.*\)\].*/\1/')
+ commit="$(git log --oneline --all --grep "\[redhat\] kernel" -n 1
--pretty="format:%s")"
# Extract just the commit message part AFTER "[redhat] ":
- title=${commit##*\[redhat\] }
+ gitlog=${commit##*\[redhat\] }
# This time, strip off "kernel-" also:
- title=${title/kernel-/}
- [ "$changelogversion" = "$title" ]
+ gitlog=${gitlog/kernel-/}
+ echo "The kernel version in the changelog ($changelog) differs from the version
in the git log ($gitlog)"
+ [ "$changelog" = "$gitlog" ]
}
@test "dist-release epilogue" {
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
7:13 p.m.
New subject: [OS-BUILD PATCHv2 5/5] redhat: Fix release tagging
From: Prarit Bhargava <prarit(a)redhat.com>
redhat: Fix release tagging
The release is tagged into the changelog and the git log. The changelog
tags were missing the $(RPMKSUBLEVEL) variable which caused the self-tests
to fail when manually run.
Fix the release tagging so the same tag is in the git log and changelog.
Signed-off-by: Prarit Bhargava <prarit(a)redhat.com>
diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -187,7 +187,7 @@
KABI_TARBALL:=$(SOURCES)/kernel-abi-stablelists-$(KVERSION)-$(DISTRO_BUILD).tar.
KABIDW := $(REDHAT)/kabi-dwarf
KABIDW_TARBALL:=$(SOURCES)/kernel-kabi-dw-$(KVERSION)-$(DISTRO_BUILD).tar.bz2
PKGRELEASE:=$(PREBUILD)$(BUILD)$(BUILDID)
-RPMVERSION:=$(RPMKVERSION).$(RPMKPATCHLEVEL)-$(PKGRELEASE)
+RPMVERSION:=$(RPMKVERSION).$(RPMKPATCHLEVEL).$(RPMKSUBLEVEL)-$(PKGRELEASE)
SPECRELEASE:=$(PREBUILD)$(BUILD)%{?buildid}%{?dist}
SRPM:=$(SRPMS)/$(PACKAGE_NAME)-$(KVERSION)-$(PKGRELEASE)$(DIST).src.rpm
@@ -580,7 +580,7 @@ dist-release-changed: setup-source
dist-release: dist-clean-sources
@$(MAKE) dist-release-changed
dist-release-tag:
- @$(GIT) tag -a -m "$(PACKAGE_NAME)-$(KVERSION)-$(PKGRELEASE)"
$(PACKAGE_NAME)-$(KVERSION)-$(PKGRELEASE)
+ @$(GIT) tag -a -m "$(PACKAGE_NAME)-$(RPMVERSION)"
$(PACKAGE_NAME)-$(KVERSION)-$(PKGRELEASE)
git-tree-check:
@if test -n "$(DIST_PUSH)" && test -z "$(shell $(GIT) remote
get-url gitlab 2>/dev/null)"; then \
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642
3:04 p.m.
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8862...
@prarit - a couple of comments.
based on internal discussions, we can see relying on HEAD to be a redhat tag
commit may not work. In the same vein, when @ptalbert clicks 'Merge' through
out the day and someone creates an MR off that merge, do we run into the same
problem? Though the self-test seems isolated to dist-release, which makes the
change fine because the expected output *is* a redhat tag commit.
Second, the SRPM skip. self-tests are about controlled output. If someone
makes a change to scripts or Makefile, then the self-tests needs to regenerate
the srpm to verify the updated contents. If that is unnecessary and we are ok
with the users srpm, then what is the point of the check? Assuming the users
srpm is correct for self-tests is a bad idea. Either regenerate it or remove
the check.
4:41 p.m.
On 3/23/22 16:04, Don Zickus (via Email Bridge) wrote:
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8862...
@prarit - a couple of comments.
based on internal discussions, we can see relying on HEAD to be a redhat tag
commit may not work. In the same vein, when @ptalbert clicks 'Merge' through
out the day and someone creates an MR off that merge, do we run into the same
problem? Though the self-test seems isolated to dist-release, which makes the
change fine because the expected output *is* a redhat tag commit.
The test is dependent on the dist-release adding the tag. I've been
thinking about HEAD and have some other ideas unrelated to the tests
about how to deal with it.
Second, the SRPM skip. self-tests are about controlled output. If someone
makes a change to scripts or Makefile, then the self-tests needs to regenerate
the srpm to verify the updated contents. If that is unnecessary and we are ok
with the users srpm, then what is the point of the check? Assuming the users
srpm is correct for self-tests is a bad idea. Either regenerate it or remove
the check.
These tests will be run as part of CI. In that case there will always
be ONE test. The problem with local execution is that if there are many
SRPMs it may not be obvious which specific SRPM should be consumed. It
could be the user has a broken SRPM because they are testing code, etc.
I do think the SRPM test is interesting in the long term to identify
dist-srpm issues but certainly can be convinced otherwise. I'm not
convinced all the tests are truly valid tests.
P.
_______________________________________________
kernel mailing list -- kernel(a)lists.fedoraproject.org
To unsubscribe send an email to kernel-leave(a)lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
4:38 a.m.
From: Patrick Talbert on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8870...
@dzickusrh Did you mean to make this a blocking comment? Otherwise this MR is
technically ready to go...
9:08 a.m.
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8874...
I did yes. Sorry about that.
9:14 a.m.
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8874...
@prarit said:
```
The test is dependent on the dist-release adding the tag. I've been
thinking about HEAD and have some other ideas unrelated to the tests
about how to deal with it.
```
Ok, so that is fine then.
```
These tests will be run as part of CI. In that case there will always
be ONE test. The problem with local execution is that if there are many
SRPMs it may not be obvious which specific SRPM should be consumed. It
could be the user has a broken SRPM because they are testing code, etc.
I do think the SRPM test is interesting in the long term to identify
dist-srpm issues but certainly can be convinced otherwise. I'm not
convinced all the tests are truly valid tests.
```
So again, if we are using an srpm to validate a test, the self-tests needs to
generate it using the current code. In the case of multiple SRPMS, we should
know what the name is going to be, right? We have the Makefile and env logic.
If this test does not need the SRPM, then the 'skip' seems odd.
Or are you just trying to fix up a test that you have no idea why it exists
(because it may not be obvious)?
9:18 a.m.
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8874...
@prarit - actually, I thought 'git worktree' was used here to clone the env to
avoid destroying the users env (ie SRPM). Therefore, self-tests should be in
a clean room env and a single SRPM is expected. Perhaps it doesn't work like
that no more?
9:23 a.m.
From: Prarit Bhargava on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8874...
Or are you just trying to fix up a test that you have no idea why it
exists
(because it may not be obvious)?
:D LOL yes. I don't want to change anything until I get more commits out to
clean up the Makefile. I have some better ideas for tests, including
verifying VARIABLE output with different top-level Makefile variable
information. I think that (hopefully) will result in some of the existing
tests being obsoleted.
Until then, I don't want to change anything out of concern for maintaining the
status quo.
Personal opinion: The tests aren't really that interesting and IMO it's best
to junk them all. But, since one DID point out a bug, I decided to keep them
in place.
9:25 a.m.
From: Prarit Bhargava on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8874...
Maybe another way to state this is: At this time, my intention is only to fix
the existing tests. Later commits will introduce new tests.
Does that explain it? /me isn't sure if he has
7:45 a.m.
From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1642#note_8890...
After talking with @prarit, we came to the conclusion that there is a lot of
work to do here. This change unblocks other work and we will circle back
later to resolve this. Resolving thread for now.
757
days inactive
760
days old
kernel@lists.fedoraproject.org
14 comments
4 participants
participants (4)
-
Don Zickus (via Email Bridge) -
Patrick Talbert (via Email Bridge)
-
Prarit Bhargava -
Prarit Bhargava (via Email Bridge)