search

vslavik / winsparkle

App update framework for Windows, inspired by Sparkle for macOS
http://winsparkle.org
Other
1.31k stars 267 forks source link

Upgrade wxWidgets to 3.2.1 #249

Closed eagleoflqj closed 1 year ago

eagleoflqj commented 1 year ago

Motivation: support building winsparkle on win arm64. Rough steps I took:

eagleoflqj commented 1 year ago

This may (or may not) fix #227 and #224

vslavik commented 1 year ago

This may (or may not) fix https://github.com/vslavik/winsparkle/issues/227 and https://github.com/vslavik/winsparkle/issues/224

This is a necessary, but not sufficient, step towards that.

I'm seeing some weird things in the diff here - the project files is clearly changed in much more significant ways than updating wx, and I'm pretty sure is broken as the result. I also don't understand some of the changes in setup.h - do you remember why you had to enabled some extra features? Generally the fewer are enabled, the better, as it results in smaller binaries.

vslavik commented 1 year ago

fix build for vs2017

Is this related to the other changes, i.e. was vs2017 support previously working?

eagleoflqj commented 1 year ago

I'm pretty sure is broken as the result.

Why? CI passes for vs2017 and vs2019, and my local vs2022 also works. The diff of project files is largely due to using the latest version of bakefile (Why use the latest? Because it claims to support vs2022). If you regenerate project files on my branch, you will notice a even bigger diff, which really makes the build fail, and I reverted them.

I also don't understand some of the changes in setup.h

I understand the setup.h was a customized version of the setup.h in wxWidgets 3.1.1rc. So at first I compared the difference of them, copy the setup.h from wxWidgets 3.2.1, and apply the difference. It fails to compile, so I did the minimal edit to make it compile.

Is this related to the other changes, i.e. was vs2017 support previously working?

I'm pretty sure it's the latest bakefile that changes

<WindowsTargetPlatformVersion Condition="$(VisualStudioVersion)&gt;=16">10.0</WindowsTargetPlatformVersion>

to

<WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion>

which make CI of my first commit fail, so I have to revert it. I don't want to modify generated file but I have to.

eagleoflqj commented 1 year ago

Generally the fewer are enabled, the better, as it results in smaller binaries.

I agree. Some default options that are imported due to upgrading setup.h may not be necessary, and I'll check them.

eagleoflqj commented 1 year ago

@vslavik Anything I should address here?

vslavik commented 1 year ago

@vslavik Anything I should address here?

Well, the extra noise in project files (yes, they need to be manually updated), but it's OK, I'll do it myself when merging.

vslavik commented 1 year ago

Thanks a lot; this is now merged with an extra project cleanup commit (to keep manual changes to bakefile-generated projects) and a small fix to remove the need for WXWIN_COMPATIBILITY_3_0 mentioned above.

lijy91 commented 1 year ago

@vslavik Could you please release a new version?

vslavik commented 1 year ago

@vslavik Could you please release a new version?

Need to address #227 and #177 and possibly #224 (all enabled by this PR) first.

vslavik commented 1 year ago

This is now in 0.8.0: https://github.com/vslavik/winsparkle/releases/tag/v0.8.0