This repository has been archived by the owner. It is now read-only.

Gracefully handle packages breaking because of soname bumps #1116

Merged
merged 6 commits into from Jan 14, 2018

Conversation

@ollieparanoid
Member

ollieparanoid commented Jan 12, 2018

Fixes #893. I ended up writing a lot here in the description in the hope that this makes it easier to understand the problem and how it's solved. Feedback whether that is good or bad is welcome (but actual reviewing is more important 馃槈).

Introduction: soname bumps break our packages

Linux programs as packaged in Alpine (and most other distributions) link against dynamic libraries (.so files). These libraries may release incompatible upgrades, which "bump" (increase) the version of the library file (the "soname").

Whenever you build a package with Alpine's abuild (directly or e.g. via pmbootstrap build), and it uses such dynamic libraries, abuild will automatically scan for those and add the exact soname as dependency.

Here's one example. All depends starting with so: are such sonames:

$ pmbootstrap parse_apkindex ~/.local/var/pmbootstrap/packages/x86_64/APKINDEX.tar.gz testapp
{
    "pkgname": "testapp",
    "version": "0-r1",
    "arch": "x86_64",
    "timestamp": "1500000000",
    "depends": [
        "testlib",
        "so:libc.musl-x86_64.so.1",
        "so:libtestlib.so.0"
    ],
    "provides": [
        "cmd:testapp"
    ]
}

postmarketOS uses its own binary package repository together with the ones from Alpine. That means: When a postmarketOS package links against a dynamic library from Alpine, it's currently possible that said library gets a soname bump in Alpine, and then you can't install the binary postmarketOS package anymore because it was built against the old version.

Alpine's apk package manager will write an error - but it is not obvious how the user can work around that if we didn't notice it and fix it in the binary repo: You'd need to mark the postmarketOS aport, which has the outdated dependency, for rebuild. To do it properly, one would increase the pkgrel (that's also how Alpine handles rebuilds for soname upgrades, I've asked in their IRC channel).

For reference, this is what apk says when a dependency had a soname bump:

ERROR: unsatisfiable constraints:
  so:libtestlib.so.1 (missing):
    required by:
                 testapp-1.0-r0[so:libtestlib.so.1]
                 testapp-1.0-r0[so:libtestlib.so.1]

Changes

New command: pmbootstrap pkgrel_bump --auto

Automatically find all packages from the postmarketOS binary repository (disable with pmbootstrap --mirror-pmOS="") as well as the locally built packages, that are broken because of soname bumps. Increase the pkgrel in each package's APKBUILD to mark it for rebuilt.

Variations

  • pmbootstrap pkgrel_bump --auto --dry: dry run mode, doesn't change anything and yields a non-zero exit code in case there's a package that would be bumped (this was --auto-dry before and got changed, see below)
  • pmbootstrap pkgrel_bump hello-world: bump the pkgrel of one specific package without checking the soname dependencies, useful for related problem #978

Automatic check before installing packages

Before any package gets installed (e.g. during pmbootstrap install, which generates a full system image), each package and its dependencies are checked against being broken because of soname bumps. When that is the case, we abort and ask the user to run pmbootstrap pkgrel_bump --auto.

$ pmbootstrap install --add=testapp
*** (1/5) PREPARE NATIVE CHROOT ***
*** (2/5) CREATE DEVICE ROOTFS ("qemu-amd64") ***
NOTE: Run 'pmbootstrap pkgrel_bump --auto' to mark packages with outdated dependencies for rebuild. This will most likely fix this issue (soname bump?).
NOTE: More dependency calculation logging with 'pmbootstrap -v'.
ERROR: Could not find package 'so:libtestlib.so.1' in the aports folder and could not find it in any APKINDEX.
Run 'pmbootstrap log' for details.
See also: <https://postmarketos.org/troubleshooting>

Testcases

  • test_soname_bump.py: executes the dry run, fails if it finds broken packages (so we check for breakage in our binary packages repo whenever a new pull request comes in, usually at least once per day, example output).
  • test_soname_bump.py: builds the new test aports testlib and testapp from test/testdata/pkgrel_bump/aports, and creates the soname breakage on purpose to verify that pkgrel_bump --auto works as intended.

How to test

After reviewing the code, please create the soname problem locally, and verify that pmbootstrap properly detects it at installation time:

# Copy testlib and testapp to aports, so we can build them
cp -r test/testdata/pkgrel_bump/aports aports/_temp

# Selet the qemu-amd64 device (because it uses x86_64 packages) and delete
# all chroots. Then build testapp and testlib (gets pulled in as dependency)
pmbootstrap zap
pmbootstrap config device qemu-amd64
pmbootstrap build testapp

