[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
Closed #3660 via #3878. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#event-13090818721 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
@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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2121229911 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2120615938 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2070878701 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
@b4n approved this pull request. LGTM as far as 0-testing goes -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#pullrequestreview-2015727530 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
@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… -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#pullrequestreview-2015726694 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> ``` > $ 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2054056772 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2054055869 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> > 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 ``` -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2054054773 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> This underlying issue has been fixed in upstream ctags repository. Ok, will get included when the ctags is next updated from upstream. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2038623056 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
This underlying issue has been fixed in upstream ctags repository. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2037147774 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2036167518 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
Is `_WIN32` set for cross builds? For Geany Windows builds are usually cross built on Linux. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2035767733 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-2035221498 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
> 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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-1784261659 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: correctly set WIN32 in config.h (PR #3660)
An alternative would be to replace all checks for WIN32 in the code with _WIN32, but there are lots of places.. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3660#issuecomment-1781180150 You are receiving this because you are subscribed to this thread. Message ID: