Skip to content

[wxwidgets] Validate and fix#24047

Merged
strega-nil-ms merged 27 commits intomicrosoft:masterfrom
dg0yt:wxwidgets
Apr 18, 2022
Merged

[wxwidgets] Validate and fix#24047
strega-nil-ms merged 27 commits intomicrosoft:masterfrom
dg0yt:wxwidgets

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Apr 8, 2022

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/wxwidgets/vcpkg.json b/ports/wxwidgets/vcpkg.json
index 435348b..089e02e 100644
--- a/ports/wxwidgets/vcpkg.json
+++ b/ports/wxwidgets/vcpkg.json
@@ -25,7 +25,7 @@
     "zlib"
   ],
   "features": {
-    "example" : {
+    "example": {
       "description": "Example source code and CMake project"
     }
   }
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d72783cb3aeddfd667861caef1060e54ca6fa7a9 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 4dc8f8d..f7f406b 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7498,7 +7498,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..22f10f4 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "a8eb52c74cddc858d6a59187bedb666456e83cf9",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

@talregev
Copy link
Contributor

talregev commented Apr 8, 2022

Follow this fix. Thank you.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..8cca2d4 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "b3f82f8e52d357a2fd6346a06fa49765a0784bb6",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..ea5554f 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "9b75b56e40db43bb78a31348bc14580fb4f19784",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..f2f1495 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "b8aa5c1b09f21f3336f6c10dfc7ebef626dcd4f8",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..c78d7a6 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "97db7c1b0d34158952ecdc6db6ce6a0aae282256",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..2519621 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "faa68115d2c002efc172b9082a75ba93b80ce00a",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout bd1ef2df46303989eeb048eb7aa9b816aa46365e -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index c7016f8..6b904ab 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 9
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..797d132 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "ed6660583c3bd29abfd0421bf6f9d9a42069f1f4",
+      "version-semver": "3.1.5",
+      "port-version": 9
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 10, 2022

In the current state of the PR (9adb008), the port behaves much better:

There are still some shortcomings:

All open issues for wxwidgets:
https://github.com/microsoft/vcpkg/issues?q=is%3Aissue+is%3Aopen+wxwidgets+in%3Atitle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might make it an error if there is something else than NOT, CONFIG:DEBUG, LINK_ONLY, to catch future changes.

@talregev
Copy link
Contributor

Should I test it too? Is ready for test?

@JackBoosY JackBoosY self-assigned this Apr 11, 2022
@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Apr 11, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout f6af75acc923c833a5620943e3fc7d5e4930f0df -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 630a0f7..0ca1fdc 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7502,7 +7502,7 @@
     },
     "wxwidgets": {
       "baseline": "3.1.5",
-      "port-version": 8
+      "port-version": 10
     },
     "x-plane": {
       "baseline": "3.0.3",
diff --git a/versions/w-/wxwidgets.json b/versions/w-/wxwidgets.json
index ec84429..8a0b7ce 100644
--- a/versions/w-/wxwidgets.json
+++ b/versions/w-/wxwidgets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "78544fc3c99af9bc945de55497e778692a8ccaf5",
+      "version-semver": "3.1.5",
+      "port-version": 10
+    },
     {
       "git-tree": "dba058c37782edf771e7a62ae1bef98274c86b9f",
       "version-semver": "3.1.5",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

@Thomas1664
Copy link
Contributor

One question about examples:
WxWidgets installs examples by default although you marked it a feature:

wxwidgets:x64-windows:/share/wxwidgets/example/CMakeLists.txt
wxwidgets:x64-windows:/share/wxwidgets/example/popup.cpp
wxwidgets:x64-windows:/share/wxwidgets/example/sample.xpm

Is this intended?

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 15, 2022

WxWidgets installs examples by default

Not by default but in CI. wxwidgets[examples] is a dependency of the vcpkg-ci-wxwidgets test port.
(This whole work was test-first, and it is likely to break again if I reduce test coverage.)

Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from that

@@ -0,0 +1,21 @@
cmake_minimum_required(VERSION 3.7)
Copy link
Contributor

@strega-nil-ms strega-nil-ms Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this being named top level CMakeLists.txt; I'd prefer it to be in an example folder.

Also, I need to test this with the windows.cmake CMAKE_CROSSCOMPILING change, since I noticed find_package(wxWidgets) is broken by CMAKE_CROSSCOMPILING

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this being named top level CMakeLists.txt; I'd prefer it to be in an example folder.

Okay.

Also, I need to test this with the windows.cmake CMAKE_CROSSCOMPILING change, since I noticed find_package(wxWidgets) is broken by CMAKE_CROSSCOMPILING

Which change do you refer to? Which breakage?

AFAICS the CMake find module doesn't support win32 mode with CMAKE_CROSSCOMPILING:
https://github.com/Kitware/CMake/blob/6453bd046ef23798c64333042d76851f15eff2fc/Modules/FindwxWidgets.cmake#L221-L225

(Mingw was added only 5 days ago. For MSVC, apart from fixing upstream, we could try to hack it the opposite way I did for mingw.)

Copy link
Contributor

@strega-nil-ms strega-nil-ms Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmh, it looks like this is a vcpkg thing; CMAKE_CROSSCOMPILING is never set on Windows targeting Windows. (it's not set right now either, but we're going to be setting it in windows.cmake in the future)

Comment on lines +10 to +14
if(MINGW AND NOT CMAKE_CROSSCOMPILING)
# Force FindwxWidgets.cmake unix mode, matching mingw install layout
set(_vcpkg_wxwidgets_fake_crosscompiling 1)
set(CMAKE_CROSSCOMPILING 1)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(MINGW AND NOT CMAKE_CROSSCOMPILING)
# Force FindwxWidgets.cmake unix mode, matching mingw install layout
set(_vcpkg_wxwidgets_fake_crosscompiling 1)
set(CMAKE_CROSSCOMPILING 1)
endif()
if(MINGW AND NOT CMAKE_CROSSCOMPILING)
# Force FindwxWidgets.cmake unix mode, matching mingw install layout
set(_vcpkg_wxwidgets_backup_crosscompiling "${CMAKE_CROSSCOMPILING}")
set(CMAKE_CROSSCOMPILING 1)
endif()
if(WIN32 AND CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
set(_vcpkg_wxwidgets_backup_crosscompiling "${CMAKE_CROSSCOMPILING}")
set(CMAKE_CROSSCOMPILING 0)
endif()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strega-nil-ms I applied this with modifications.

Comment on lines +45 to +50
if(MINGW)
if(_vcpkg_wxwidgets_fake_crosscompiling)
unset(CMAKE_CROSSCOMPILING)
unset(_vcpkg_wxwidgets_fake_crosscompiling)
endif()
elseif(WIN32 AND "@VCPKG_LIBRARY_LINKAGE@" STREQUAL "static")
Copy link
Contributor

@strega-nil-ms strega-nil-ms Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(MINGW)
if(_vcpkg_wxwidgets_fake_crosscompiling)
unset(CMAKE_CROSSCOMPILING)
unset(_vcpkg_wxwidgets_fake_crosscompiling)
endif()
elseif(WIN32 AND "@VCPKG_LIBRARY_LINKAGE@" STREQUAL "static")
if(DEFINED _vcpkg_wxwidgets_backup_crosscompiling)
set(CMAKE_CROSSCOMPILING "${_vcpkg_wxwidgets_backup_crosscompiling}")
unset(_vcpkg_wxwidgets_backup_crosscompiling)
endif()
if(WIN32 AND NOT MINGW AND "@VCPKG_LIBRARY_LINKAGE@" STREQUAL "static")

@strega-nil-ms strega-nil-ms added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Apr 15, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

@dg0yt dg0yt marked this pull request as draft April 17, 2022 11:52
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • scripts/test_ports/cmake-user/vcpkg.json
  • scripts/test_ports/vcpkg-ci-wxwidgets/vcpkg.json

Valid values for the license field can be found in the documentation

@dg0yt dg0yt marked this pull request as ready for review April 17, 2022 19:34
@JackBoosY JackBoosY requested a review from strega-nil-ms April 18, 2022 09:05
Copy link
Contributor

@talregev talregev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on the ci and wsl2.

@strega-nil-ms
Copy link
Contributor

Thanks!

@strega-nil-ms strega-nil-ms merged commit 48d15f4 into microsoft:master Apr 18, 2022
@autoantwort
Copy link
Contributor

autoantwort commented Apr 19, 2022

The lib now contains absolute paths:
vcpkg/packages/wxwidgets_arm64-osx/tools/wxwidgets/wx-config:

ldlibs_base="/Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libz.a -lwxregexu-3.1 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk/usr/lib/libiconv.tbd -framework CoreFoundation -framework Security -framework Carbon -framework Cocoa -framework IOKit -framework QuartzCore"
ldlibs_core="-lwx_baseu-3.1 /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libjpeg.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libpng.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libz.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libtiff.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/liblzma.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libjpeg.a /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libz.a -lm  -framework AudioToolbox -framework WebKit "
ldlibs_xml="-lwx_baseu-3.1 /Users/leanderSchulten/git_projekte/Lichtsteuerung/vcpkg_installed/arm64-osx/lib/libexpat.a"

@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 19, 2022

I can take a look tonight. wx-config is used by FindwxWidgets.cmake so it must be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

8 participants