ci: rework

[umarcor]

Close #40

The main purpose of this PR is to switch from using a manual build procedure to a PKGBUILD recipe. Having a working PKGBUILD allows upstreaming this project to official MSYS2 repositories.

The proposed recipe fails because install is not a make target. @WerWolv what do you prefer, writing installation steps manually in the PKGBUILD file or adding them through cmake?

NOTE: the resulting *.zst package can be extracted and distributed to be used outside of MSYS2, as long as the additional dependencies mentioned in the readme are added.

[marysaka]

I'm not really a fan of doing a package only approach here.
I originally designed this to be able to generate nightly build easily in the future for Windows.
I think it would be better to move this to another action.

[marysaka]

Seems to fail on the install part

[umarcor]

I'm not really a fan of doing a package only approach here.
I originally designed this to be able to generate nightly build easily in the future for Windows.
I think it would be better to move this to another action.

I'm sorry, but I don't understand. The proposed solution is equivalent to the existing procedure. The only difference is the PREFIX. You can either ignore or extract the package and you get the same result.

This provides nightly builds already. Ready to use tarballs are generated after each push or pull request. The only missing step is uploading the artifact.

I would understand creating a diferent job if the existing one didn't use MSYS2, but that's not the case. However, I can keep both procedures in the matrix.

[umarcor]

Seems to fail on the install part

As commented above, there is no install make target. That needs to be addressed.

[WerWolv]

Thanks for the PR!
For the install steps, I think I'd prefer adding them to cmake. But I'll leave everything else to @Thog, I don't know that much about the whole subject.

[umarcor]

I updated the PR for running two jobs in parallel. One builds manually, the other one uses the PKGBUILD file.

For the install steps, I think I'd prefer adding them to cmake.

Will you take care of that? For reference: https://github.com/trabucayre/openFPGALoader/blob/master/CMakeLists.txt#L174-L182

[umarcor]

The GNU/Linux workflow contributed in the last hours is now merged with Windows jobs in a single workflow named 'Build'. A shield is added to the README. See:

• https://github.com/WerWolv/ImHex/actions/runs/403395465
• https://github.com/umarcor/ImHex/tree/ci/update

[umarcor]

• This PR is now based on cmake: add install target #40. Hence, make install succeeds.
• The package generated by makepkg-mingw is uploaded as an artifact. That's a nightly package that any MSYS2 user can download and install.
• A test job is executed after uploading the artifact, in order to test the package in a clean environment.

The PKGBUILD recipe needs to be enhanced, in order to declare runtime dependencies. Nonetheless, this PR can be merged as is, since it meets CI purposes.

[marysaka]

When I said doing a nightly system, I was meaning:

1. A release zip containing everything to be able to run the app out of the box (without MINGW64)
2. The release should be hosted on GitHub release because of how small the artifact storage is on GitHub (not to mention that we could keep those nightly around to help bisecting issues in the future)

As a note, I also don't think that having a test job is useful right now. If tests need to be run, it must be part of the build job.

[umarcor]

1. A release zip containing everything to be able to run the app out of the box (without MINGW64)

As commented above, that is doable by taking the existing package/artifact as a base. Just need to add the extra DLLs which don't exist outside of MSYS2. In fact, I believe that is the natural procedure, given that the tool is built inside MSYS2 in any case.

2. The release should be hosted on GitHub release because of how small the artifact storage is on GitHub (not to mention that we could keep those nightly around to help bisecting issues in the future)

There is no limit for free and open source projects, AFAIAA. Furthermore, artifacts of this project are really small. It is unlikely to hit the limit in the 90 day window. Nonetheless, the retention of artifacts can be configured.

Artifacts can be pushed as assets of a (pre)release. See, for instance:

• https://github.com/ghdl/ghdl/blob/master/.github/workflows/push.yml#L173-L187
• https://github.com/ghdl/ghdl/releases/tag/nightly

As a note, I also don't think that having a test job is useful right now. If tests need to be run, it must be part of the build job.

Build dependencies and runtime dependencies are different. If the artifacts are tested in the build environment, it is easy to overlook which are the runtime dependencies in practice. Testing the tool in a clean environment is the closest to what users of the tool will get when trying an artifact/release. The issue I found is that ImHex seems to ignore CLI arguments. That's why the current test is rather stupid.

[marysaka]

