Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move build system back to Trusty (with fixes) #1115

Merged
merged 5 commits into from
Oct 24, 2017

Conversation

phoerious
Copy link
Member

Description

This patch resolves #1114 by reverting our AppImage build system back to Ubuntu 14.04, but with custom builds for libyubikey and libykpers-1. It also ships a required code fix to prevent the YubiKey from appearing twice in the menu (only happens when building with 14.04, newer Ubuntu versions are not affected, but it may also be a problem of a specific Qt version).

Additionally, this patch resolves #691 by restricting our global menu workaround to Qt versions older than 5.9.0.

Motivation and context

Moving to CentOS solved a few problems. It cleaned up our Dockerfile and didn't necessarily require a newer Qt version and YubiKeys worked out of the box. Unfortunately, AppImages built with CentOS don't integrate with Ubuntu's global menu (at least not with the standard Qt 5.6) and don't run on older Ubuntu-based systems (we expected that).
The loss of compatibility would be an okay trade, if there were no way to enable (bug-free) YubiKey support for Trusty-built AppImages. But with a bit more tinkering and a few additional code fixes, I managed to do exactly that, so there is no more reason to keep our CentOS build system.

I would also suggest to provide a new 2.2.2 build which was built with the fixed Ubuntu 14.04 system (but with the 2.2.2 code base, of course, accepting that YubiKeys are detected twice). We cannot replace already published binaries, but we can add an additional AppImage build to the release page and link that from the website instead of the previous CentOS-based AppImage. An upcoming 2.2.3 (or 2.3.0, whichever comes first) may then also ship the code fixes.

How has this been tested?

AppImage runs fine on Ubuntu 16.10 and 14.04, YubiKey is detected successfully and only once.
Global menu is working in Unity.

Types of changes

  • ✅ Bug fix (non-breaking change which fixes an issue)

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@phoerious phoerious added this to the v2.2.3 milestone Oct 24, 2017
@phoerious phoerious requested a review from a team October 24, 2017 14:53
@phoerious phoerious changed the title Hotfix/issue 1114 appimage compatibility Move build system back to Trusty (with fixes) Oct 24, 2017
@phoerious phoerious force-pushed the hotfix/issue-1114-appimage-compatibility branch from f052dcb to 3f3b4ed Compare October 24, 2017 15:12
@phoerious phoerious merged commit 83fd387 into release/2.2.3 Oct 24, 2017
@phoerious phoerious deleted the hotfix/issue-1114-appimage-compatibility branch October 24, 2017 15:58
droidmonkey added a commit that referenced this pull request Dec 12, 2017
- Prevent database corruption when locked [#1219]
- Fixes apply button not saving new entries [#1141]
- Switch to Consolas font on Windows for password edit [#1229]
- Multiple fixes to AppImage deployment [#1115, #1228]
- Fixes multiple memory leaks [#1213]
- Resize message close to 16x16 pixels [#1253]
phoerious added a commit that referenced this pull request Dec 13, 2017
- Prevent database corruption when locked [#1219]
- Fixes apply button not saving new entries [#1141]
- Switch to Consolas font on Windows for password edit [#1229]
- Multiple fixes to AppImage deployment [#1115, #1228]
- Fixes multiple memory leaks [#1213]
- Resize message close to 16x16 pixels [#1253]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants