tests/lib: generalize RPM build support #3449

Merged
merged 8 commits into from Jun 19, 2017

Conversation

Projects
None yet
7 participants
Contributor

morphis commented Jun 9, 2017

We had two independent functions before to build the RPM for Fedora and openSUSE. This PR combines both into a single function which uses rpmbuild.

Simon Fels added some commits Jun 8, 2017

zyga approved these changes Jun 9, 2017

+1, just one comment

tests/lib/prepare-project.sh
+ cp $rpm_dir/RPMS/$arch/snap*.rpm $GOPATH
+ if [[ "$SPREAD_SYSTEM" = fedora-* ]]; then
+ # On Fedora we have an additional package for SELinux
+ cp $rpm_dir/RPMS/noarch/snap*.rpm $GOPATH
@zyga

zyga Jun 9, 2017

Contributor

Could you use the more precise package name here in case we get some other packages and this matches them by accident?

@morphis

morphis Jun 9, 2017

Contributor

We will only ever get other packages in $rpm_dir/RPMS which are build by snapd.spec so this is fairly safe to do. We also do a cleanup of $rpm_dir a bit higher up.

tests/lib/prepare-project.sh
+ rpm_dir=$HOME/rpmbuild
+ ;;
+ opensuse-*)
+ rpm_dir=/usr/src/packages
@Conan-Kudo

Conan-Kudo Jun 9, 2017

Contributor

You can set rpmbuild to use the same path in both places.

just write the following to ~/.rpmmacros:

%_topdir %(echo $HOME)/rpmbuild
@Conan-Kudo

Conan-Kudo Jun 9, 2017

Contributor

Alternatively, you can just do rpm --eval "%_topdir" to get what the topdir is.

deps=()
n=0
IFS=$'\n'
- for dep in $(rpm -qpR /usr/src/packages/SRPMS/snapd-1337.*.src.rpm); do
+ for dep in $(rpm -qpR $rpm_dir/SRPMS/snapd-1337.*.src.rpm); do
@Conan-Kudo

Conan-Kudo Jun 9, 2017

Contributor

You can install builddeps pretty easily in Fedora and openSUSE.

  • Fedora: dnf builddep </path/to/package.src.rpm>
  • openSUSE: zypper si -d </path/to/package.src.rpm>

You may want to ensure builddep is available by doing dnf install 'dnf-command(builddep)', since I'm not sure whether the Linode VM has the package that provides it installed already...

@Conan-Kudo

Conan-Kudo Jun 12, 2017

Contributor

I talked to @morphis about this on IRC, apparently Zypper is dumb and doesn't let you do source-install with local SRPMs. Whatever...

codecov-io commented Jun 12, 2017

Codecov Report

Merging #3449 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3449      +/-   ##
==========================================
- Coverage   77.16%   77.16%   -0.01%     
==========================================
  Files         373      373              
  Lines       25793    25793              
==========================================
- Hits        19904    19902       -2     
- Misses       4132     4133       +1     
- Partials     1757     1758       +1
Impacted Files Coverage Δ
overlord/snapstate/snapstate.go 81.28% <0%> (-0.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59e14ca...c263e8d. Read the comment docs.

tests/lib/prepare-project.sh
@@ -83,8 +81,10 @@ build_rpm() {
# Cleanup all artifacts from previous builds
rm -rf $rpm_dir/BUILD/*
- # Install all necessary build dependencies
+ # Build our source package
rpmbuild --with testkeys --nocheck -bs $packaging_path/snapd.spec
@Conan-Kudo

Conan-Kudo Jun 12, 2017

Contributor

--nocheck has no effect with rpmbuild -bs, so this is a useless parameter.

Looks good to me at this point.

+1, nitpicks about indentation

tests/lib/prepare-project.sh
+
+ # And now build our binary package
+ rpmbuild \
+ --with testkeys \
@fgimenez

fgimenez Jun 13, 2017

Contributor

nit: indentation is broken here, we are using 4 spaces

@morphis

morphis Jun 13, 2017

Contributor

Fixed

tests/lib/prepare-project.sh
+
+ cp $rpm_dir/RPMS/$arch/snap*.rpm $GOPATH
+ if [[ "$SPREAD_SYSTEM" = fedora-* ]]; then
+ # On Fedora we have an additional package for SELinux
@fgimenez

fgimenez Jun 13, 2017

Contributor

same indentation nit as above

@morphis

morphis Jun 13, 2017

Contributor

Fixed

Simon Fels added some commits Jun 13, 2017

Contributor

niemeyer commented Jun 13, 2017

Test seems broken.

mvo5 approved these changes Jun 19, 2017

@mvo5 mvo5 merged commit 1495246 into snapcore:master Jun 19, 2017

5 of 7 checks passed

xenial-i386 autopkgtest finished (failure)
Details
xenial-ppc64el autopkgtest finished (failure)
Details
artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment