Skip to content

Conversation

@svmhdvn
Copy link
Contributor

@svmhdvn svmhdvn commented Jan 19, 2024

This change replaces Cirrus CI tasks for linux and macos with Github Actions equivalents.

Github actions doesn't seem to have a "compute credits" or similar restriction for public instances. Since kakoune is mainly hosted on github, it's also really well integrated with the pull request flow.

See https://github.com/svmhdvn/kakoune/actions/runs/7578698028 for a successful run.

@krobelus
Copy link
Contributor

Perhaps the problem with Cirrus was that a hung test (which has been fixed since) used up all available time.
This change makes it so we no longer specify the compiler version explicitly, and moves from g++-10 to g++-11. Both seem fixable if desired.

@svmhdvn
Copy link
Contributor Author

svmhdvn commented Jan 19, 2024

Perhaps the problem with Cirrus was that a hung test (which has been fixed since) used up all available time.

Sure, but I still find it more valuable to be free from these limits in the first place, and to reduce further dependencies on other services outside of the (implied constant) Github.

Both seem fixable if desired.

Yes exactly, I can easily change the toolchain versions (or add more to the matrix) if desired by kakoune's maintainers.

@krobelus
Copy link
Contributor

Looks like Cirrus works again.
In the meantime we also added Sourcehut CI which covers everything except macOS.
The status quo seems acceptable but let's see what the future brings

@svmhdvn
Copy link
Contributor Author

svmhdvn commented May 7, 2025

I'd like to revisit this as the same issue is occurring. Any comments on this, either an approval or rejection?

@krobelus
Copy link
Contributor

krobelus commented May 7, 2025

I'd like to revisit this as the same issue is occurring. Any comments on this, either an approval or rejection?

Thanks for flagging this. I'd be happy to see this land.
I wonder if the compute-credit problem also happens for Cirrus' FreeBSD images.
Sourcehut CI is nicer to work with, but this gives us macOS coverage, which is a nice addition.

Can we use the same container images as for cirrus? i.e. clang 13 and gcc 10?
I'd guess it would be easier to accept this PR if it is limited only changing the CI provider.
The status quo means we need to bump compiler versions explicitly, which seems potentially useful

@svmhdvn
Copy link
Contributor Author

svmhdvn commented May 7, 2025

Sourcehut CI is nicer to work with, but this gives us macOS coverage, which is a nice addition.

But people contribute to kakoune on Github, and thus will mostly pay attention to (and will notice) the results of their PR CI runs on Github. This might be better for catching developer errors more quickly.

I'd guess it would be easier to accept this PR if it is limited only changing the CI provider. The status quo means we need to bump compiler versions explicitly, which seems potentially useful.

Why is this an important consideration? Since kakoune uses the latest c++ standard, IMO it would be beneficial to always be tracking the latest compilers, which my PR reflects. Additionally, developers are most likely to be running those compilers as well, since it uses out-of-the-box package manager versions.

@krobelus
Copy link
Contributor

krobelus commented May 7, 2025

But people contribute to kakoune on Github, and thus will mostly pay
attention to (and will notice) the results of their PR CI runs on
Github. This might be better for catching developer errors more quickly.

Yes, it's very true that github CI is much more accessible and familiar.
I'm very much in favor of having a CI that integrates with GitHub
(which SourceHut no longer supports IIRC).

(I have made the mistake of not looking at CI before submitting patches one
too many times, so I'm always running a hut builds submit -f now before
sending patches.)

I'd guess it would be easier to accept this PR if it is limited only
changing the CI provider. The status quo means we need to bump compiler
versions explicitly, which seems potentially useful.

Why is this an important consideration? Since kakoune uses the latest c++
standard, IMO it would be beneficial to always be tracking the latest
compilers, which my PR reflects. Additionally, developers are most likely
to be running those compilers as well, since it uses out-of-the-box package
manager versions.

yes developers are expected to be running something more modern, but as I
see it, the version defined in CI is supposed to be a lower bound we can
reasonably ask users to have.

Today on IRC, a user reported this build failure on Debian:

g++: error: unrecognized command-line option ‘-std=c++2b’; did you mean ‘-std=c++20’?

Perhaps they're on GCC 10 too.
Debian stable has GCC 12 nowadays, so it's probably time to bump to that one.

In fact, the c++2b build failure seems to be the one causing our CI to fail (e.g. https://github.com/mawww/kakoune/runs/41304539219).
So I'm not sure if it's actually about compute credits?

Regarding explicit vs. implicit compiler version, I don't
have a very strong opinion. What I wanted to point out is
that if we let github actions update compiler version (they're on gcc 13 now),
we might not notice that we introduce build failures for users - though I
guess this is somewhat unlikely and not a big problem.

@svmhdvn
Copy link
Contributor Author

svmhdvn commented May 7, 2025

Regarding explicit vs. implicit compiler version, I don't
have a very strong opinion. What I wanted to point out is
that if we let github actions update compiler version (they're on gcc 13 now),
we might not notice that we introduce build failures for users - though I
guess this is somewhat unlikely and not a big problem.

(Emphasis mine.)

I agree with what you're saying about expecting the "lowest bound" compiler versions to work. However, the goal of this CI infrastructure is to track the master branch, which is by definition bleeding-edge. IMO, I'd personally want to be notified of as many pedantic issues as possible in the master branch of CI, so that when it comes time to tag a new release, that release will work on a wide range of compiler versions and will stay that way for a long time.

It might be better to ensure that prior to tagging a release, the codebase builds and runs on that "lower bound" that we expect stable distros like debian to have.

@mawww
Copy link
Owner

mawww commented May 11, 2025

I try to keep Kakoune compiling on whatever debian stable compiler is, so ideally that would be the version that is tested. This is already tested by the sourcehut build (which uses debian images) so we are covered on that front. It would be nice to see breakage on github pull requests before merging, but to be honest I do not often check the CI for PRs, and AFAIR its pretty rare that we break the debian stable build.

@mawww mawww merged commit 2d8c0b8 into mawww:master May 11, 2025
5 of 6 checks passed
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