CI: Enable Qt5 + lightpreview #406

jonathanlinat · 2024-01-07T05:07:16Z

This PR primarily focuses on facilitating the building and releasing of lightpreview for all operating systems using Qt5.

Additionally, it enhances the runners, updates the external actions, and improves the clarity of the existing steps.

jonathanlinat · 2024-01-07T07:38:22Z

@ericwa It seems you need to approve this PR to let GitHub Actions execute the corresponding workflow and validate that everything works as expected.

.github/workflows/cmake.yml

.github/workflows/continuous-building.yml

jonathanlinat · 2024-01-07T17:36:56Z

@ericwa I'll take a look tonight and get back to you.

we are deploying from GitHub Actions now

ericwa · 2024-01-07T18:23:51Z

I downloaded the windows build and tested lightpreview and it seems to work fine, so that's a good start! We can drop the use of appveyor for releases now. We could remove appveyor entirely - it takes about 2x as long to build as GH actions does. But GH actions is less convenient for people trying out dev builds - requires login, the .zips are wrapped in multiple layers of zip and extra folders, etc. But I also don't like maintaining two different flavors of Windows build (GH actions / appveyor) and splitting testing between them - it means we might miss an issue in a Windows GH release that didn't show up in the Appveyor dev builds for some reason (e.g. different compiler version), so that's an argument for dropping appveyor entirely.

I'll test the Linux artifact in a VM in a bit.

For the mac one, I don't think lightpreview will work because we're not copying the Qt libraries into the archive. We don't need to fix it right now.. I think it's going to be hard to get lightpreview packaging unless I get a new mac.

ericwa · 2024-01-07T18:45:12Z

Seems like the linux build is failing because CPack is packaging the archive as merge-Linux.zip

CPack: Create package using ZIP
CPack: Install projects
CPack: - Run preinstall target for: ericw-tools
CPack: - Install project: ericw-tools []
CPack: Create package
CPack: - package: /home/runner/work/ericw-tools/ericw-tools/build-linux/merge-Linux.zip

and then later in the build-linux-64.sh we try to unzip it as: unzip -X ericw-tools-*.zip

to hopefully not break on PR builds

ericwa · 2024-01-07T22:21:51Z

The linux build is looking good now. I'm testing it on Ubuntu 23.04 since that's the VM I had available, and the lightpreview binary works (using the system copy of Qt 5).

jonathanlinat · 2024-01-07T22:39:41Z

@ericwa That's great and I am glad it works. 👌🏻

I am pretty sure we can replicate the same for macOS.

We can probably ask someone on Discord to test the macOS version of the tool.

jonathanlinat · 2024-01-07T23:31:05Z

The linux build is looking good now. I'm testing it on Ubuntu 23.04 since that's the VM I had available, and the lightpreview binary works (using the system copy of Qt 5).

I confirm it works (22.04).

jonathanlinat · 2024-01-08T01:42:51Z

[...] the .zips are wrapped in multiple layers of zip and extra folders [...]

Regarding the nested ZIP files, I can take a look at them, if that's okay with you. I've made the corresponding changes to my starter kit to ensure the doc and bin directories are the direct content of the downloadable assets. It's a relatively easy task to achieve.

we don't use sha256sum anymore

on my macOS test system, running cpack separately is doubling the build time

…gets

ericwa · 2024-01-08T08:02:17Z

[...] the .zips are wrapped in multiple layers of zip and extra folders [...]

Regarding the nested ZIP files, I can take a look at them, if that's okay with you. I've made the corresponding changes to my starter kit to ensure the doc and bin directories are the direct content of the downloadable assets. It's a relatively easy task to achieve.

Sure, if it's easy, thanks!

Finally got the CI to produce a working lightpreview.app (this is on 10.15, x86_64):

Also fixed the command-line tools which had been broken on macOS since 2.0.0-alpha1 due to an missing rpath setting.

I'll confirm tomorrow that I didn't break the Linux/Windows builds.
Other than that I think this is good to merge whenever you're ready.

jonathanlinat · 2024-01-08T18:54:30Z

@ericwa I introduced the additional steps to re-pack the binaries as proposed.

Additionally, I decided to remove the last step from this PR. I'll create another PR with a new workflow to include a release job (continuous-releasing) for better organization, as I did in my starter kit.

https://github.com/jonathanlinat/quake-leveldesign-starterkit/blob/main/.github/workflows/continuous-releasing.yml

jonathanlinat · 2024-01-08T20:31:58Z

Here we go.

Ref. https://github.com/jonathanlinat/ericw-tools/actions/runs/7452439710

I would also recommend adding, in another PR, an extra step for each system to include the README and LICENSE files from the repository in the artifacts (ZIP file), together with the doc and bin directories.

ericwa · 2024-01-09T02:43:56Z

all good if I squash and merge @jonathanlinat ?

jonathanlinat · 2024-01-09T03:15:45Z

all good if I squash and merge @jonathanlinat ?

Absolutely.

ericwa · 2024-01-09T03:17:34Z

thanks for this!

* chore(CI): refine current 'cmake.yml' file content * chore(CI): enable Qt5 on Linux and macOS * Appveyor.yml: remove deploy step we are deploying from GitHub Actions now * build-linux-64.sh: adjust .zip wildcard to hopefully not break on PR builds * build-linux-64.sh: remove fragile unzip/readelf lines * lightpreview\CMakeLists.txt: only install Qt on Win/Mac * lightpreview: attempt to fix mac Qt packaging * lightpreview: apple fixes * Install qtdbus on macOS * try alternate dbus module name * try cmake find_package for dbus * also find Qt5PrintSupport * build-osx.sh: remove coreutils install we don't use sha256sum anymore * lightpreview/CMakeLists.txt: create .app bundle on macOS * Update CMakeLists.txt * build-*: run cpack as part of the build command on my macOS test system, running cpack separately is doubling the build time * cmake: factor out add_loader_path_to_rpath function, apply to all targets * chore(CI): add steps to re-pack the 'doc' and 'bin' directories * chore(CI): remove GitHub release-related step (to be re-introduced) * chore(CI): adjust inflating destination directories --------- Co-authored-by: Eric Wasylishen <[email protected]>

jonathanlinat added 2 commits January 6, 2024 21:51

chore(CI): refine current 'cmake.yml' file content

ffeaf29

chore(CI): enable Qt5 on Linux and macOS

2705828