Fixes https://github.com/geany/geany/issues/3435
@AntumDeluge @eli-schwartz You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3466
-- Commit Summary --
* meson: Fix conditions when function is absent in Windows * meson: Define WIN32 macro for ctags
-- File Changes --
M meson.build (22)
-- Patch Links --
https://github.com/geany/geany/pull/3466.patch https://github.com/geany/geany/pull/3466.diff
If required, I can try to add the mingw meson build in CI and replace autotools with meson also.
After a quick shot, the changes look well to me. Tested on native Windows and built fine.
If required, I can try to add the mingw meson build in CI and replace autotools with meson also.
Not sure if the Meson is already ready to be used for CI. Ideally, the CI builds generate full installers in release quality. Maybe it would be better to first port Geany-Plugins to Meson?
A few concerns: - is it possible to build Geany-Plugins with Autotools against a Geany built with Meson? - the Meson build installs the docs into the root prefix instead of `share/doc/geany` - the Meson build does not install libexec/geany/geany-run-helper.bat (for full details find the lift of installed files by Autotools and Meson attached) [tree_at.txt](https://github.com/geany/geany/files/11301612/tree_at.txt) [tree_meson.txt](https://github.com/geany/geany/files/11301613/tree_meson.txt)
Maybe we should discuss that CI topic in separate issue or pull request.
is it possible to build Geany-Plugins with Autotools against a Geany built with Meson?
WFM on Linux at least
the Meson build installs the docs into the root prefix instead of share/doc/geany
WFM on Linux, must be something about meson for windows, maybe the generated installer?
@Biswa96 because of the difficulty building Geany on windows the CI builds are important so people will test PRs, otherwise they rarely get tested on Windows. So the meson CI has to do the build before it can be accepted to replace the autotools build. So its part of this.
@techee referencing to the build log for mingw here. Ctags seems to have a _lot_ of size and signedness mismatch errors. Are they all safe? They don't show on an autotools CI mingw build (#3465 for eg) maybe meson has more strict default flags, and they need to be relaxed?
is it possible to build Geany-Plugins with Autotools against a Geany built with Meson?
I didn't have any problems doing so on Linux with PIC build.
Not sure if the Meson is already ready to be used for CI. Ideally, the CI builds generate full installers in release quality.
I would say this is an argument against dropping autotools from CI. Either way, you want to test meson in CI because you do not want meson's support to regress, surely?
is it possible to build Geany-Plugins with Autotools against a Geany built with Meson?
I can't think of a reason why not. Surely, if both build systems install the same files -- which they do on Linux at least, right?
the Meson build installs the docs into the root prefix instead of `share/doc/geany`
This is a deliberate inconsistency, for whatever reason:
https://github.com/geany/geany/blob/7af6eafc0525bfafc8bdb2fbd29452b5f5409f56...
Maybe inadvisable, maybe it should be changed, but the current state of affairs is that meson has been instructed to do different things on mingw, so it does. This can be easily changed if desired. Perhaps it was unclear in the initial port what the optimal approach was.
the Meson build does not install libexec/geany/geany-run-helper.bat (for full details find the lift of installed files by Autotools and Meson attached)
This is pretty easy to do now that the concern has been raised.
I would say this is an argument against dropping autotools from CI. Either way, you want to test meson in CI because you do not want meson's support to regress, surely?
Both autotools and meson are tested on Linux by CI, this is for Windows mingw builds which have never had meson builds on CI before. Agree at this stage it should not _replace_ autotools CI as I said above, but it seems to do so IIUC.
Surely, if both build systems install the same files -- which they do on Linux at least, right?
Actually no, on Linux the Geany Autotools installs libs in in $prefix/lib but the Meson build uses the correct place, $prefix/lib/x86_64-linux-gnu (or i386-linux-gnu or aarch64 or whatever as appropriate). But so long as the plugin Autotools uses `pkg-config` to find them (as I assume it does on Linux) it should work on windows as well.
Perhaps it was unclear in the initial port what the optimal approach was.
Most probably, IIUC the guy who did the original port is on holidays, so he is forgiven for no input until he returns.
@Biswa96 pushed 1 commit.
bafaaef3031611c39535f655343f57defce526ba CI: Add mingw meson build
I have added a basic mingw64 meson workflow. The CI needs permission to allow that.
@techee referencing to the build log for mingw here. Ctags seems to have a lot of size and signedness mismatch errors. Are they all safe? They don't show on an autotools CI mingw build (https://github.com/geany/geany/pull/3465 for eg) maybe meson has more strict default flags, and they need to be relaxed?
I've seen these too but I haven't investigated much what is going on. Also there are some warnings about unused functions - we build only a subset of ctags, and, as a result, some code isn't used at all.
I have added a basic mingw64 meson workflow. The CI needs permission to allow that.
Permission granted. Could you rebase on current master, https://github.com/geany/geany/pull/3438 has only just been merged to allow manual start of the workflow :).
Thanks for the new workflow, this might help as a quick run of the Mingw build with Meson. It probably runs way faster than the full bloated Autotools build. I think this is a good addition to the existing Mingw workflow. Then we can replace the Autotools build some time in the future after we are confident that the Meson Mingw build is mature enough.
@Biswa96 pushed 3 commits.
d0f84fcee7bff902896ab61e259e6e2eed54e1c1 meson: Fix conditions when function is absent in Windows 8d6e4d3b33c0e0c0f563e871b27cc14ce9da7521 meson: Define WIN32 macro for ctags cd35bd52030908b632930532c6d563ea9119337f CI: Add mingw meson build
Rebase done.
Is there any progress on this topic?
I guess we are waiting for @kugel- to review. Maybe @eli-schwartz could have a look, for me this would suffice.
One thing to note: with the new workflow based on the `msys2/setup-msys2` action, we now depend on a proprietary Windows runner in the CI. I didn't make up my mind about this yet.
Maybe @eli-schwartz could have a look, for me this would suffice.
The configuration data seems fine to me. As discussed on the bug report:
I guess the intention was to replicate the olde timey idiom of not defining `THING` or `#define THING 1` if it was defined.
As you say most of the actual tests just look for "defined" and don't care what value, so hopefully true/false will work just fine.
My personal sense of aesthetics suggests consistently defining things, but this PR implements the compatible approach instead, which is a very justifiable decision to make and should work quite well.
I haven't reviewed the second or third commits -- I am not a mingw or generally a Windows person, so I can't do a good job evaluating when to `-DWIN32`.
One thing to note: with the new workflow based on the `msys2/setup-msys2` action, we now depend on a proprietary Windows runner in the CI. I didn't make up my mind about this yet.
I would not worry about this, because the current state of affairs is that this isn't tested, and adding additional proprietary CI is something I would personally describe as "easy come, easy go". As long as it works, it works, and does no harm. If it turns out to be unmaintainable, or breaks when transferring to a different software forge (sourcehut, gitlab, gitea...), then you just delete it again.
But there's no particular reason to think it will stop working any time soon, and it provides free bug reports about things breaking, so why not. :)
Closed #3466.
github-comments@lists.geany.org