# Bump the pkgrel of testlib. The package is written in a way that this will
# also increase the soname version.
pmbootstrap pkgrel_bump testlib

# This should rebuild testlib
pmbootstrap build testlib

# Delete the old testlib package to break testapp intentionally
sudo rm ~/.local/var/pmbootstrap/packages/x86_64/testlib-1.0-r0.apk
pmbootstrap index

# Perform a normal installation and ask pmbootstrap to add testapp. It should
# complain that the package depends on the missing "so:libtestlib.so.0" and
# abort the installation
pmbootstrap install --add=testapp

# This should fix the problem by increasing the testapp pkgrel
pmbootstrap pkgrel_bump --auto
pmbootstrap install --add=testapp

# Clean up
rm -r aports/_temp
sudo rm ~/.local/var/pmbootstrap/packages/x86_64/testlib*
sudo rm ~/.local/var/pmbootstrap/packages/x86_64/testapp*
pmbootstrap index

ollieparanoid added some commits Jan 12, 2018

Gracefully handle packages breaking because of soname bumps
Fixes #893. Changes:
* New action: "pmbootstrap pkgrel_bump"
* pmbootstrap detects missing soname depends when trying to install
  anyting, and suggests "pkgrel_bump --auto" to fix it
* Testcase test_soname_bump.py checks the pmOS binary package repo
  for soname breakage, so we see it when CI runs for new PRs
test_pkgrel_bump.py: "zap" instead of "apk del"
"apk del testlib" doesn't return 0 when testlib was not installed.
This caused the testcase to fail on Travis.
@MartijnBraam

I've reviewed the code and everything seems ok, the only thing I can think of is that its maybe neater to have a seperate --dry and --auto instead of --auto and --auto-dry

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Jan 13, 2018

Member

Thanks for the review! Yeah that's how I've tried to do it first, but it didn't seem to be possible with argparse to make --auto [--dry] mutually exclusive with the packages.

Now that I think about it, we could add the --dry parameter in general, so it works with both --auto and packages. I'll update the PR accordingly.

Member

ollieparanoid commented Jan 13, 2018

Thanks for the review! Yeah that's how I've tried to do it first, but it didn't seem to be possible with argparse to make --auto [--dry] mutually exclusive with the packages.

Now that I think about it, we could add the --dry parameter in general, so it works with both --auto and packages. I'll update the PR accordingly.

@MartijnBraam

This comment has been minimized.

Show comment
Hide comment
@MartijnBraam

MartijnBraam Jan 13, 2018

Member

Yeah thats what I meant, don't make them exclusive but make --dry a modifier for the current command as a lot of people will expect.

Member

MartijnBraam commented Jan 13, 2018

Yeah thats what I meant, don't make them exclusive but make --dry a modifier for the current command as a lot of people will expect.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Jan 13, 2018

Member

Updated. If you have time, please review again and test it 馃憤

Member

ollieparanoid commented Jan 13, 2018

Updated. If you have time, please review again and test it 馃憤

Show outdated Hide outdated pmb/helpers/frontend.py Outdated
@MartijnBraam

This comment has been minimized.

Show comment
Hide comment
@MartijnBraam

MartijnBraam Jan 13, 2018

Member

It seems to work good but I find this output not very nice:

$ ./pmbootstrap.py pkgrel_bump testlib --dry
[00:51:31] Increase 'testlib' pkgrel (3 -> 4)
[00:51:31] ERROR: pkgrels of package(s) would have been bumped!
[00:51:31] Run 'pmbootstrap log' for details.
[00:51:31] See also: <https://postmarketos.org/troubleshooting>

I also noticed I can't build testapp with --force after bumping the testlib but thats probably not an issue with the bumping code.

(004033) [00:55:39] Calculate depends of packages ['testlib'], arch: x86_64
(004033) [00:55:40] (native) build x86_64/testapp-1.0-r0.apk
(004033) [00:55:40] (native) % rm -rf /home/pmos/build
(004033) [00:55:40] % sudo cp -r /home/martijn/Projects/Other/pmbootstrap/aports/_temp/testapp/ /home/martijn/.local/var/pmbootstrap/chroot_native/home/pmos/build
(004033) [00:55:40] (native) % chown -R pmos:pmos /home/pmos/build
(004033) [00:55:40] (native) % cd /home/pmos/build && busybox su pmos -c 'CARCH=x86_64 abuild -d -f'
>>> testapp: abuild 3.1.0-r4
>>> testapp: Checking sanity of /home/pmos/build/APKBUILD...
>>> WARNING: testapp: No maintainer
>>> testapp: Cleaning temporary build dirs...
>>> testapp: Entering fakeroot...
/usr/bin/abuild: line 2554: ./testapp: not found
>>> ERROR: testapp*: check failed
>>> ERROR: testapp: all failed
Member

