Define wxUSE_GUI=0 when using only base libraries with CMake (PR #26074)
VZ
This fix consists of two parts: first, don't define toolkit definitions (such as WXMSW, WXGTK3 etc) when compiling targets using non-GUI libraries only. And second, set wxUSE_GUI to 0 if it's not defined and the toolkit symbol is not defined either in CMake-specific setup.h.
Note that we still keep the now useless fallback definition of wxUSE_GUI in build/cmake/setup.h.in just because it's simpler to keep this in all the setup.h files generated by build/update-setup-h than.
Closes #26043.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26074
Commit Summary
- c2aa1be Define wxUSE_GUI=0 when using only base libraries with CMake
File Changes
(2 files)
- M build/cmake/functions.cmake (14)
- M build/cmake/setup.h.in (10)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26074.patch
- https://github.com/wxWidgets/wxWidgets/pull/26074.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)Besides the CI errors, the following test also fails for me.
Use CMake to create a msvc solution, and build the wxrc utility. It only links to wxxml and wxbase libraries so no __WXMSW__ is defined. All builds fine.
Then modify wxrc.cpp with the patch below. I'd expect it to still build fine, but it fails because wxUSE_GUI is defined.
This happens because it includes platform.h which defines __WXMSW__ before including setup.h.
diff --git "a/utils/wxrc/wxrc.cpp" "b/utils/wxrc/wxrc.cpp" index 73bd8b287b8..dbec0570247 100644 --- "a/utils/wxrc/wxrc.cpp" +++ "b/utils/wxrc/wxrc.cpp" @@ -18,6 +18,10 @@ #include "wx/wxcrtvararg.h" #endif +#if wxUSE_GUI +#error gui defined in console app +#endif + #include "wx/cmdline.h" #include "wx/xml/xml.h" #include "wx/ffile.h"
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)Besides the CI errors,
Sorry, I should have waited for the CI builds to finish...
However I think that these failures indicate real problems that should be fixed in any case:
- For wxGTK and wxOSX build failures, we shouldn't include GUI-only headers such as
wx/colour.hifwxUSE_GUI==0. This was just hidden before, but it was always invalid. - For wxiOS build failure, we should test for the platform, i.e.
__WXDARWIN_IPHONE__instead of the toolkit (wxOSX_USE_IPHONE).
There is also the usual confusion between __WXOSX__ which is used both as toolkit and platform... Untangling this risks being tricky, but should be useful.
the following test also fails for me.
Just to be clear: this is not due to my changes, right? I.e. this was already the case before AFAICS and not just with CMake but with anything else too.
Use CMake to create a msvc solution, and build the
wxrcutility. It only links towxxmlandwxbaselibraries so no__WXMSW__is defined. All builds fine.Then modify
wxrc.cppwith the patch below. I'd expect it to still build fine, but it fails becausewxUSE_GUIis defined. This happens because it includesplatform.hwhich defines__WXMSW__before includingsetup.h.
I think this is the real problem, it shouldn't be doing this if wxUSE_GUI==0.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 4 commits.
- 36a08c4 Merge testdate.h into asserthelper.h
- 76cec3f Don't include GUI headers when building console test
- 9d218d8 Check for __WXDARWIN_IPHONE__ when defining wxSystem()
- 5135fea Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)There is also the usual confusion between WXOSX which is used both as toolkit and platform... Untangling this risks being tricky, but should be useful.
I had to check const_cpp.h for all the MAC/OSX/DARWIN defines, and mostly to inform myself:
__WXOSX_COCOA__and__WXOSX_IPHONE__are the toolkits__WXOSX__(previously__WXMAC__) is documented as both toolkits, but configure and cmake only define it for cocoa- The
DARWINdefines are for the platform (and come fromplatform.h) and are used a few times in wx code
Just to be clear: this is not due to my changes, right? I.e. this was already the case before AFAICS and not just with CMake but with anything else too.
It was indeed already the case before. I only tested CMake, not other build systems.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)I think that to solve the problem shown by your wxrc example we need to replace the checks for __WXMSW__ in wx/msw/setup.h with the tests for __WINDOWS__ and then include wx/setup.h before defining __WXMSW__.
Do you (or anybody else) see(s) any problems with doing this?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)wxiOS now fails due to undefined wxMimeTypesManager::GetFileTypeFromExtension (and a couple of other similar) symbol(s) but I don't see why: it is defined in src/common/mimecmn.cpp which is part of base and so is definitely linked in.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)Do you (or anybody else) see(s) any problems with doing this?
I think that will work, I don't see other #if checks what would be affected.
wxiOS now fails due to undefined wxMimeTypesManager::GetFileTypeFromExtension
There is also this warning:
/Users/runner/work/wxWidgets/wxWidgets/src/common/filesys.cpp:55:19: warning: unused variable 'mime' [-Wunused-variable]
which would indicate wxUSE_MIMETYPE is disabled. And indeed:
https://github.com/wxWidgets/wxWidgets/blob/1b582af60d99388b56b383fe240b647387a5e4b4/include/wx/osx/iphone/chkconf.h#L31-L34
But I haven't figured out why wxrc.cpp thinks it is enabled.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)Probably related to the base libraries not having any toolkit defines, and then __WXMAC__ wont be defined. And then it wont include the platform specific chkconf.h:
https://github.com/wxWidgets/wxWidgets/blob/1b582af60d99388b56b383fe240b647387a5e4b4/include/wx/platform.h#L394-L401
https://github.com/wxWidgets/wxWidgets/blob/1b582af60d99388b56b383fe240b647387a5e4b4/include/wx/chkconf.h#L1271-L1272
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)Thanks, this must be it, indeed. I'll return to this tomorrow as this clearly requires more changes...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 3 commits.
- cafa693 Fix test for macOS 10.14
- e2e4ff2 Improve macOS/iOS chkconf.h headers handling
- e240730 Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 4 commits.
- fbbbf7b Set deployment target to 10.13 for CMake Xcode CI build
- d94975a Remove redundant __WXOSX__ test from wx/evtloop.h
- 4e820eb Fix using wxConsoleEventLoop in non-GUI applications under macOS
- 849f2b8 Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 2 commits.
- 7d44298 Fix using wxConsoleEventLoop in non-GUI applications under macOS
- 6bfc45d Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 2 commits.
- 894a04f Fix using wxConsoleEventLoop in non-GUI applications under macOS
- e20bde4 Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz pushed 2 commits.
- 2f15e8a Test for Apple platforms, not toolkits, in non-GUI tests
- 615b68b Define wxUSE_GUI=0 when using only base libraries with CMake
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)This seems to finally work, so I'd like to merge it. Any reviews would be welcome, of course!
BTW, @barracuda156 this could help with some issues you have been having (or, alternatively, this could result in new and exciting breakage, of course) and shows the right way of testing for the Apple platforms, I believe.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)Nice, I'll try it again. Do you also plan to remove the toolkit defines from the base libraries in makefile/configure/wx-config and the Visual Studio / XCode projects?
Because usually I try to have CMake do the same thing as configure.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26074)Nice, I'll try it again.
Thanks! I'll wait for you to retest this just in case you find any new problems.
Do you also plan to remove the toolkit defines from the base libraries in makefile/configure/wx-config and the Visual Studio / XCode projects? Because usually I try to have CMake do the same thing as configure.
We can do it for the projects (although I'd rather not touch Xcode ones), but I'm not sure about wx-config output — this would be an incompatible change and it doesn't really fix any problems, unlike the change to CMake.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26074)We can do it for the projects (although I'd rather not touch Xcode ones), but I'm not sure about wx-config output — this would be an incompatible change and it doesn't really fix any problems, unlike the change to CMake
OK, then I guess it is fine to only have it in CMake.
I didn't find new problems, but I'm only testing on Windows.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Sergey Fedorov
barracuda156 left a comment (wxWidgets/wxWidgets#26074)@vadz Thank you, I will try this in a few days.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.