Skip to content
This repository has been archived by the owner on Dec 18, 2022. It is now read-only.

[Testing] Attempt to speedup flatpak builds #609

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions .github/workflows/cmake_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,38 @@ defaults:
shell: bash

jobs:

skip_test:
name: "CI Build Skipping Logic"
runs-on: ubuntu-latest
permissions:
actions: write
checks: read
contents: read
deployments: read
issues: read
discussions: read
packages: read
pull-requests: read
repository-projects: read
security-events: read
statuses: read
outputs:
should_skip: ${{ steps.skip_check.outputs.should_skip }}
steps:
- id: skip_check
uses: fkirc/skip-duplicate-actions@master
with:
paths: '[]'
paths_ignore: '["**/**.md", "**/**.dox2", "**/**.dox", "**/**.dox.in", "**/LICENSE.txt", "/.builds/**", "/.github/ISSUE_TEMPLATE/**", "/.github/funding.yml", "/.vscode/**"]'
do_not_skip: '["workflow_dispatch", "schedule"]'
cancel_others: 'true'
skip_after_successful_duplicate: 'true'
concurrent_skipping: 'same_content_newer'

build:
needs: skip_test
if: ${{ needs.skip_test.outputs.should_skip != 'true' }}
name: ${{ matrix.config.name }}
runs-on: ${{ matrix.config.os }}
strategy:
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/flatpak.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ name: Packaging
on:
push:
branches: [master]
pull_request:
workflow_dispatch:

jobs:
flatpak:
name: "Flatpak"
Expand Down
4 changes: 3 additions & 1 deletion cmake-modules/FindVampHostSDK.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ find_path(VampHostSDK_INCLUDE_DIR
mark_as_advanced(VampHostSDK_INCLUDE_DIR)

find_library(VampHostSDK_LIBRARY
NAMES vamp-hostsdk
# The library has different file names on Windows (VampHostSDK) compared
# to Linux and macOS (vamp-hostsdk).
NAMES vamp-hostsdk VampHostSDK
PATHS ${PC_VampHostSDK_LIBRARY_DIRS}
DOC "VampHostSDK library"
)
Expand Down
28 changes: 14 additions & 14 deletions packaging/flatpak/org.tenacity.Tenacity.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,18 @@
"modules": [
{
"name": "wxwidgets",
"rm-configure": true,
"buildsystem": "cmake-ninja",
"config-opts": [
"--with-libpng",
"--with-zlib",
"--with-cxx=17",
"--disable-sdltest",
"--disable-webview",
"--disable-webviewwebkit",
"--disable-ribbon",
"--disable-propgrid",
"--disable-richtext",
"--with-expat=builtin",
"--with-libiconv=/usr"
"-D wxUSE_LIBPNG=builtin",
"-D wxUSE_ZLIB=builtin",
Comment on lines +46 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here about builtin vs sys

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From some older aarch64 flatpak CI build log I see this:

   Which libraries should wxWidgets use?
                                       STL                no
                                       jpeg               sys
                                       png                sys
                                       regex              builtin
                                       tiff               sys
                                       lzma               yes
                                       zlib               sys
                                       expat              builtin
                                       libmspack          no
                                       sdl                no

You are changing it from using system libpng and zlib, which are already built and available from the SDK, to bundled copies, increasing build time from sub-configuring and compiling those.

"-D wxBUILD_CXX_STANDARD=17",
"-D wxBUILD_TESTS=OFF",
"-D wxUSE_WEBVIEW=OFF",
"-D wxUSE_WEBVIEW_WEBKIT=OFF",
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be other things we can probably disable that we don't use, out of wx components. I can have a look later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all these can be disabled:

wxUSE_LIBGNOMEVFS (probably gets disabled automatically due to no legacy gnome-vfs in flatpak SDK, might be good to be explicit)
wxUSE_WEBVIEW
wxUSE_WEBVIEW_WEBKIT
wxUSE_RIBBON
wxUSE_PROPGRID
wxUSE_AUI
wxUSE_STC
wxUSE_MDI
wxUSE_MEDIACTRL
wxUSE_RICHTEXT

For some reason we seem to do something about wxXML and XRC in toplevel cmake file, but I can't spot any actual usage of what would need wxUSE_XML and/or wxUSE_XRC.
We could probably disable some image formats too, like TIFF overall, but we can fine-tune later, it doesn't save much if using system library from SDK. The above are the "big ticket" items.

Copy link
Member

@leio leio Sep 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these might be useful for non-flatpak CI builds too, where we build wx ourselves too (Ubuntu). For example disabling wxUSE_MEDIACTRL gets rid of the need for gstreamer completely.

"-D wxUSE_RIBBON=OFF",
"-D wxUSE_PROPGRID=OFF",
"-D wxUSE_RICHTEXT=OFF",
"-D wxUSE_EXPAT=builtin"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect expat is available in some flatpak SDK we depend on anyways, so system version could probably be used as well.

],
"cleanup": [
"/share/bakefile"
Expand Down Expand Up @@ -161,6 +160,7 @@
},
{
"name": "soundtouch",
"buildsystem": "cmake-ninja",
"sources": [
{
"type": "git",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're at it, can you update soundtouch to 2.3.1? It has some fixes for CMake. I'm not sure any of those are required for Tenacity, but still good to be on the latest version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I had already noticed that version difference and was going to look into seeing if there were any breaking changes before doing exactly that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SoundTouch 2.3.1 has some build fixes for Mixxx. No changes should be required for Tenacity.

Expand All @@ -185,8 +185,8 @@
"buildsystem": "cmake-ninja",
"builddir": true,
"config-opts": [
"-DCMAKE_BUILD_TYPE=RelWithDebInfo",
"-DWX_CONFIG=/app/bin/wx-config"
"-D CMAKE_BUILD_TYPE=RelWithDebInfo",
"-D WX_CONFIG=/app/bin/wx-config"
],
"post-install": [
"install -Dm644 ../help/tenacity.metainfo.xml -t /app/share/metainfo",
Expand Down
2 changes: 1 addition & 1 deletion vcpkg
Submodule vcpkg updated from f8d74b to 9c84b3