As commented above, that is doable by taking the existing package/artifact as a base. Just need to add the extra DLLs which don't exist outside of MSYS2. In fact, I believe that is the natural procedure, given that the tool is built inside MSYS2 in any case.

To me, we shouldn't unpack a package and then repack it with the needed dependencies so this could be done in the win-manual I suppose.

There is no limit for free and open source projects, AFAIAA. Furthermore, artifacts of this project are really small. It is unlikely to hit the limit in the 90 day window. Nonetheless, the retention of artifacts can be configured.

Artifacts can be pushed as assets of a (pre)release. See, for instance:

• https://github.com/ghdl/ghdl/blob/master/.github/workflows/push.yml#L173-L187
• https://github.com/ghdl/ghdl/releases/tag/nightly

I think that could be a nice idea, we will need to have some action to do periodic artifacts cleanup (using https://github.com/c-hive/gha-remove-artifacts)

Build dependencies and runtime dependencies are different. If the artifacts are tested in the build environment, it is easy to overlook which are the runtime dependencies in practice. Testing the tool in a clean environment is the closest to what users of the tool will get when trying an artifact/release. The issue I found is that ImHex seems to ignore CLI arguments. That's why the current test is rather stupid.

Testing a GUI based application is kind of hard, especially on CI (we cannot create an OpenGL context without a GPU) so I don't think it's quite possible to do it in a nice way. You can add to this that if we want to be closer to a regular user environment, we need to do it outside MSYS2 on Windows as we aren't expecting end user to have MSYS2 installed. One thing that we can probably do however is having a tool that parse the resulting PE and ensure that all libraries are present (by checking for existence of libraries in standard Windows paths and current directory of the PE)

[umarcor]

To me, we shouldn't unpack a package and then repack it with the needed dependencies so this could be done in the win-manual I suppose.

You don't need to unpack and repack the content, since it's already available for you in a subdir. Anyway, it is feasible to do it manually too, should you be willing to do so.

I think that could be a nice idea, we will need to have some action to do periodic artifacts cleanup (using c-hive/gha-remove-artifacts)

My experience is that there is no need for artifact cleanup. That's a requirement for users/projects with non-free repos or with a really heavy activity. Nevertheless, adding gha-remove-artifacts should be harmless.

Testing a GUI based application is kind of hard, especially on CI (we cannot create an OpenGL context without a GPU) so I don't think it's quite possible to do it in a nice way.

The purpose of the Test step in this PR is not verifying the GUI features of the tool. Instead, it's for checking that the package/tarball which users download/install contains all the expected resources; plus, that executable files are actually executable. Since different build options are used, some binary might crash or some assets might be missing. That's what we want to check.

For example, GTKWave is a GUI application, but gtkwave --help and/or gtkwave --version return content in CLI without starting the GUI. That allows smoke-testing the binary.

You can add to this that if we want to be closer to a regular user environment, we need to do it outside MSYS2 on Windows as we aren't expecting end user to have MSYS2 installed.

Maybe I didn't explain myself properly: my purpose is to package and upstream ImHex, so that MSYS2 users can install it through pacman. Therefore, I believe we want to have the package tested on MSYS2, independently of other manual procedures being supported/tested too.

Incidentally, you are using MSYS2 for the manual build procedure, instead of a "more native" workflow, such as MSVC. As a result, you can reuse the existing package and just add the missing DLLs, which you can extract from the MSYS2 environment that you are already using. Nevertheless, if you want to do everything manually, that's up to you.

One thing that we can probably do however is having a tool that parse the resulting PE and ensure that all libraries are present (by checking for existence of libraries in standard Windows paths and current directory of the PE)

This is a rather common requirement: building packages on MSYS2 and then distributing them to be used outside of MSYS2. There are a few projects that try to get the list of DLLs/dependencies automatically:

• https://github.com/achadwick/styrene
• https://www.msys2.org/wiki/Distributing/
• https://www.msys2.org/docs/package-management/#finding-dependencies-of-a-package

Yet, the natural procedure is to build an MSYS2 package and then produce a larger zipfile. Should you want to skip the first step and do it manually, you might need to tweak some of those helpers.

[umarcor]

GitHub Actions's web GUI was updated. Now the dependencies of the jobs are easily seen: https://github.com/WerWolv/ImHex/actions/runs/407237481

@Thog can we have this merged?

[marysaka]

lgtm, thanks 👍