Skip to content
This repository has been archived by the owner on Feb 20, 2024. It is now read-only.

Update wxWidgets project wizard #21

Closed
wants to merge 9 commits into from
Closed

Update wxWidgets project wizard #21

wants to merge 9 commits into from

Conversation

PBfordev
Copy link

Remove support for obsolete wxWidgets versions 2.6 and 2.8.

Improve linking with GCC on MSW in wxWidgets project wizard:

  • Allow linking with libraries introduced in wxWidgets 2.9:
    PropertyGrid, Ribbon, STC, and WebView.
  • Allow linking with OpenGL for the monolithic build too.
  • Do not offer to link with the 3rd party libraries (image formats,
    expat, regex, scintilla, zip), they are not needed for the
    DLL build and required for the static build.
  • Allow linking with Winsock2 instead of Winsock.

By default, assume Unicode will be used and the generated application
will be GUI instead of console one.

Make more options persistent and other minor improvements.

Remove support for obsolete wxWidgets versions 2.6 and 2.8.

Improve linking with GCC on MSW in wxWidgets project wizard:
* Allow linking with libraries introduced in wxWidgets 2.9:
PropertyGrid, Ribbon, STC, and WebView.
* Allow linking with OpenGL for the monolithic build too.
* Do not offer to link with the 3rd party libraries (image formats,
expat, regex, scintilla, zip), they are not needed for the
DLL build and required for the static build.
* Allow linking with Winsock2 instead of Winsock.

By default, assume Unicode will be used and the generated application
will be GUI instead of console one.

Make more options persistent and other minor improvements.
@wh11204
Copy link

wh11204 commented May 21, 2021

When debugging C::B with C::B I get this error in the first wizard page:

XRC error: 253: invalid long specification "1|wxEXPAND"

line 253 contains

<option>1|wxEXPAND</option>

There is also a crash when ending the wizard, but that also happens with the current wizard. I will post a ticket in Sourceforge.

I can't check the results of the wizard because of the crash, but I like the new version

@PBfordev
Copy link
Author

PBfordev commented May 21, 2021

When debugging C::B with C::B I get this error in the first wizard page:

Thanks, fixed. I have no idea where it came from.

There is also a crash when ending the wizard, but that also happens with the current wizard. I will post a ticket in Sourceforge.

I have never encountered a crash with the wizard shipped with C::B or the one with my changes. I also did not get any message about incorrect XRC property? I am running official 64-bit C::B 20.03 on Win10.

EDIT: I have just tried with the current nightly (svn-r12446), still no crash with either wizard. Perhaps there is a certain combination of parameters that makes it crash? I have tried many different combination of parameters but perhaps never hit the "right" one.

@wh11204
Copy link

wh11204 commented May 21, 2021

