[OSX] Compile osx with wxwidgets v3.1.7

[talregev]

Compile osx with wxwidgets v3.1.7.
wxwidgets v3.1.6 (hash 1990792) Is a dangerous version and we should not use it.
In the same day wxwidgets have a patch version v3.1.6-rc2 (hash 35a6d7b)
and after 2 days they have v3.1.6-final (hash 35a6d7b).
From here:
https://github.com/wxWidgets/wxWidgets/tags
Next release version is 3.1.7.

@davidpanderson For your notification.

[BrianNixon]

In what way is 3.1.6 “dangerous”?

There are no differences at all between 3.1.6 and 3.1.6-final affecting Mac:
https://github.com/wxWidgets/wxWidgets/compare/v3.1.6..v3.1.6-final

[CharlieFenton]

@talregev If you are referring to this PR: Fix memory leak in ListBox destructor, the file changed by that PR is not used by the MacOS build of wxWidgets.

[talregev]

Any reason not to switch to 3.1.7?

[CharlieFenton]

Any reason not to switch to 3.1.7?

Yes:

• It increases the likelihood of introducing unexpected problems.
• It requires more changes than just updating the dependencyNames.sh file.
  In fact, I am trying to understand why it passed the CI build checks. It will not build on my Mac if I only change the one file as you did. I suspect that the cache may contain wxWidgets-3.1.6 in addition to wxWidgets-3.1.7, but the log does not give me enough information.

Also, please respond to the question from @BrianNixon to show why this is needed:

In what way is 3.1.6 “dangerous”?

Thank you.

[AenBleidd]

@CharlieFenton,

In fact, I am trying to understand why it passed the CI build checks.

Answered you privately in the email. Please check

[talregev]

@CharlieFenton,

In fact, I am trying to understand why it passed the CI build checks.

Answered you privately in the email. Please check

Why in privacy?
The cache is invalidate when I change the dependencyNames.sh. https://github.com/BOINC/boinc/blob/master/.github/workflows/osx.yml#L34
I also compile our code with wxwidgets 3.1.7 vcpkg that clearly don't take the binaries from OSX cache.
Our code is compatible to 3.1.7. I guess it something in your local system.

[AenBleidd]

@talregev

Why in privacy?

Because it's politely to answer the same way the question was asked.
I just mentioned here to give a clear understanding to everyone that this question is answered and not an issue with our CI.

And sometimes people like to discuss something privately using old good emails.

[CharlieFenton]

Why in privacy?

Because I didn't want to get into yet another argument with you.

As I have told you before, I have purposely hard coded the dependency version numbers into the Xcode project file so that it will fail to build with any different version of those files. In fact it does fail with several "File not found" errors on my Mac when I remove the wxWidgets-3.1.6 and provide only wxWidgets-3.1.7.

The CI build should also fail with "file not found" errors. Due to the xcpretty "beautifier" that buildMacBOINC-CI.sh uses, I was not able to tell exactly what the build did. Since there were no errors, it did not produce a full log artifact (which would not be piped through the "beautifer".)

@AenBleidd has shown me why this protection I built in did not work in the CI build. It is because the CI build script overrides the Xcode settings to accept whatever versions are in the Cache. I will have to fix that.

As I have told you before, I feel very strongly that it is important to have strict control over the versions of dependent libraries used with any given version of BOINC, to prevent introducing hard to reproduce bugs caused by the interaction between our code and the dependent libraries . @talregev I know you disagree and I don't want to get into an argument with you about this, but this is why I don't like the idea of using vcpkg to automatically update all dependent libraries to the latest version. Your concern that wxWidgets-3.1.6 is "dangerous" actually demonstrates the potential harm from vcpkg.

Note also lines 4560-4561 here which indicate that my custom patch should be adjusted for wxWidgets-3.1.7 (even though the patch utility figured it out):

patching file src/osx/cocoa/choice.mm
4561Hunk #1 succeeded at 93 (offset -37 lines).

[CharlieFenton]

I have been asked to review this. I have already written my review here. But mainly, I am still waiting for an answer to this question. If there is no need to do this, we should not do it.

[CharlieFenton]

@talregev If you clearly show us that wxWidgets 3.1.6 is dangerous on the Mac, I will create a PR to update the Mac build correctly. Otherwise, you are wasting my time and that of @BrianNixon and @AenBleidd.

