/snapd

packaging/fedora, tests: Fix build for snapd and enable a test for Fedora #3577

Closed
wants to merge 3 commits into
from

Conversation

Reviewers

@mvo5 mvo5

@zyga zyga

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@Conan-Kudo @codecov-io @chipaca @zyga @mvo5 @sergiocazzolato
@Conan-Kudo
Contributor

Conan-Kudo commented Jul 11, 2017 •
Edited 1 time

These changes come from pending changes for Fedora packaging for snapd 2.27.

Signed-off-by: Neal Gompa ngompa13@gmail.com

@Conan-Kudo Conan-Kudo referenced this pull request Jul 11, 2017

Closed

tests: make main/help run on opensuse and fedora again #3544

@codecov-io

codecov-io commented Jul 11, 2017 •
Edited 1 time

Codecov Report

Merging #3577 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3577      +/-   ##
==========================================
- Coverage   76.82%   76.81%   -0.02%     
==========================================
  Files         379      379              
  Lines       26314    26314              
==========================================
- Hits        20216    20213       -3     
- Misses       4304     4306       +2     
- Partials     1794     1795       +1
Impacted Files Coverage Δ
interfaces/sorting.go 93.33% <0%> (-3.34%) ⬇️
overlord/snapstate/snapstate.go 81.17% <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 75e3302...e2bcc3f. Read the comment docs.
@mvo5

mvo5 reviewed Jul 11, 2017
View changes

Looks good, just one question inline.

packaging/fedora/snapd.spec
@@ -345,6 +348,10 @@ GOFLAGS="$GOFLAGS -tags withtestkeys"
%gobuild -o bin/snapctl $GOFLAGS %{import_path}/cmd/snapctl
%gobuild -o bin/snap-update-ns $GOFLAGS %{import_path}/cmd/snap-update-ns
+# We don't need mvo5 fork for seccomp, as we have seccomp 2.3.x
@mvo5

mvo5 Jul 11, 2017

Collaborator

Nice!

packaging/fedora/snapd.spec
popd
+# Rename snapd.core-fixup.sh
@mvo5

mvo5 Jul 11, 2017

Collaborator

Why is this renamed? The unit file referes to it with the .sh suffix. Having said this, this unit is only relevant on core systems so its fine to just not package it at all on a fedora/opensuse system.

@Conan-Kudo
Contributor

Conan-Kudo commented Jul 11, 2017

@mvo5 I just purge the file now.
@chipaca
Member

chipaca commented Jul 11, 2017

Could you also include, in this pr, the change to tests/main/help/task.yaml that makes that test run on fedora?

@Conan-Kudo Conan-Kudo changed the title from packaging/fedora: Fix build for snapd to packaging/fedora, tests: Fix build for snapd and enable a test on Fedora Jul 11, 2017

@Conan-Kudo
Contributor

Conan-Kudo commented Jul 11, 2017

@chipaca Done.

@Conan-Kudo Conan-Kudo changed the title from packaging/fedora, tests: Fix build for snapd and enable a test on Fedora to packaging/fedora, tests: Fix build for snapd and enable a test for Fedora Jul 11, 2017

@Conan-Kudo
packaging/fedora: Fix build for snapd 
These changes come from pending changes for Fedora packaging for
snapd 2.27.

Signed-off-by: Neal Gompa <ngompa13@gmail.com>
9845d8e
@zyga

zyga requested changes Jul 11, 2017
View changes

NICE but I think some of the quoting is off.

packaging/fedora/snapd.spec
popd
+# Remove snappy core specific scripts
+rm %{buildroot}%{_libexecdir}/snapd/snapd.core-fixup.sh
@zyga

zyga Jul 11, 2017

Contributor

Thanks!

tests/lib/pkgdb.sh
@@ -279,13 +279,13 @@ distro_install_build_snapd(){
packages=
case "$SPREAD_SYSTEM" in
ubuntu-*|debian-*)
- packages="${GOHOME}"/snapd_*.deb
+ packages="${GOHOME}/snapd_*.deb"
@zyga

zyga Jul 11, 2017

Contributor

Hmm, does it really do the right thing? Compare ls "$HOME/*" and ls "$HOME"/*?

sergiocazzolato and others added some commits Jul 10, 2017

@sergiocazzolato @Conan-Kudo
tests: fix package building issues
7a34344
@Conan-Kudo
tests: Enable main/help for Fedora 
Signed-off-by: Neal Gompa <ngompa13@gmail.com>

Some checks were not successful

1 failing and 6 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
@mvo5
artful-amd64 — autopkgtest finished (success)
Details
@mvo5
xenial-amd64 — autopkgtest finished (success)
Details
@mvo5
xenial-i386 — autopkgtest finished (success)
Details
@mvo5
xenial-ppc64el — autopkgtest finished (success)
Details
@mvo5
yakkety-amd64 — autopkgtest finished (success)
Details
@mvo5
zesty-amd64 — autopkgtest finished (success)
Details
8e6a772
@Conan-Kudo
Contributor

Conan-Kudo commented Jul 11, 2017

@zyga Fixed.
@zyga

zyga approved these changes Jul 11, 2017
View changes

Woot, thanks

tests/lib/pkgdb.sh
;;
fedora-*)
- packages="${GOHOME}"/snap-confine*.rpm "${GOPATH}"/snapd*.rpm
+ packages="${GOHOME}/snap-confine*.rpm ${GOPATH}/snapd*.rpm"
@zyga

zyga Jul 11, 2017

Contributor

Same here

tests/lib/pkgdb.sh
;;
opensuse-*)
- packages="${GOHOME}"/snapd*.rpm
+ packages="${GOHOME}/snapd*.rpm"
@zyga

zyga Jul 11, 2017

Contributor

And here

tests/lib/prepare-project.sh
@@ -76,7 +76,7 @@ build_rpm() {
mkdir -p "/tmp/pkg/snapd-$version"
cp -rav -- * "/tmp/pkg/snapd-$version/"
mkdir -p "$rpm_dir/SOURCES"
- (cd /tmp/pkg || tar c${archive_compression}f "$rpm_dir/SOURCES/$archive_name" "snapd-$version" "$extra_tar_args")
+ (cd /tmp/pkg && tar c${archive_compression}f "$rpm_dir/SOURCES/$archive_name" "snapd-$version" $extra_tar_args)
@zyga

zyga Jul 11, 2017

Contributor

Nice catch!!

tests/main/help/task.yaml
@@ -1,4 +1,5 @@
summary: Check commands help
+systems: [+fedora-*]
@zyga

zyga Jul 11, 2017

Contributor

❤️

This was referenced Jul 11, 2017

Merged
Merged

@zyga zyga closed this Jul 12, 2017

@Conan-Kudo Conan-Kudo deleted the Conan-Kudo:packaging-fedora-fixbuild branch Jul 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment