This is the partner PR to https://github.com/geany/infrastructure/pull/11:
the latest Scintilla version doesn't compile in the Mingw64 cross compiler CI job. This is because the used cross compiler toolchain doesn't support C++ features like "std:future". See https://sourceforge.net/p/mingw-w64/bugs/959/.
So we use the "posix" variant of this compiler toolchain which *does* support it. To be able to start Geany with this change, we need to copy the corresponding C++ runtime library into the GTK bundle and so into the installer.
While at it, I added some information about the used compiler and library versions as suggested in https://github.com/geany/geany/pull/3551#issuecomment-1736865318. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3568
-- Commit Summary --
* CI: Copy used C++ runtime library into the GTK bundle * CI: Log compiler and library versions in build output
-- File Changes --
M scripts/ci_mingw64_geany.sh (12)
-- Patch Links --
https://github.com/geany/geany/pull/3568.patch https://github.com/geany/geany/pull/3568.diff
@eht16 pushed 1 commit.
30372ece08c0d4986b36afcb1652b2cc3f26689f CI: Log compiler and library versions in build output
@eht16 pushed 2 commits.
354c7ba4e66803f16d9e0452b7829d47cb742587 CI: Copy used C++ runtime library into the GTK bundle b18c76edc9eb10a5c0097d4231c387885e2f266f CI: Log compiler and library versions in build output
@kugel- approved this pull request.
Meta-criticism: I dislike that that we (apparently) cannot fix this in Geany alone but need a change on geany/infrastructure. Regular contributors are not really able to propose such changes.
How should we fix it in Geany?
An external library which we include in our build (Scintilla) uses features a special compiler toolchain does not support.
It's not a problem with our code that we could fix.
Agree with @eht16, the Geany repository does not define the toolset to use, and it should not, otherwise it would restrict where it can be built to places where our "blessed" toolset is available.
I'm not surprised that a generic cross compiler/stdlib does not support fancy features like futures and threads. Unfortunately Scintilla uses the known buggy configure tool "tell the user what the requirements are" to check the toolchain :-)
How should we fix it in Geany?
I don't have an answer (hence I approved this PR) but my perspective is that Geany should the CI requirements, otherwise regular contributors might be unable to propose certain changes without breaking CI (because they cannot fix the CI in the same context).
Imagine a regular contributor wanted to update to a scintilla version that has new requirements.
might be unable to propose certain changes without breaking CI
Even if the CI was all in Geany repository I don't think it will run the new CI just because its in the PR. This was one of githubs recent security fixes IIRC so actions can't push the modified code from the PR to release.
CI still checks out the PR branch and runs the scripts within, doesn't it?
CI still checks out the PR branch and runs the scripts within, doesn't it?
Ahh, yes, in fact thats what Github were warning about, by default the action can do anything, so what CI can do should be restricted (by the Geany security expert team ;-) and then we are back to the original position where normal people can't modify CI, sigh, I hate security issues.
Maybe you could open a new issue to discuss how we can rework the CI infrastructure, if you like. IMO this is out of scope of this PR.
If nobody stops me, I would like to merge this and the infrastructure PR to fix the CI (and do this for G-P as well afterwards).
Go for it, it's approved after all.
Merged #3568 into master.
github-comments@lists.geany.org