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

CI improvements #23

Merged
merged 92 commits into from
Mar 5, 2025
Merged

CI improvements #23

merged 92 commits into from
Mar 5, 2025

Conversation

janheinrichmerker
Copy link
Contributor

  • use of GitHub Actions matrix builds for building the C libraries and CLI
  • add all OS-specific libraries/CLIs to releases
  • add Python 3.13 support
  • build using GCC and clang

@janheinrichmerker janheinrichmerker self-assigned this Mar 4, 2025
@janheinrichmerker janheinrichmerker marked this pull request as draft March 4, 2025 12:45
@TheMrSheldon
Copy link
Member

What do you think about moving some of the badges away from the top of the readme since that looks rather cluttered now. Instead, the top of the readme could contain OS support badges (though we should also stress that these are the OS's that we automatically test for and probably not the only ones we support) and the language specific measures could either be moved into the language specific readme sections or in the language specific subfolders (/(c/python/jvm)/readme.md).

@TheMrSheldon
Copy link
Member

Thank you very much for your hard work on the PR! I will look into it soon

@TheMrSheldon TheMrSheldon self-requested a review March 5, 2025 07:23
@TheMrSheldon
Copy link
Member

Build Debian Package should only be necessary from ubuntu-16.04 and with one compiler (clang-18) since, if the CLI can be built on various systems, so can the Debian Package and we only need to build it once to publish it in the release. As such, the Debian Package Build may even only be done on Release if we want.

@janheinrichmerker
Copy link
Contributor Author

janheinrichmerker commented Mar 5, 2025

What do you think about moving some of the badges away from the top of the readme since that looks rather cluttered now.

I would maybe like to keep the badges at the top. If it looks a bit cluttered, that might actually also be a handy visual indicator that we support and test "everything".

@janheinrichmerker
Copy link
Contributor Author

Build Debian Package should only be necessary from ubuntu-16.04 and with one compiler (clang-18) since, if the CLI can be built on various systems, so can the Debian Package and we only need to build it once to publish it in the release. As such, the Debian Package Build may even only be done on Release if we want.

Is this guaranteed? Or could it fail if some tool is missing? In either case, I would like to have all artifacts built before the release. So that we only do a GitHub release if everything works. And then, a matrix test does not hurt.

@TheMrSheldon
Copy link
Member

Build Debian Package should only be necessary from ubuntu-16.04 and with one compiler (clang-18) since, if the CLI can be built on various systems, so can the Debian Package and we only need to build it once to publish it in the release. As such, the Debian Package Build may even only be done on Release if we want.

Is this guaranteed? Or could it fail if some tool is missing? In either case, I would like to have all artifacts built before the release. So that we only do a GitHub release if everything works. And then, a matrix test does not hurt.

AFAIK yes. CPack is part of CMake and the Package target does not do anything more than building the binaries and using CPack.

@janheinrichmerker
Copy link
Contributor Author

Build Debian Package should only be necessary from ubuntu-16.04 and with one compiler (clang-18) since, if the CLI can be built on various systems, so can the Debian Package and we only need to build it once to publish it in the release. As such, the Debian Package Build may even only be done on Release if we want.

Is this guaranteed? Or could it fail if some tool is missing? In either case, I would like to have all artifacts built before the release. So that we only do a GitHub release if everything works. And then, a matrix test does not hurt.

AFAIK yes. CPack is part of CMake and the Package target does not do anything more than building the binaries and using CPack.

Still, does it hurt to test it? Let us not rely too much on some knowledge of the inner workings of CMake. To me as a C amateur, it looks like the package build "under the hood" could potentially run something completely different from the library build. And that detail is hidden in the CMakeLists.txt files (which are, frankly, not very easily understandable to non C professionals)
By the way, the Debian package job almost always finishes before the C library job anyway (as Windows is slooow), so it does not add to the overall build time.

@janheinrichmerker
Copy link
Contributor Author

I think, re-enabling the failing build configs and fixing each of these issues should be done in separate issues and PRs after we merge this one. I'd also like to work on Maven Central publishing, so lets get this merged 🙂

@TheMrSheldon
Copy link
Member

I think, re-enabling the failing build configs and fixing each of these issues should be done in separate issues and PRs after we merge this one. I'd also like to work on Maven Central publishing, so lets get this merged 🙂

Yeah, makes sense to me

@janheinrichmerker janheinrichmerker merged commit 9607455 into master Mar 5, 2025
66 checks passed
@janheinrichmerker janheinrichmerker deleted the ci-improvements branch March 5, 2025 11:40
@janheinrichmerker janheinrichmerker mentioned this pull request Mar 5, 2025
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants