Fix wxMSW build errors when wxUSE_REGKEY=0 (PR #26332)
stahta01
Tested with
`# Testing done under MSys2 UCRT64 enviroment
cd "C:\devel\repos\wxwidgets_libs\wxWidgets"
rm -fR cmake_build_msw_shared_unicode
mkdir -p cmake_build_msw_shared_unicode
cmake
-DwxBUILD_TOOLKIT="msw"
-DBUILD_SHARED_LIBS="ON"
-DwxBUILD_SHARED="ON"
-DwxUSE_REGKEY="OFF" -DwxUSE_MIMETYPE="OFF" -DwxUSE_WEBVIEW="OFF"
-S .
-B cmake_build_msw_shared_unicode
cmake --build cmake_build_msw_shared_unicode
`
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26332
Commit Summary
- 2cbfb3e Fix wxMSW build errors when wxUSE_REGKEY=0
File Changes
(7 files)
- M include/wx/config.h (2)
- M include/wx/msw/combo.h (4)
- M src/common/datetime.cpp (2)
- M src/msw/joystick.cpp (2)
- M src/msw/settings.cpp (4)
- M src/msw/uilocale.cpp (2)
- M src/msw/utilswin.cpp (4)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26332.patch
- https://github.com/wxWidgets/wxWidgets/pull/26332.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)I did not know what to do with the MIMETYPE and WEBVIEW build errors.
—
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#26332)Looks good to me, thanks.
I did not know what to do with the MIMETYPE and WEBVIEW build errors.
MIME types can't be meaningfully supported without registry access, so I think wxUSE_MIMETYPE should be set to 0 in this case. I am not sure that is the "WEBVIEW build error"?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)Part of the error was this; I am building again to get full error
wxWidgets/src/msw/webview_ie.cpp:66:13: error: 'wxRegKey' was not declared in this scope
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)Building with [-DwxUSE_REGKEY="OFF" -DwxUSE_MIMETYPE="OFF" -DwxUSE_WEBVIEW_IE="OFF"] built okay
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lauri Nurmi
lanurmi left a comment (wxWidgets/wxWidgets#26332)Out of curiosity, is there some real-world advantage in building with wxUSE_REGKEY disabled?
—
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#26332)I thought you would want to add the logic for disabling the components that don't compile without it to wx/chkconf.h, do you plan to do it? If not, I can merge this as is.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)I plan to do a single PR that does that in the future once I find all the things that need done. I have a few other related things I am testing to make wxWidgets to build and run under MSys2 /usr/bin Cygwin like environment.
—
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#26332)FWIW I am pretty sure that wx supports MSYS2 just fine if you simply use the default build options.
I also don't understand why do you want to disable wxRegKey and while I don't want to prevent you from fixing build problems due to it, this is definitely a very low priority.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)FWIW I am pretty sure that wx supports MSYS2 just fine if you simply use the default build options.
I also don't understand why do you want to disable
wxRegKeyand while I don't want to prevent you from fixing build problems due to it, this is definitely a very low priority.
The MSys2 has either MINGW environments or the CYGWIN like environment that I am trying to build for right now.
And I am finding some corner cases that I verify exists under the UCRT64 MINGW enviroment.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)I have decided to start the work in this PR to do the checks; might be a while before I figure out how to do it. It been over 5 years since I looked at those files.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 1 commit.
- 6bf83df Add aborts when wxUSE_REGKEY=0
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)I found #14514 issue and think I can add a fix to that to this PR. Do it in this PR or in a separate PR after this one?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 3 commits.
- b01c07b Fix wxMSW build errors when wxUSE_REGKEY=0
- 646d361 change wxUSE_CONFIG_NATIVE setting logic in
- 726e8d7 Add aborts when wxUSE_REGKEY=0
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 1 commit.
- dbe3c79 Add wxUSE_MIMETYPE guard to samples
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26332)I added the needed changes to this PR; if needed I can remove them.
This is fine, please feel free to add any other fixes for building with wxUSE_XXX=0 here too. Just don't forget to remove the draft status when this is ready.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)Everything but a single task has finished marking as ready because that appears to be taking a long time to start running.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz approved this pull request.
Mostly looks good, thanks, but I wonder if a couple of the changes are really necessary now.
> @@ -20,7 +20,7 @@ // under Windows we prefer to use the native implementation but can be forced // to use the file-based one -#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE +#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE && wxUSE_REGKEY
Is this still needed after the changes in wx/msw/chkconf.h? AFAICS it shouldn't be.
In samples/regtest/regtest.cpp:
> @@ -25,7 +25,7 @@ #include "wx/imaglist.h" #include "wx/tokenzr.h" -#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) +#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) && wxUSE_REGKEY
Same question as for the change wx/config.h.
In samples/typetest/typetest.h:
> + MyApp()
+ {
+ m_textCtrl = nullptr;
+#if wxUSE_MIMETYPE
+ m_mimeDatabase = nullptr;
+#endif // wxUSE_MIMETYPE
+ }
Ideal would be to replace this with
MyApp() = default;and initialize the variables in their declarations, this would reduce the amount of #ifs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 commented on this pull request.
In samples/typetest/typetest.h:
> + MyApp()
+ {
+ m_textCtrl = nullptr;
+#if wxUSE_MIMETYPE
+ m_mimeDatabase = nullptr;
+#endif // wxUSE_MIMETYPE
+ }
I am a good C programmer but a bad C++ programmer so what you posted is not something I really understand.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 commented on this pull request.
In samples/regtest/regtest.cpp:
> @@ -25,7 +25,7 @@ #include "wx/imaglist.h" #include "wx/tokenzr.h" -#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) +#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) && wxUSE_REGKEY
I will test and see if it works without it
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 commented on this pull request.
> @@ -20,7 +20,7 @@ // under Windows we prefer to use the native implementation but can be forced // to use the file-based one -#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE +#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE && wxUSE_REGKEY
I will test and see if it works without it
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 7 commits.
- 69b8d2d Fix wxMSW build errors when wxUSE_REGKEY=0
- 9377ec7 change wxUSE_CONFIG_NATIVE setting logic in
- e16c6ce Add aborts when wxUSE_REGKEY=0
- aabed97 Fix build of sample when wxUSE_REGKEY=0
- b40c110 Add wxUSE_MIMETYPE guard to samples
- 3bd3baa Build fixes when wxUSE_INTL=0
- f3af038 Fix sample builds for wxUSE_INTL=0
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 commented on this pull request.
> @@ -20,7 +20,7 @@ // under Windows we prefer to use the native implementation but can be forced // to use the file-based one -#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE +#if defined(__WINDOWS__) && wxUSE_CONFIG_NATIVE && wxUSE_REGKEY
It tested okay on my local PC so pushed the changes
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 commented on this pull request.
In samples/regtest/regtest.cpp:
> @@ -25,7 +25,7 @@ #include "wx/imaglist.h" #include "wx/tokenzr.h" -#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) +#if wxUSE_CONFIG_NATIVE && defined( __WINDOWS__ ) && wxUSE_REGKEY
It tested okay on my local PC so pushed the changes, this one failed on the cross compiler once maybe it will work this time. It has always worked on my local PC.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)Out of curiosity, is there some real-world advantage in building with
wxUSE_REGKEYdisabled?
I am building wxWidgets for the MSYS2 Cygwin like environment. At first, it shut off wxUSE_REGKEY and other things till I got most of them fixed. And I decided that wxUSE_REGKEY being off might be a good idea. I have gotten both wxGTK and wxMSW to build. But minimal only works with wxMSW because my gtk3 appears to be broken.
Tim S.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
stahta01 left a comment (wxWidgets/wxWidgets#26332)I have no idea what to do with "samples/typetest/typetest.h"
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
> @@ -2055,7 +2055,7 @@ bool wxFileProperty::DisplayEditorDialog(wxPropertyGrid* pg, wxVariant& value)
m_dlgTitle.empty() ? _("Choose a file") : m_dlgTitle,
m_initialPath.empty() ? path : m_initialPath,
file,
- m_wildcard.empty() ? wxGetTranslation(wxALL_FILES) : m_wildcard,
+ m_wildcard.empty() ? wxString(wxGetTranslation(wxALL_FILES)) : m_wildcard,
Maybe we should let wxGetTranslation always return a wxString instead of the input type / TString.
Since the return type changed from const wxString& to wxString in #26196, this comment above the functions seems obsolete and there is no need to return TString in this and the functions below it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
I've left suggestions that can be applied to do what I meant in the typetest sample.
In samples/typetest/typetest.h:
> + MyApp()
+ {
+ m_textCtrl = nullptr;
+#if wxUSE_MIMETYPE
+ m_mimeDatabase = nullptr;
+#endif // wxUSE_MIMETYPE
+ }
- MyApp()
- {
- m_textCtrl = nullptr;
-#if wxUSE_MIMETYPE
- m_mimeDatabase = nullptr;
-#endif // wxUSE_MIMETYPE
- }
+ MyApp() = default;
In samples/typetest/typetest.h:
> @@ -35,7 +47,9 @@ class MyApp: public wxApp
private:
wxTextCtrl* m_textCtrl;
⬇️ Suggested change
- wxTextCtrl* m_textCtrl; + wxTextCtrl* m_textCtrl = nullptr;
In samples/typetest/typetest.h:
> @@ -35,7 +47,9 @@ class MyApp: public wxApp
private:
wxTextCtrl* m_textCtrl;
+#if wxUSE_MIMETYPE
wxMimeTypesManager *m_mimeDatabase;
⬇️ Suggested change
- wxMimeTypesManager *m_mimeDatabase; + wxMimeTypesManager* m_mimeDatabase = nullptr;
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
stahta01
@stahta01 pushed 1 commit.
- 083873b Update samples/typetest/typetest.h
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
