Use downstream packaging in spread tests #103

Merged
merged 9 commits into from Aug 16, 2016

Conversation

Projects
None yet
2 participants
Collaborator

zyga commented Aug 15, 2016

This patch changes spread setup phase to build the native package (for
Ubuntu and soon for Debian) in a way that is more consistent with the
actual release process. This should help us release with confidence as
all CI tests will run with real downstream packaging applied to real
upstream tarball and build in an isolated manner with sbuild.

This also paves the way towards the removal of the debian/ packaging
from the upstream repository. This will be done in a separate pull
request to avoid clutter.

The spread prepare stage was moved to .spread-prepare.sh so that it can
be shell-checked more easily.

The debian-8 spread target was removed because currently the downstream
packaging is not compatible. Debian maintainers were notified to the
issue so that we should be able to re-enable Debian (sid) support soon.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

Use downstream packaging in spread tests
This patch changes spread setup phase to build the native package (for
Ubuntu and soon for Debian) in a way that is more consistent with the
actual release process. This should help us release with confidence as
all CI tests will run with real downstream packaging applied to real
upstream tarball and build in an isolated manner with sbuild.

This also paves the way towards the removal of the debian/ packaging
from the upstream repository. This will be done in a separate pull
request to avoid clutter.

The spread prepare stage was moved to .spread-prepare.sh so that it can
be shell-checked more easily.

The debian-8 spread target was removed because currently the downstream
packaging is not compatible. Debian maintainers were notified to the
issue so that we should be able to re-enable Debian (sid) support soon.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
.spread-prepare.sh
+
+
+# Create source distribution tarball and place it in the top-level directory.
+create_dist_tarball() {
@mvo5

mvo5 Aug 16, 2016

Contributor

This appears to be useful outside of spread as well, maybe it could be a toplevel release.sh script?

@zyga

zyga Aug 16, 2016

Collaborator

Sure!

.spread-prepare.sh
@@ -0,0 +1,226 @@
+#!/bin/sh
+# This script is started by spread to prepare the execution environment
+set -x
@mvo5

mvo5 Aug 16, 2016

Contributor

(nitpick^2) why multiple lines, set -eux seems to be ok?

@zyga

zyga Aug 16, 2016

Collaborator

Just my habit, I'll combine those into one line :)

.spread-prepare.sh
+# Create source distribution tarball and place it in the top-level directory.
+create_dist_tarball() {
+ # Load the version number from a dedicated file
+ local pkg_version=
@mvo5

mvo5 Aug 16, 2016

Contributor

I wonder if we could combine this and the following line to local pkg_version=$(cat "$top_dir/VERSION). Using local in this way should be fine with dash https://wiki.ubuntu.com/DashAsBinSh#local - interessting enough local is not posix compliant, but I think we can ignore this :)

@zyga

zyga Aug 16, 2016

Collaborator

I did this because of spellcheck, otherwise the error code from cat is lost. I don't mind it either way but the current code is spellcheck clean which feels good.

@mvo5

mvo5 Aug 16, 2016

Contributor

Oh, indeed, thanks. That is a good reason.

.spread-prepare.sh
+
+ # Create a scratch space to run configure
+ scratch_dir="$(mktemp -d)"
+ trap 'rm -rf "$scratch_dir"' EXIT
@mvo5

mvo5 Aug 16, 2016

Contributor

I see multiple trap usages in this script, they will override each other, a helper like https://github.com/Debian/apt/blob/master/test/integration/framework#L247 may help here for multiple traps.

@zyga

zyga Aug 16, 2016

Collaborator

Hmm, I assumed that traps in functions are okay (since they run in sub shells). Perhaps I was wrong. Let me check this in more detail. If anything there could be one trap for all things that need cleanup.

.spread-prepare.sh
+ make dist
+
+ # Ensure we got the tarball we were expecting to see
+ test -f "snap-confine-$pkg_version.tar.gz"
@mvo5

mvo5 Aug 16, 2016

Contributor

I think this one is redundant (nice but redundant) because the mv below will already fail if the file is not there.

@zyga

zyga Aug 16, 2016

Collaborator

Yep, the move was added later (lots of internal cycles on this script). I'l get rid of it.

.spread-prepare.sh
+# build Ubuntu binary package and place it in $top_dir
+# $1 = /etc/os-release ID field
+# $2 = /etc/os-release VERSION_ID field (possibly empty!)
+build_debian_or_ubuntu_package() {
@mvo5

mvo5 Aug 16, 2016

Contributor

This function is very long. This makes me unhappy, I'm exploring some ideas right now what could be done about it.

.spread-prepare.sh
+ esac
+
+ # Ensure that we have a sbuild chroot ready
+ if ! schroot -l | grep "chroot:${distro_codename}-.*-sbuild"; then
@mvo5

mvo5 Aug 16, 2016

Contributor

While sbuild is as close as you can get to the real buildds, I'm not sure if its not overkill. given that we run in a fresh linode machine and all. That would also allow us to remove the sbuild keys. But maybe going too far here, happy to discuss this.

@zyga

zyga Aug 16, 2016

Collaborator

This just keeps us honest. I think if the price (in terms of time spent) is small enough it is just the right thing to do. Fedora build will similarly use mock (which takes around a minute) to build the package.

The goal is to avoid "uhhh" moments and stress when we actually release.

.spread-prepare.sh
+ cd "$scratch_dir"
+
+ # Fetch the current Ubuntu packaging for the appropriate release
+ git clone -b "$distro_packaging_git_branch" "$distro_packaging_git" distro-packaging
@mvo5

mvo5 Aug 16, 2016

Contributor

We can probably use gbp buildpackage here (which is hopefully closer to what we will do on a real release).

Contributor

mvo5 commented Aug 16, 2016

Thnaks for merging my changes! I think it would be nice to extract release.sh (or similar name, whatever you prefer) and then we can continue to iterate. I think its getting nicer already.

zyga added some commits Aug 16, 2016

Abbreviate shell 'set' statements
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Split-off release.sh from spread-prepare.sh
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Improve error handling in spread-prepare.sh
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit 3eb3362 into master Aug 16, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zyga zyga deleted the improve-ci-system branch Aug 16, 2016

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