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

(draft) Github action addon #368

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

(draft) Github action addon #368

wants to merge 21 commits into from

Conversation

G-eos
Copy link
Contributor

@G-eos G-eos commented Apr 29, 2024

Add build from GitHub Action for MacOS-13 and Ubuntu.
This is a first trial, if you find it useful, please detail which platform and OS version is of interest, I will try to do my best to add them.

Action for Mac and Ubuntu is coming from INSTALL.md
Update: add FreeBSD build for trial too

Note: add build for MacOS 12 & 13. 14 fails (as we already know)

See result here

@guiv42
Copy link
Collaborator

guiv42 commented May 1, 2024

For sure this approach has serious advantages.
Most contributors (if not all) do not have the possibility to build all flavors of TuxGuitar, so this approach could help detect possible incompatibilities introduced by a PR for example.

I think the most interesting use cases are:

  • build with the native-modules: contributors working only in Eclipse most probably never build in this configuration (I never succeeded)
  • build the Android and the FreeBSD versions: most contributors probably do not have set up the required environment
  • build the macOS version: not accessible to all contributors
  • for each platform, build both SWT and JFX variants

Then, I must admit there are also drawbacks. The most critical one I think: this is somehow redundant with the build script used by @helge17 to build and release TuxGuitar in all flavors. Also with some information available in INSTALL.md: typically the list of build dependencies.
So this means double maintenance whenever something needs to change in the build environment.

Ideally the build script, the INSTALL.md doc and the GitHub actions should share a maximum of possible data. For example:

  • a dedicated script to download/install SWT could be separated from the main build script. It could be called from both the GitHub actions and the main build script, and referred to in INSTALL.md (and contribute.md also!)
  • same thing for JFX on FreeBSD
  • build dependencies could be defined by a script, referenced by both INSTALL.md and the Github actions

Also: most of TuxGuitar's code is independent from the OS, and there is an abstraction layer for ui frameworks (SWT vs. JFX). So it is clearly possible to create an incompatibility, but this should really not happen very often.
After about 1 year of active development, I admit I faced personally all use cases of incompatibilities I mentioned above. So I can't say it's useless. But to be honest it happened to me about 3-4 times. Considering the number of commits I pushed, effective coverage remains quite low. Most of the incompatibilities I've introduced would not have been detected by these Github actions.

@helge17: what do you think?

@helge17
Copy link
Owner

helge17 commented Sep 17, 2024

My development environment is currently quite stable and a migration to Github actions would mean a lot of effort. That's why I prefer to stick with my build script. There is a certain redundancy in the script, the documentation and the Github actions, that's true. But again, I fear that the effort to fix this redundancy is currently greater than the benefit. I also think it is better to list the individual commands in the documentation instead of just saying "call script xyz".

But if Github actions would help you, we can merge PR #368. I will try to keep the files up to date. Maybe I'll fall back on it at some point, for example if I can no longer manage to keep all my virtual machines running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants