To match the autotools build. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3660
-- Commit Summary --
* meson: correctly set WIN32 in config.h
-- File Changes --
M meson.build (4)
-- Patch Links --
https://github.com/geany/geany/pull/3660.patch https://github.com/geany/geany/pull/3660.diff
An alternative would be to replace all checks for WIN32 in the code with _WIN32, but there are lots of places..
An alternative would be to replace all checks for WIN32 in the code with _WIN32, but there are lots of places..
And they are in ctags code, so this would have to be fixed upstream.
Not tested, but looks OK.
And they are in ctags code, so this would have to be fixed upstream.
I have created a pull request to fix it in upstream ctags repository.
Is `_WIN32` set for cross builds? For Geany Windows builds are usually cross built on Linux.
Is `_WIN32` set for cross builds?
`_WIN32` is set in compiler which targets Windows platform - no header or IDE is required. You can verify that using `cc -dM -E - < /dev/null | grep WIN32` command.
This underlying issue has been fixed in upstream ctags repository.
This underlying issue has been fixed in upstream ctags repository.
Ok, will get included when the ctags is next updated from upstream.
Is `_WIN32` set for cross builds?
`_WIN32` is set in compiler which targets Windows platform - no header or IDE is required. You can verify that using `cc -dM -E - < /dev/null | grep WIN32` command.
Yep, looks good. This is in a Docker container running the image we use in CI for cross-compilation: ``` root@470e6364089b:/build# x86_64-w64-mingw32-gcc -dM -E - < /dev/null | grep WIN32 #define __WIN32__ 1 #define _WIN32 1 #define WIN32 1 #define __WIN32 1 ```
And natively in MSYS2 on Windows: ``` $ cc -dM -E - < /dev/null | grep WIN32 137:#define __WIN32__ 1 183:#define _WIN32 1 301:#define WIN32 1 325:#define __WIN32 1 ```
Is there anything wrong with merging this?
Even if it would be cooler to not have to set the flag, as it is done by Autotools as well, it might be ok.
And if uctags is updated next time, we could try to remove it again.
$ cc -dM -E - < /dev/null | grep WIN32 137:#define __WIN32__ 1 183:#define _WIN32 1 301:#define WIN32 1 325:#define __WIN32 1
Yes, gcc and clang both defines those macros which are not official or documented by Microsoft. Those are probably for compatibility with ancient code. Android has similar issue about `ANDROID` vs `__ANDROID__` but the later one should be used according to documentation.
@b4n commented on this pull request.
@@ -185,6 +185,10 @@ if python_command == '' or python_command == 'auto'
endif cdata.set('PYTHON_COMMAND', python_command)
+if host_machine.system() == 'windows'
This is [fine](https://mesonbuild.com/Cross-compilation.html), but we probably should stop using `target_machine` everywhere else…
@b4n approved this pull request.
LGTM as far as 0-testing goes
Is there anything wrong with merging this?
Even if it would be cooler to not have to set the flag, as it is done by Autotools as well, it might be ok.
And if uctags is updated next time, we could try to remove it again.
I don't see any reason not to merge this indeed, if it fixes something.
Since the changes in upstream ctags have been pulled into Geany already, the last use was in `ctags/gnu_regex/regex_internal.h` which is our code, if I'm not mistaken.
So, what do you think about #3878 to remove the `WIN32` macro completely? CI and manual builds on Windows with Autotools and Meson succeeded.
@eht16 no, it's ctags, or rather glibc 2.10. Apparently though upstream ctags switched to gnulib's in universal-ctags/ctags#3054, so we might want to follow that… which might or might not solve this.
Closed #3660 via #3878.
github-comments@lists.geany.org