abdes
Tests
Here comes a bigger addition to our test.
Test are now also run in Windows with msbuild and nmake. We can also now check everthing on all 4 flavour of msys and under Cygwin. And we make sure that Release and Debug builds are tested.
In addition I prefixed the tests with cryptopp-, so they can be easier run, selected or omitted when integrated in bigger chain-build (I at least prefer to have everthing tested).
Things that still need to be done: Ensure that a buildtype is selected. As the build-type now is submitted to the tests, there needs to be a value. The big question is default to Release or Debug. Add Fixtures to tests: It doesn't seem to be enough to just add DEPENDS to the properties to have them failsave running in parallel. Ccache is not restore- or saveable in Cygwin atm. Issue was reported to upstream. Work on the ToDo to maybe reduce the usage DATA_DIR define, so tests could reuse the ccache of the main-build.
For Cygwin and MSys-32 it was needed to disable asm. I've read somewhere that this shouldn't be needed and if needed it should be reported on the mailinglist. Maybe @noloader has the best connections for this (creating a branch for the two where they fail is possible on request)
And maybe adding the lint to normal commits would also be a good idea to avoid the need of editing afterwards just because of a dot at the end.
And I guess the vars in the workflows could be revisited, as some (like build-type) are not needed or used anymore.
And maybe adding the lint to normal commits would also be a good idea to avoid the need of editing afterwards just because of a dot at the end.
Check this: https://github.com/abdes/cryptopp-cmake/blob/master/CONTRIBUTING.md#conventional-commits
So, my major concern with this PR is not its quality, but rather its size. It brings way too many changes in one go. Some of the changes are "bug fixes", some others are "general enhancements to the test framework and build system", some others are to bring cygwin in, some others are for msys2, some others are for mingw.
Preferably we need to have these changes made in an atomic fashion where each change comes into its own PR so we can have reliable and meaningful history.
I am also getting very uncomfortable with the size of the workflow file. It needs to be broken down into manageable pieces. Some reusable workflows, maybe some actions and splitting the targets into multiple chunks:
- core targets that are used for releasing the artifacts
- additional test targets for windows
- additional test targets for msys2
- additional test targets for cygwin
- additional test targets for mingw
- additional test targets for linux
- additional test targets for macos
That way these separate blocks can evolve quite independently without us needing to check every PR what is the impact on every possible target.
I know you've done an amazing job with this PR and this is additional work needed to break it down into multiple PRs. Let me know what you think and how I can help you with this rework.
Check this: https://github.com/abdes/cryptopp-cmake/blob/master/CONTRIBUTING.md#conventional-commits
I know about this, but as I never came in touch with npm or all that stuff this would be just another packagemanager which throws stuff at my system. I don't know where or what, how to influence, update or remove (and if remove, is it complete) that stuff.
Some of the changes are "bug fixes", some others are to bring
cygwinin, some others are formsys2, some others are formingw.
Yes, I didn't see any meaning in testing on a new platform and bring to fix for it later, or fixing something and bringing in the test for it later.
But you're right, splitting it would make this much more reviewable as you'll get the patches in order. From the histories perspective this makes no difference as all commits go in there. 1x 20 commits or 10x 2 sum up to the same amount. The difference would have an impact if they would be squash-merged, but then the whole commit-message thing wouldn't have any meaning.
I am also getting very uncomfortable with the size of the workflow file. It needs to be broken down into manageable pieces. Some reusable workflows, maybe some actions and splitting the targets into multiple chunks:
That's a problem, I think. As you can see, every workflow file get's it's own entry in the actions-tab. Maybe it's only me, but as I can't host all the runners locally and github uses only a subset (with some enhancements) of yaml, writing such a thing is more like qualified guessing. It's committing, seeing what breaks and reediting the commit including deleting of the failed run. Having 5 or 6 files will not really help to keep an overview in this section.
For sure I would prefer to have them all in their own files and then having one master that runs them all, but unfortunately yaml doesn't know any include statment.
* core targets that are used for releasing the artifacts
To be honest, I never got the deeper meaning of this. I wouldn't even know about them if I wouldn't have read the workflow and I still don't know an easy way of accessing them. Plus (beside windows) I can't imagine someone would find them really useful as these systems have working packaging systems that allow them to easily remove them when not needed anymore. Maybe this will change in the future as CPack is the next on my list, but then we should link them in the README aswell.
I could imagine a workflow that is one run on release for this, outside of the usual workflow. But I'm not sure how to test them without creating (a bunch of) fake-releases.
* additional test targets for msys2 * additional test targets for cygwin * additional test targets for mingw
Splitting out these is no problem at all. (mingw is msys)
* additional test targets for windows * additional test targets for macos * additional test targets for linux
As it seems (didn't look deeper) they rely on the same pieces of cmake code included in the worklow file. It can be stripped to the relevant parts, but will still have some duplication in it.
That way these separate blocks can evolve quite independently without us needing to check every PR what is the impact on every possible target.
If you mean that a change for one system in that big file could impact the other systems, yes. Otherwise I don't get it.
Let me know what you think and how I can help you with this rework.
As everything is in place, I'll give it a shot with how it could look like in my head, shouldn't be that hard to reorder and split into multiple branches. I just need to know how you think about the right way.
My proposal would be to split out release and then do the testing in system-specific sections but having that sections in one big workflow-file. Just like have them in the separate files that are glued together to keep the actions-tab cleaner. That way only the touched sections would need to be checked.
I think we're quite aligned. Maybe some of the ideas I had are a bit over-fetched (the splitting of workflow file into independent files), but I know it is feasible and I just need to do the proper research for it. In the meantime, your suggestion of keeping one file but having clearly demarcated sections is fine.
Priority for now is split the PR and in particular extract bug fixes (thank you for finding and fixing them) and general enhancements (also thank you for making the overall build/test system more reliable).
Then you can push cygwin, msys/mingw in separate PRs.
Regarding the release, I agree with you and I'm looking forward to see your future proposals for improving this and leveraging CPack.