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, spread: add Arch Linux to CI systems #4285
Conversation
bboozzoo
added some commits
Nov 23, 2017
| @@ -237,6 +277,9 @@ if [ -z "$SNAPD_PUBLISHED_VERSION" ]; then | ||
| fedora-*|opensuse-*) | ||
| build_rpm | ||
| ;; | ||
| + arch-*) | ||
| + build_pkg |
zyga
Nov 23, 2017
Contributor
Can we please call this build_arch_pkg, people may find this confusing as package is a generic term.
bboozzoo
added some commits
Nov 23, 2017
|
We are still running on the Linode kernel: |
bboozzoo
added some commits
Nov 24, 2017
bboozzoo
referenced this pull request
Nov 24, 2017
Merged
tests/main/manpages: set LC_ALL=C as man may complain if the locale is unset or unsupported #4295
| @@ -339,13 +345,13 @@ prepare: | | ||
| # NOTE: At this stage the source tree is available and no more special | ||
| # considerations apply. | ||
| - "$TESTSLIB"/prepare-restore.sh --prepare-project | ||
| + ( . "$TESTSLIB"/prepare-restore.sh --prepare-project ) |
bboozzoo
Nov 24, 2017
Contributor
We need to do a system upgrade in Arch in order to get a consistent system state. I figured that moving this operation into project prepare was the best fit (see 7773050 for implementation). However, there is a slight complication, in order to do a reboot properly, we use the REBOOT command which is only defined at the top level spread prepare|restore.. and so on, so it's not visible in prepare-restore.sh which is executed in a separate shell. To work around this problem, I've opted for sourcing the script instead.
Other options are to move the upgrade piece to prepare block, or use some special exit code in prepare-restore.sh to indicate that reboot is needed.
mvo5
Nov 27, 2017
Collaborator
Could you please add a comment (copy/paste this here is probably fine) about the details why this is done in the way you do it?
| +distro_upgrade() { | ||
| + case "$SPREAD_SYSTEM" in | ||
| + arch-*) | ||
| + if pacman -Syu --noconfirm 2>&1 | grep -q "there is nothing to do" ; then |
bboozzoo
Nov 24, 2017
Contributor
MATCH is only defined in the shell that runs the script defined in spread.yaml or task.yaml. If pkgdb.sh is used in separate shell, the function may not be defined.
zyga
Nov 24, 2017
Contributor
I have a solution for missing MATCH that came out of my refactoring. I have it ready but I'll only post it on Monday to let you guys rest too :)
bboozzoo
added some commits
Nov 24, 2017
| @@ -74,6 +74,9 @@ backends: | ||
| - opensuse-42.2-64: | ||
| workers: 3 | ||
| manual: true | ||
| + - arch-2017.07.01: | ||
| + workers: 1 |
mvo5
reviewed
Nov 27, 2017
Thanks a driving this, really great to see arch in the CI. Some questions/suggestion inline.
| @@ -339,13 +345,13 @@ prepare: | | ||
| # NOTE: At this stage the source tree is available and no more special | ||
| # considerations apply. | ||
| - "$TESTSLIB"/prepare-restore.sh --prepare-project | ||
| + ( . "$TESTSLIB"/prepare-restore.sh --prepare-project ) |
bboozzoo
Nov 24, 2017
Contributor
We need to do a system upgrade in Arch in order to get a consistent system state. I figured that moving this operation into project prepare was the best fit (see 7773050 for implementation). However, there is a slight complication, in order to do a reboot properly, we use the REBOOT command which is only defined at the top level spread prepare|restore.. and so on, so it's not visible in prepare-restore.sh which is executed in a separate shell. To work around this problem, I've opted for sourcing the script instead.
Other options are to move the upgrade piece to prepare block, or use some special exit code in prepare-restore.sh to indicate that reboot is needed.
mvo5
Nov 27, 2017
Collaborator
Could you please add a comment (copy/paste this here is probably fine) about the details why this is done in the way you do it?
| +distro_upgrade() { | ||
| + case "$SPREAD_SYSTEM" in | ||
| + arch-*) | ||
| + if pacman -Syu --noconfirm 2>&1 | grep -q "there is nothing to do" ; then |
bboozzoo
Nov 24, 2017
Contributor
MATCH is only defined in the shell that runs the script defined in spread.yaml or task.yaml. If pkgdb.sh is used in separate shell, the function may not be defined.
zyga
Nov 24, 2017
Contributor
I have a solution for missing MATCH that came out of my refactoring. I have it ready but I'll only post it on Monday to let you guys rest too :)
| + -e "s/pkgver=.*/pkgver=$version/" \ | ||
| + -e "s/package_snapd-git()/package_snapd()/" \ | ||
| + /tmp/pkg/PKGBUILD | ||
| + awk ' |
mvo5
Nov 27, 2017
Collaborator
Hm, is this really the only way? It looks really hard to read, what are the inputs/outputs here?
bboozzoo
Nov 29, 2017
Contributor
We need to comment out this whole pkgver section: https://github.com/bboozzoo/snapd/blob/a3a463199cc12b4dbb70e92339f7c45c518317eb/packaging/arch/PKGBUILD#L33 then
update source=("git+ssh://...") to point to tar.gz, rename package_snapd-git() to package_snapd() and rename the pacakge to snapd.
| @@ -12,6 +12,16 @@ prepare: | | ||
| echo "Having installed the test snap" | ||
| . $TESTSLIB/snaps.sh | ||
| install_local test-snapd-tools | ||
| + # Arch does not use /etc/alternatives | ||
| + if [[ "$SPREAD_SYSTEM" == arch-* ]]; then | ||
| + mkdir /etc/alternatives |
mvo5
Nov 27, 2017
Collaborator
Hm, I get the feeling this may hide a bug in our code? Shouldn't we fail gracefully if this dir does not exists? cc @zyga
zyga
Nov 27, 2017
Contributor
I think so though perhaps some tests are overzealous. What did you see @bboozzoo ?
bboozzoo
Nov 27, 2017
Contributor
DIRECTORY/alternatives fails because it tries to get a inode of this directory in the host's filesystem and fails as there is no such directory on Arch.
codecov-io
commented
Nov 28, 2017
•
Codecov Report
@@ Coverage Diff @@
## master #4285 +/- ##
==========================================
- Coverage 78.05% 78.03% -0.02%
==========================================
Files 450 450
Lines 30899 30899
==========================================
- Hits 24118 24113 -5
- Misses 4772 4776 +4
- Partials 2009 2010 +1
Continue to review full report at Codecov.
|
|
Another Arch curiosity:
Turns out |
bboozzoo commentedNov 23, 2017
•
Edited 1 time
-
bboozzoo
Nov 23, 2017
This is an early PR adding support for Arch Linux in the CI pipeline. Hopefully this will allow to catch the cross-distro incompatibilities sooner.
Related forum topic: https://forum.snapcraft.io/t/integrate-arch-linux-into-ci-pipeline/2904