The crash was with r12450 and wxWidgets 3.1.5 in 32 bits mode., with this settings:

  • wxWidgets 3.1
  • wxSmith / Frame based
  • $(#wx31) (will ask for #wx later!)
  • GNU GCC / Only release
  • Check "Use wxWidgets DLL", "Enable Unicode", "Configure advanced"
  • "Use _WXDEBUG..." unchecked, select "GUI mode application"
  • Press Finish

I can't reproduce the crash on another computer with the same setup but in 64 bits and "Use monolithic wxWidgets" checked (because this computer does not have a non-monolithic build). Tomorrow I will check the original setup on a third computer.

BTW, can you check why both wizards force creation of a "wx" global variable even for wxWidgets 3.1 projects?

@PBfordev
Copy link
Author

PBfordev commented May 21, 2021

I could try only 64-bit C::B build and following your steps (however, after "Use _WXDEBUG..." there is a page with a library list before one can press Finish), I could not make the wizard crash.

Regarding $(#wx), I use it for any wxWidgets version, but for testing my wizard changes I used $(#wx315) pointing to another wxWidgets folder with no problems. I can also input a hard-coded path instead of a C::B variable and the wizard still works. So I do not understand what the issue is?

@obfuscated
Copy link
Owner

The main question is does the wizard still works on Linux?

I see many places where your requiring gcc based compiler.
I don't remember if LLVM was gcc based in the settings of C::B, but I think it is an important compiler and we shouldn't disallow it.
What will happen if I try to use LLVM instead of GCC?
Also GCC on windows is so broken, slow and cumbersome, that I don't know how people still use it. :)

@obfuscated
Copy link
Owner

@wh11204

The crash was with r12450 and wxWidgets 3.1.5 in 32 bits mode., with this settings:

Would it be possible to attach a debugger and tell us where exactly the crash happepns?

@obfuscated
Copy link
Owner

@wh11204

The crash was with r12450 and wxWidgets 3.1.5 in 32 bits mode., with this settings:

Would it be possible to attach a debugger and tell us where exactly the crash happepns?

OK, I see you've done this in the ticket...

@wh11204
Copy link

wh11204 commented May 22, 2021

I get the crash on the third computer using 32 bits. From now on I will post about the crash on the ticket.

Regarding the wx variable. the script forces me to create one even if I don't want/need it. This is the offending line:
Wizard.AddGenericSelectPathPage(_T("WxPath"), wxpath_msg, _T("wxWidgets' location:"), _T("$(#wx)"));

Checking if it exists before accesing it would fix the issue. This is used in other part of the script:
if (GetUserVariableManager().Exists(_T("#wx")))

@PBfordev
Copy link
Author

PBfordev commented May 22, 2021

The main question is does the wizard still works on Linux?

I do not use Linux and the changes I made, aside from removing the two outdated wxWidgets versions, are all MSW-only, as on Linux wx-config is used for setting-up the libraries. Nevertheless, I tried running it there and it seems to work the same as the non-modified wizard.

I see many places where your requiring gcc based compiler.

AFAICT, my changes in no way affect which compiler is required, they only extend what was there.

Also GCC on windows is so broken, slow and cumbersome, that I don't know how people still use it. :)

GCC is the compiler of choice for wxWidgets (see the selection of official binaries), and it seems Code::Blocks itself on MSW not only builds with it but also ships with it. As I wrote above, I have no experience with LLVM. I am not a programmer, I do not even know what it is, am just vaguely aware of clang.

@PBfordev
Copy link
Author

PBfordev commented May 22, 2021

Regarding the wx variable. the script forces me to create one even if I don't want/need it. This is the offending line:
Wizard.AddGenericSelectPathPage(_T("WxPath"), wxpath_msg, _T("wxWidgets' location:"), _T("$(#wx)"));

Checking if it exists before accesing it would fix the issue. This is used in other part of the script:
if (GetUserVariableManager().Exists(_T("#wx")))

I see now, thanks. As creating this variable when using wxWidgets is considered a good idea, I always had it there so I did not get any warnings. I suppose I could check if the variable exists but perhaps notifying the user that they should have this variable set was the intent here?

@PBfordev
Copy link
Author

PBfordev commented May 22, 2021

The original intent of the changes was updating the libraries on MSW to post-2.8 wxWidgets, making only the minimal necessary changes.

As I am not familiar with Squirrel, wizard API, platforms besides MSW and compilers except MSVC and GCC I still think that was a good idea. The code seemed friendly to C++ users, I thought improving the wizard could be possible, even considering all my limitations. But these limitations are the reason why I wanted to avoid extensive changes.

Assuming there is decent chance of the wizard update being merged, I am willing to clean up the code. However, the limitations are still there...

@obfuscated
Copy link
Owner

I see no reason we would ignore your changes.
Keep in mind that after you've resolved all the problems, you'll have to provide final descriptive commit message. We're using svn and all your commits would be merged in a single commit.

If you want to get attribution to some other name you'll also have to specify this explicitly at the moment I'll use PBfordev.

@wh11204
Copy link

wh11204 commented May 22, 2021

C::B used the "wx" variable for self-building using wxWidgets 2.8, builds with wx3.0 uses "wx30" and those with wx3.1 uses "wx31". Using wx content as path default was a good idea when wx2.8 was still around, but know "wx" may not exist or point to a wxWidgets folder different to the wx3.0 or wx3.1 you are creating the new project for,

I would use "wx30" or "wx31" (depending on selected wxWidgets version), if it exists use the value else if "wx" exists use the value else use an empty string.

@obfuscated
Copy link
Owner

I would use "wx30" or "wx31" (depending on selected wxWidgets version), if it exists use the value else if "wx" exists use the value else use an empty string.

I'm not sure I like this idea.
Currently the library names contains version numbers. If we can automate this somehow using global variables it will be best and then we could get rid of wx30 and wx31 even for C::B. But unfortunately the global variables are really hard to use if you don't know what is expected from you (the user). :(

@PBfordev
Copy link
Author

I think I am done for now, I will wait for the answer about using ConfigManager vs GetConfigManager().

BTW, are those ugly _T() around the strings still needed in scripts? If not, I will happily remove them from everywhere in this wizard.

@obfuscated
Copy link
Owner

BTW, are those ugly _T() around the strings still needed in scripts? If not, I will happily remove them from everywhere in this wizard.

They are needed if you're targeting 20.03 for sure.
In master you need to construct wxStrings somehow when you want to pass the string to some of the methods in the C::B API which expects wxStrings. Most of the functions expecting wxString don't have a way to convert Squirrel strings to wxString, yet. It might be possible in the future, but currently it is not.

@PBfordev
Copy link
Author

PBfordev commented May 23, 2021

I believe I have resolved all issues so the code is finished.

Keep in mind that after you've resolved all the problems, you'll have to provide final descriptive commit message.

I'd like the commit be titled Update wxWidgets project wizard with message:

Remove support for obsolete wxWidgets versions 2.6 and 2.8.

Improve linking with GCC on MSW in wxWidgets project wizard:
* Allow linking with libraries introduced in wxWidgets 2.9:
PropertyGrid, Ribbon, STC, and WebView.
* Allow linking with OpenGL for the monolithic build too.
* Do not ask whether to link with the 3rd party libraries (image formats,
expat, regex, scintilla, zip), they are not needed for the
DLL build and required for the static build.
* Allow linking with Winsock2 instead of Winsock.

By default, assume Unicode will be used and the generated application
will be GUI instead of console one.

Make more options persistent and minor code improvements.

If you want to get attribution to some other name you'll also have to specify this explicitly at the moment I'll use PBfordev.

If possible I go just by "PB", my Username on SourceForge is pbfordev.

@obfuscated
Copy link
Owner

@wh11204 Do you see anything that needs to be changed in this wizard?

@wh11204
Copy link

wh11204 commented Jun 21, 2021

I just would like to avoid forced creation of wx variable, as commented above

@obfuscated
Copy link
Owner

I just would like to avoid forced creation of wx variable, as commented above

But what should happen if there is now such variable? You should be obliged/forced to enter a valid path to the wx install folder?
OK, @PBfordev can you look at this?

A failed attempt to use global variable wx as wxWidgets location only when
the variable exists.

[CI Skip]
@PBfordev
Copy link
Author

Regarding forcing wx variable, I wanted to use it only when it exists, see the last commit. However, it seems that just testing for an existence of a global variable with GetUserVariableManager().Exists(_T("#SomeVariable") always creates an empty global variable SomeVariable.

Am I seeing things? If not, I am not sure how to proceed.

@obfuscated
Copy link
Owner

@PBfordev Is this change in behaviour compared to the wizard without your changes?
@wh11204 Same question: is the current wizard working without forcing you to define #wx?

@wh11204
Copy link

wh11204 commented Jun 22, 2021

The current wizard does force creation of the wx variable, just after indicating wxWidgets location is $(#wx31) and clicking Next

@obfuscated
Copy link
Owner

The current wizard does force creation of the wx variable, just after indicating wxWidgets location is $(#wx31) and clicking Next

@wh11204 So this is a problem in trunk and we have to solve this separately?

@PBfordev
Copy link
Author

@PBfordev Is this change in behaviour compared to the wizard without your changes?

Sorry, I do not understand. As I wrote, executing GetUserVariableManager().Exists(_T("#SomeVariable") leads to creating of global variable SomeVariable, regardless of my changes in the wizard script.

FWIW, I could not find string "UserVariableManager" in any other wizard besides this one.

@PBfordev
Copy link
Author

PBfordev commented Jun 22, 2021

The current wizard does force creation of the wx variable, just after indicating wxWidgets location is $(#wx31) and clicking Next

I think this is because the variable is, probably unintentionally for the reason I describe above, created here:
https://github.com/obfuscated/codeblocks_sf/blob/master/src/plugins/scriptedwizard/resources/wxwidgets/wizard.script#L230

@wh11204
Copy link

wh11204 commented Jun 23, 2021

So this is a problem in trunk and we have to solve this separately?

Yes. If I change part of the code in function OnLeave_WxPath(fwd) to

if (GetUserVariableManager().Exists(_T("#wx0"))) { local gvar = ReplaceMacros(_T("$(#wx1)")); if (gvar.Matches(dir_nomacro)) dir = gvar; }
then an empty wx0 variable is silently created and I am asked to create a wx1 variable. As PBfordev said here, Exists() does create the variable and return true, The creation of wx1 is just a side effect.

@wh11204
Copy link

wh11204 commented Jun 23, 2021

I found the problem, line 231 of src\sdk\uservarmanager.cpp must be changed from

return !m_CfgMan->Exists(cSets + m_ActiveSet + _T('/') + member + _T("/base"));

to

return m_CfgMan->Exists(cSets + m_ActiveSet + _T('/') + member + _T("/base"));

i.e. remove the !

@wh11204
Copy link

wh11204 commented Jun 23, 2021

The solution above fixes the detection, so wx1 is not created, but wx0 is still silently created.

EDIT; the call to AssertPath() within Exists() in configmanager.cpp:1129 creates the variable.

@obfuscated
Copy link
Owner

@PBfordev I've rebased, edited and prepared your changes for merge/push/commit. They can be found in https://github.com/obfuscated/codeblocks_sf/tree/experiments/wxwidgets_wizard

I've removed the stuff related to the GetUserVariableManager().Exists(_T("#wx")) call, because it looked suspicious to me. After testing I've found and I've fixed a problem in the dialog, so I now think it works as expected.

Please retest and let me know if I broke something.

@wh11204 Please do the same.

@PBfordev
Copy link
Author

I have realized there is a cosmetic bug in the wizard. The checkbox for linking Winsock2 should be disabled when linking wxWidgets dynamically. However, due to wrong case of the XRC variable, this does not happen for the multilib build. I am not sure if I should make a PR with the fix (I have changed the case in the script as well as the XRC file to make it same for both monolithic and multilib build).

I've removed the stuff related to the GetUserVariableManager().Exists(_T("#wx")) call, because it looked suspicious to me. After testing I've found and I've fixed a problem in the dialog, so I now think it works as expected.

If the bug which creates a global variable when just querying its existence is fixed, the test makes sense to me. I believe it is better to check for the existence of this variable before using it for the path, but I guess it's just me.

I cannot test the changes, as I have not yet learnt how to build C::B, I hope I will make it next week.

@obfuscated
Copy link
Owner

I have realized there is a cosmetic bug in the wizard. The checkbox for linking Winsock2 should be disabled when linking wxWidgets dynamically. However, due to wrong case of the XRC variable, this does not happen for the multilib build. I am not sure if I should make a PR with the fix (I have changed the case in the script as well as the XRC file to make it same for both monolithic and multilib build).

Just push the fix in this PR and I'll pick it up.

I've removed the stuff related to the GetUserVariableManager().Exists(_T("#wx")) call, because it looked suspicious to me. After testing I've found and I've fixed a problem in the dialog, so I now think it works as expected.

If the bug which creates a global variable when just querying its existence is fixed, the test makes sense to me. I believe it is better to check for the existence of this variable before using it for the path, but I guess it's just me.

The check for existence is done internally by the wizard page in the on-page-change/changing event when the path is actually used, so I think all is good as far as I can test.

@PBfordev
Copy link
Author

PBfordev commented Jun 29, 2021

I will submit a PR with the fix for the cosmetic bug.

BTW, does the current C::B code in the master branch in this repo (as of commit da3cdd7) build? I did just try (CodeBlocks_wx31.cbp, wxWidgets 3.1.5, GCC 10.3) and failed but I may have done something wrong so I do not dare to ask in the forum: https://pastebin.com/VKedNZgF

@obfuscated
Copy link
Owner

I will submit a PR with the fix for the cosmetic bug.

In this PR please. Too many PRs is too annoying.

BTW, does the current C::B code in the master branch in this repo (as of commit da3cdd7) build? I did just try (CodeBlocks_wx31.cbp, wxWidgets 3.1.5, GCC 10.3) and failed but I may have done something wrong so I do not dare to ask in the forum: https://pastebin.com/VKedNZgF

Are you following the build instructions https://wiki.codeblocks.org/index.php/Installing_Code::Blocks_from_source_on_Windows ? Are you somehow enabling STL for wxWidgets' containers?

@PBfordev
Copy link
Author

PBfordev commented Jun 30, 2021

Are you somehow enabling STL for wxWidgets' containers?

Ooops, I have reused a wxWidgets build without realizing it was customized. No problem with a vanilla build (aside from C::B not being DPI aware, unlike 20.03), sorry for the noise.

1. Incorporate the changes in wizard script and resource from branch experiments/wxwidgets_wizard.
2. Fix case in the name of resource file object.
@PBfordev
Copy link
Author

PBfordev commented Jun 30, 2021

@obfuscated, as you wished, I have pushed the bug fix (changing a single letter in the XRC file) to this PR, the changes you made in the experimental branch are included.

I have noticed that when global variable wx does not exist, is used as wxWidgets location, and Next is pressed in the wizard; the Global Variable dialog is shown and the variable is created. I assume this is by design.

@obfuscated
Copy link
Owner

@obfuscated, as you wished, I have pushed the bug fix (changing a single letter in the XRC file) to this PR, the changes you made in the experimental branch are included.

It would have been better if you've included only your changes or you've applied them as separate commits, but I'll handle it.

I have noticed that when global variable wx does not exist, is used as wxWidgets location, and Next is pressed in the wizard; the Global Variable dialog is shown and the variable is created. I assume this is by design.

Yes, this is how it is supposed to work. If you remove $(#wx), specify a path and press next you must not be asked for the definition of the wx variable.

@obfuscated
Copy link
Owner

In svn... later in github... Thanks for contribution.

@obfuscated obfuscated closed this Jun 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants