ericwa / ericw-tools

Quake/Hexen 2 Map compiling tools - branch of http://disenchant.net/utils
http://ericwa.github.io/ericw-tools
GNU General Public License v2.0
329 stars 57 forks source link

CI: Enable Qt5 + lightpreview #406

Closed jonathanlinat closed 8 months ago

jonathanlinat commented 8 months ago

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 commented 8 months ago

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

jonathanlinat commented 8 months ago

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

ericwa commented 8 months ago

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 commented 8 months ago

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

ericwa commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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).

image

jonathanlinat commented 8 months ago

[...] 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.

ericwa commented 8 months ago

[...] 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): Screen Shot 2024-01-08 at 12 54 06 AM 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 commented 8 months ago

@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 commented 8 months ago

Here we go.

image

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 commented 8 months ago

all good if I squash and merge @jonathanlinat ?

jonathanlinat commented 8 months ago

all good if I squash and merge @jonathanlinat ?

Absolutely.

ericwa commented 8 months ago

thanks for this!