MartijnBraam commented Jan 13, 2018

It seems to work good but I find this output not very nice:

$ ./pmbootstrap.py pkgrel_bump testlib --dry
[00:51:31] Increase 'testlib' pkgrel (3 -> 4)
[00:51:31] ERROR: pkgrels of package(s) would have been bumped!
[00:51:31] Run 'pmbootstrap log' for details.
[00:51:31] See also: <https://postmarketos.org/troubleshooting>

I also noticed I can't build testapp with --force after bumping the testlib but thats probably not an issue with the bumping code.

(004033) [00:55:39] Calculate depends of packages ['testlib'], arch: x86_64
(004033) [00:55:40] (native) build x86_64/testapp-1.0-r0.apk
(004033) [00:55:40] (native) % rm -rf /home/pmos/build
(004033) [00:55:40] % sudo cp -r /home/martijn/Projects/Other/pmbootstrap/aports/_temp/testapp/ /home/martijn/.local/var/pmbootstrap/chroot_native/home/pmos/build
(004033) [00:55:40] (native) % chown -R pmos:pmos /home/pmos/build
(004033) [00:55:40] (native) % cd /home/pmos/build && busybox su pmos -c 'CARCH=x86_64 abuild -d -f'
>>> testapp: abuild 3.1.0-r4
>>> testapp: Checking sanity of /home/pmos/build/APKBUILD...
>>> WARNING: testapp: No maintainer
>>> testapp: Cleaning temporary build dirs...
>>> testapp: Entering fakeroot...
/usr/bin/abuild: line 2554: ./testapp: not found
>>> ERROR: testapp*: check failed
>>> ERROR: testapp: all failed
@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Jan 14, 2018

Member

It seems to work good but I find this output not very nice:

How would you do it?

I also noticed I can't build testapp with --force after bumping the testlib but thats probably not an issue with the bumping code.

Good catch! That's a bug in the APKBUILD of testapp then, we probably need to cd in check() to fix this. I'll update the PR accordingly.

Member

ollieparanoid commented Jan 14, 2018

It seems to work good but I find this output not very nice:

How would you do it?

I also noticed I can't build testapp with --force after bumping the testlib but thats probably not an issue with the bumping code.

Good catch! That's a bug in the APKBUILD of testapp then, we probably need to cd in check() to fix this. I'll update the PR accordingly.

@MartijnBraam

This comment has been minimized.

Show comment
Hide comment
@MartijnBraam

MartijnBraam Jan 14, 2018

Member

I think it shouldn't trigger the normal error troubleshooting lines in the dry run mode so it becomes:

$ ./pmbootstrap.py pkgrel_bump testlib --dry
[00:51:31] Increase 'testlib' pkgrel (3 -> 4)
[00:51:31] pkgrels of package(s) would have been bumped!

So its probably as easy as just calling exit instead of throwing an exception to change the exit code.

Member

MartijnBraam commented Jan 14, 2018

I think it shouldn't trigger the normal error troubleshooting lines in the dry run mode so it becomes:

$ ./pmbootstrap.py pkgrel_bump testlib --dry
[00:51:31] Increase 'testlib' pkgrel (3 -> 4)
[00:51:31] pkgrels of package(s) would have been bumped!

So its probably as easy as just calling exit instead of throwing an exception to change the exit code.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Jan 14, 2018

Member

Fixed both issues. Please mark it as reviewed if you think it's ready to be shipped. Thanks!

Member

ollieparanoid commented Jan 14, 2018

Fixed both issues. Please mark it as reviewed if you think it's ready to be shipped. Thanks!

@MartijnBraam

Seems to work fine now with the latest changes 馃殺

@ollieparanoid ollieparanoid merged commit 1992f37 into master Jan 14, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.008%) to 66.777%
Details

@ollieparanoid ollieparanoid deleted the fix/893-handle-soname-bumps branch Jan 14, 2018

PureTryOut added a commit that referenced this pull request Feb 21, 2018

Gracefully handle packages breaking because of soname bumps (#1116)
Fixes #893. Changes:
* New action: "pmbootstrap pkgrel_bump"
* pmbootstrap detects missing soname depends when trying to install
  anyting, and suggests "pkgrel_bump --auto" to fix it
* Testcase test_soname_bump.py checks the pmOS binary package repo
  for soname breakage, so we see it when CI runs for new PRs
* libsamsung-ipc: bump pkgrel because of soname bump
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.