Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
tests,packaging: add package build support for openSUSE #3406
Conversation
added some commits
May 29, 2017
Some checks were not successful
Some checks were not successful
codecov-io
commented
May 29, 2017
•
Edited 1 time
-
codecov-io
May 30, 2017
-
May 30, 2017
Codecov Report
@@ Coverage Diff @@
## master #3406 +/- ##
==========================================
+ Coverage 77.26% 77.27% +<.01%
==========================================
Files 373 373
Lines 25687 25687
==========================================
+ Hits 19848 19849 +1
+ Misses 4096 4095 -1
Partials 1743 1743
Continue to review full report at Codecov.
|
added some commits
May 29, 2017
Some checks were not successful
Some checks were not successful
Some checks were not successful
Some checks were not successful
All checks have passed
morphis
referenced this pull request
May 30, 2017
Closed
tests/main: enable a first set of test cases for Fedora and openSUSE #3411
added some commits
Jun 2, 2017
Some checks were not successful
All checks have passed
| @@ -18,7 +32,7 @@ systemctl_stop() { | ||
| fi | ||
| } | ||
| -if [ "$1" = "purge" ]; then | ||
| +purge() { |
mvo5
Jun 7, 2017
Collaborator
I added a forum topic that we need to look into sharing code between this and the debian postrm (https://forum.snapcraft.io/t/share-code-between-debian-postrm-purge-and-snap-mgmt-sh/915)
| - # poor-man's "quiet" | ||
| - apt-get update >& "$tf" || ( cat "$tf"; exit 1 ) | ||
| - apt-get install -y xdelta3 curl >& "$tf" || ( cat "$tf"; exit 1 ) | ||
| + # NOTE: We can't use tests/lib/pkgdb.sh here as it doesn't exist at |
mvo5
Jun 7, 2017
Collaborator
Thanks for this comment! I was wondering exactly this and you answered it :-D
| @@ -13,6 +13,28 @@ install_build_snapd(){ | ||
| mv sources.list.back /etc/apt/sources.list | ||
| apt update | ||
| else | ||
| - distro_install_local_package "$GOHOME"/snapd_*.deb | ||
| + packages= | ||
| + case "$SPREAD_SYSTEM" in |
mvo5
Jun 7, 2017
Collaborator
Idle wondering if the name for this file (apt.sh) is still appropriate - maybe it should just get merged with pkgdb.sh - but that is a different PR I guess :)
| + dnf -y -q upgrade | ||
| + ;; | ||
| + opensuse-*) | ||
| + zypper -q update -y |
mvo5
Jun 7, 2017
Collaborator
Funny how similar all the tools are in their CLI interface. I guess apt had a lot of influence here.
| @@ -96,29 +101,53 @@ prepare_classic() { | ||
| if snap --version |MATCH unknown; then | ||
| echo "Package build incorrect, 'snap --version' mentions 'unknown'" | ||
| snap --version | ||
| - apt-cache policy snapd | ||
| + case "$SPREAD_SYSTEM" in |
mvo5
Jun 7, 2017
Collaborator
Could we abstract this to pkgdb.sh as well maybe? Feels a bit ugly to have this test here.
added some commits
Jun 7, 2017
All checks have failed
added some commits
Jun 7, 2017
Some checks were not successful
Some checks were not successful
Some checks were not successful
All checks have passed
Some checks were not successful
| @@ -1,5 +1,4 @@ | ||
| summary: Check snap ack | ||
| -systems: [-ubuntu-core-16-arm-*] |
Some checks were not successful
| @@ -171,7 +173,7 @@ install -m 644 -D packaging/opensuse-42.2/permissions %buildroot/%{_sysconfdir}/ | ||
| install -m 644 -D packaging/opensuse-42.2/permissions.paranoid %buildroot/%{_sysconfdir}/permissions.d/snapd.paranoid | ||
| # Install the systemd units | ||
| make -C data/systemd install DESTDIR=%{buildroot} SYSTEMDSYSTEMUNITDIR=%{_unitdir} | ||
| -for s in snapd.autoimport.service snapd.system-shutdown.service; do | ||
| +for s in snapd.autoimport.service snapd.system-shutdown.service snap-repair.timer snap-repair.service; do |
zyga
Jun 8, 2017
Contributor
I was under the impression that the repair functionality is only applicable to core systems. I would not recommend using it in any classic distribution where existing packaging systems are sufficient to recover from any issue.
morphis
Jun 8, 2017
Contributor
That is exactly why we're rm these system units here after they got installed by data/systemd/Makefile :-)
| @@ -317,7 +322,7 @@ restore: | | ||
| suites: | ||
| tests/main/: | ||
| summary: Full-system tests for snapd | ||
| - systems: [-fedora-*] | ||
| + systems: [-fedora-*, -opensuse-*] |
| @@ -336,14 +341,17 @@ suites: | ||
| if [[ "$SPREAD_SYSTEM" != ubuntu-core-16-* ]]; then | ||
| . $TESTSLIB/pkgdb.sh | ||
| distro_purge_package snapd | ||
| - distro_purge_package snap-confine | ||
| + if [[ "$SPREAD_SYSTEM" != opensuse-* ]]; then |
zyga
Jun 8, 2017
Contributor
This is where I would argue that the following solution is cleaner:
# NOTE: snap-confine never existed on openSUSE
distro-purge-package {debian,fedora}:snap-confine`
morphis
Jun 8, 2017
Contributor
It's always a tradeoff. Purging not existing packages is a very rare case where as installing packages is the very regular one. So limiting us to a minimal set of these is enough here.
| fi | ||
| tests/completion/: | ||
| summary: completion tests | ||
| # ppc64el disabled because of https://bugs.launchpad.net/snappy/+bug/1655594 | ||
| - systems: [-ubuntu-core-*, -ubuntu-*-ppc64el, -fedora-*] | ||
| + systems: [-ubuntu-core-*, -ubuntu-*-ppc64el, -fedora-*, -opensuse-*] |
zyga
Jun 8, 2017
Contributor
Why are those tests disabled on Fedora and openSUSE? Can you please add a comment.
| @@ -373,7 +381,7 @@ suites: | ||
| tests/regression/: | ||
| summary: Regression tests for snapd | ||
| - systems: [-fedora-*] | ||
| + systems: [-fedora-*, -opensuse-*] |
zyga
Jun 8, 2017
Contributor
Can you please document why those are disabled? Ideally we'd run them and skip the parts that involve confinement.
| @@ -392,7 +400,7 @@ suites: | ||
| tests/upgrade/: | ||
| summary: Tests for snapd upgrade | ||
| - systems: [-ubuntu-core-16-*, -fedora-*] | ||
| + systems: [-ubuntu-core-16-*, -fedora-*, -opensuse-*] |
| @@ -408,7 +416,7 @@ suites: | ||
| tests/unit/: | ||
| summary: Suite to run unit tests (non-go and different go runtimes) | ||
| - systems: [-ubuntu-core-16-*, -fedora-*] | ||
| + systems: [-ubuntu-core-16-*, -fedora-*, -opensuse-*] |
zyga
Jun 8, 2017
Contributor
Can you please document those too? I thought that we can run unit tests on openSUSE now, is this not the case?
morphis
Jun 8, 2017
Contributor
We can run unit tests on openSUSE but as explained before this PR just adds build support for openSUSE and doesn't enable any spread tests (in real we have to enable at least a single one tests/main/help to get the build running). Enabling tests will happen with later PRs step by step.
| @@ -434,7 +442,7 @@ suites: | ||
| tests/nightly/: | ||
| summary: Suite for nightly, expensive, tests | ||
| manual: true | ||
| - systems: [-fedora-*] | ||
| + systems: [-fedora-*, -opensuse-*] |
| @@ -453,7 +461,7 @@ suites: | ||
| tests/nested/: | ||
| summary: Tests for nested images | ||
| - systems: [-fedora-*] | ||
| + systems: [-fedora-*, -opensuse-*] |
| @@ -31,13 +31,24 @@ fedora_name_package() { | ||
| esac | ||
| } | ||
| +opensuse_name_package() { |
morphis
Jun 8, 2017
Contributor
Why are they backwards` It's were distro=opensuse verb=to name and what=package in this case.
zyga
Jun 8, 2017
Contributor
It just looks odd as nothing else uses this style. If anything I'd call it opensuse_package_name (object.object.object style)
| + packages="${GOHOME}/snapd_*.deb" | ||
| + ;; | ||
| + fedora-*|opensuse-*) | ||
| + packages="${GOHOME}/snap-confine*.rpm ${GOPATH}/snapd*.rpm" |
zyga
Jun 8, 2017
Contributor
snap-confine is not a package for openSUSE, I think you have to split this
morphis
Jun 8, 2017
Contributor
We could but don't have to, Zypper wont fail interestingly. However let me split these.
| + | ||
| + # Perform per distribution post-installation steps if necessary | ||
| + case "$SPREAD_SYSTEM" in | ||
| + opensuse-*) |
zyga
Jun 8, 2017
Contributor
We could query systemd preset policy instead of maintaining a switch statement.
| @@ -66,6 +66,42 @@ fedora_build_rpm() { | ||
| rm $GOPATH/*.src.rpm | ||
| } | ||
| +opensuse_build_rpm() { | ||
| + release=$(echo "$SPREAD_SYSTEM" | awk '{split($0,a,"-");print a[2]}') |
zyga
Jun 8, 2017
Contributor
Could this just come from /etc/os-release? I'm not sure why we're encoding it in SPREAD_SYSTEM
morphis
Jun 8, 2017
Contributor
It's a reliable identifier we own. /etc/os-release isn't really owned by us. Would like to keep these influence out of the picture when we're doing spread specific things like setting up the environment.
| + sed -i -e "s/^Version:.*$/Version: $version/g" packaging/opensuse-$release/snapd.spec | ||
| + | ||
| + mkdir -p /tmp/pkg/snapd-$version | ||
| + cp -rav * /tmp/pkg/snapd-$version/ |
morphis
Jun 8, 2017
Contributor
A osc build isn't possible without a login. Also I have code to unite the rpm build for Fedora and openSUSE under a single build_rpm function.
| EOF | ||
| + systemctl daemon-reload |
added some commits
Jun 8, 2017
Some checks were not successful
Some checks were not successful
Some checks were not successful
| @@ -171,7 +173,7 @@ install -m 644 -D packaging/opensuse-42.2/permissions %buildroot/%{_sysconfdir}/ | ||
| install -m 644 -D packaging/opensuse-42.2/permissions.paranoid %buildroot/%{_sysconfdir}/permissions.d/snapd.paranoid | ||
| # Install the systemd units | ||
| make -C data/systemd install DESTDIR=%{buildroot} SYSTEMDSYSTEMUNITDIR=%{_unitdir} | ||
| -for s in snapd.autoimport.service snapd.system-shutdown.service; do | ||
| +for s in snapd.autoimport.service snapd.system-shutdown.service snap-repair.timer snap-repair.service; do |
zyga
Jun 8, 2017
Contributor
I was under the impression that the repair functionality is only applicable to core systems. I would not recommend using it in any classic distribution where existing packaging systems are sufficient to recover from any issue.
morphis
Jun 8, 2017
Contributor
That is exactly why we're rm these system units here after they got installed by data/systemd/Makefile :-)
| @@ -1,5 +1,4 @@ | ||
| summary: Check snap ack | ||
| -systems: [-ubuntu-core-16-arm-*] |
morphis commentedMay 29, 2017
This is currently based on #3365 so you will see the changes for Fedora here too until that PR is merged.