[talregev]

As I have told you before, I feel very strongly that it is important to have strict control over the versions of dependent libraries used with any given version of BOINC, to prevent introducing hard to reproduce bugs caused by the interaction between our code and the dependent libraries . @talregev I know you disagree and I don't want to get into an argument with you about this, but this is why I don't like the idea of using vcpkg to automatically update all dependent libraries to the latest version. Your concern that wxWidgets-3.1.6 is "dangerous" actually demonstrates the potential harm from vcpkg.

I am not against your protection on the code. If there is a such protection it should also be on the CI for other devs or users that compile OSX code. I will request from you to add this protection to the CI.
As I understand from the time that wxwidgets have upload the new version, is not safe. If you tell me it safe for osx, I will believe you. I will also recommend that if we already switch the new library from 3.1.5 to 3.1.6 is the same risks to switch 3.1.5 to 3.1.7. Will need to do the same checks.

Beside your protection there is no need to do code change in boinc code.

If we talking about vcpkg. there is a mode that it will not update automatic the libraries and it will be forzen in time forever. Until we decide later. If we update automatic in alll other os, it doesn't mean that we need or cannot froze it for osx.
I don't understand your insist that I can control in vcpkg the way we wanted.

For vcpkg I can control and give easily any older version that was present in vcpkg.
I Can give you list of libraries for osx, and you can choose any old version for each library that you want for compile for vcpkg.

p.s. wxwidgets 3.1.7 is not the latest for vcpkg.

[talregev]

The CI build should also fail with "file not found" errors. Due to the xcpretty "beautifier" that buildMacBOINC-CI.sh uses, I was not able to tell exactly what the build did. Since there were no errors, it did not produce a full log artifact (which would not be piped through the "beautifer".)

We should switch to cat instead beautifer and tell the ci to fail on any error. otherwise it lead to errors in our code.
We already had a bug that we didn't catch because of it. @AenBleidd told me that he was fixed that bug, but we should fix it again.

[AenBleidd]

@talregev,
CI fails on error.
Currently, CI just builds everything from the cache folder, and local builds with XCode use different paths of libraries.
That is why this PR didn't fail.
There is nothing to change on CI to show build errors because there are no build errors currently.

@CharlieFenton, plans to rework the way CI builds to make it as close to the local builds as possible.

[talregev]

@BrianNixon in wxwidgets 3.1.6-final they fix a security hole. Are you sure it not also exploit in osx?
I want to be in the safe side.

[BrianNixon]

they fix a security hole

Where?
https://github.com/wxWidgets/wxWidgets/compare/v3.1.6..v3.1.6-final

If you’re referring to this:

-  security fixed and support for WebKit 2 and GStreamer 1.7 under Unix.
+  security fixes and support for WebKit 2 and GStreamer 1.7 under Unix.

that’s a typo fix, not a security fix…

[talregev]

they fix a security hole

Where? https://github.com/wxWidgets/wxWidgets/compare/v3.1.6..v3.1.6-final

If you’re referring to this:

-  security fixed and support for WebKit 2 and GStreamer 1.7 under Unix.
+  security fixes and support for WebKit 2 and GStreamer 1.7 under Unix.

that’s a typo fix, not a security fix…

Of course no the typo, they fix the SHA-1 checksums. They convert the define to long, also they use other init window function.

[BrianNixon]

fix the SHA-1 checksums

That’s only documentation

convert the define to long…

Those only affect Windows

I grant you this looks like less-than-perfect release management from wxWidgets…

[talregev]

Thank you for your replay.

[AenBleidd]

@talregev, just a note to this PR.

1. If you see that there is a security issue with any of the dependency libraries we use, and there is a new version of it - please open an issue to discuss, whether this update is required.
2. The previous point is especially important, when it's related to the MacOS, because you have no possibility to test your changes on your side, and that fact that your PR builds fine doesn't mean that in general nothing was broken.
3. If you open a PR - please be sure to test it. If you can't test it - either ask someone else to do the implementation (by opening a ticket) or at least ask project maintainers if this is needed at all. By doing that you will save a lot of time both on your side and on the side of project maintainers.

Thank you for understanding.