Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[osg] Cleanup, export unofficial config #24720

Merged
merged 44 commits into from
Jun 20, 2022
Merged

[osg] Cleanup, export unofficial config #24720

merged 44 commits into from
Jun 20, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented May 15, 2022

  • What does your PR fix?

    Maintenance improvements.

    • Major change: osg and osgearth have custom find modules and an unusual approach to <PKG>_LIBRARY_DEBUG/<PKG>_LIBRARY when asked to link the variable name <PKG>_LIBRARY. This PR adds the preference to use <PKG>_LIBRARIES instead, which carries debug+release+dependencies in standard find modules (with help of vcpkg wrappers.) This change is the key to reduce patching. It is also no longer necessary to pass parameters about static dependencies, because they only activate uneducated guessing about transitive dependencies.
    • Revises features and dependencies for more control, leveraging supports.
    • Exports unofficial CMake config from osg, and uses it for osgearth. By using the targets, we don't need to patch the order of libraries, and we also get the transitive usage requirements.
    • Builds apps (tools) only for debug.
    • Collects plugins in /plugins/osgPlugins-<VERSIONS>.
  • Which triplets are supported/not supported? Have you updated the CI baseline?

    no changes

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

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 0cf5b4305ba811d069d3ce328261eeb54bf61c3a -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index e1b624d..b68d757 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -4938,7 +4938,7 @@
     },
     "nvtt": {
       "baseline": "2.1.2",
-      "port-version": 4
+      "port-version": 5
     },
     "oatpp": {
       "baseline": "1.3.0",
@@ -5250,7 +5250,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5258,7 +5258,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/n-/nvtt.json b/versions/n-/nvtt.json
index 9eb4a0e..f829496 100644
--- a/versions/n-/nvtt.json
+++ b/versions/n-/nvtt.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "e855f75c920309ed9fda986f7efe6b91d3b6d87d",
+      "version": "2.1.2",
+      "port-version": 5
+    },
     {
       "git-tree": "5889b2b68d1c3b13bf681c75e0b8347fff3d72fc",
       "version": "2.1.2",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..890605f 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "8d475b53f0dbb8a79bb73fa102078685e34395d6",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..e68f3e4 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "935b72ab6bc513a3e24e4a42075ebdc9688343bb",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label May 16, 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 5e7cbdceacd2b3cb85e057963fdb605136805bd3 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index e1b624d..b68d757 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -4938,7 +4938,7 @@
     },
     "nvtt": {
       "baseline": "2.1.2",
-      "port-version": 4
+      "port-version": 5
     },
     "oatpp": {
       "baseline": "1.3.0",
@@ -5250,7 +5250,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5258,7 +5258,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/n-/nvtt.json b/versions/n-/nvtt.json
index 9eb4a0e..f829496 100644
--- a/versions/n-/nvtt.json
+++ b/versions/n-/nvtt.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "e855f75c920309ed9fda986f7efe6b91d3b6d87d",
+      "version": "2.1.2",
+      "port-version": 5
+    },
     {
       "git-tree": "5889b2b68d1c3b13bf681c75e0b8347fff3d72fc",
       "version": "2.1.2",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..631fff2 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "5979b8df37afd67adb070a6674b91b0f50325f73",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..8565348 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "c70c412cd0c3883bbf3d72a02b16e6e2af56fb8d",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

@dg0yt
Copy link
Contributor Author

dg0yt commented May 24, 2022

To be continued after merge of #24888 which fixes installation order and linkage issues with port nvtt.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label May 24, 2022
@dg0yt

This comment was marked as outdated.

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 38bb87c5571555f1a4f64cb4ed9d2be0017f9fc1 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index df8f9b9..e4322ce 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5262,7 +5262,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5270,7 +5270,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..65a9859 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "4c80843aaa9d2229d1ce743b41b982200facdbac",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..c8c95bb 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "16370c4b9e4c0087123e107d34c1b7133454078c",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/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 38bb87c5571555f1a4f64cb4ed9d2be0017f9fc1 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index df8f9b9..e4322ce 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5262,7 +5262,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5270,7 +5270,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..65a9859 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "4c80843aaa9d2229d1ce743b41b982200facdbac",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..201621c 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "efc747ce157d5f42a19ea03005b0956f6f4a3cc5",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/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 38bb87c5571555f1a4f64cb4ed9d2be0017f9fc1 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index df8f9b9..e4322ce 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5262,7 +5262,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5270,7 +5270,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..62d6db0 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "eebbb2f327c0fda8c9c5cf700815866cb6bb0bbd",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..201621c 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "efc747ce157d5f42a19ea03005b0956f6f4a3cc5",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/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!

All manifest files must be formatted

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

Diff
diff --git a/ports/osg/vcpkg.json b/ports/osg/vcpkg.json
index 341db51..a58d5ab 100644
--- a/ports/osg/vcpkg.json
+++ b/ports/osg/vcpkg.json
@@ -171,25 +171,25 @@
           "name": "osg",
           "default-features": false,
           "features": [
-            "rest-http-device"
+            "nvtt"
           ],
-          "platform": "!windows"
+          "platform": "!x86"
         },
         {
           "name": "osg",
           "default-features": false,
           "features": [
-            "sdl1"
+            "rest-http-device"
           ],
-          "platform": "!osx & !uwp"
+          "platform": "!windows"
         },
         {
           "name": "osg",
           "default-features": false,
           "features": [
-            "nvtt"
+            "sdl1"
           ],
-          "platform": "!x86"
+          "platform": "!osx & !uwp"
         },
         "tiff"
       ]
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout faed44dfa013088fe1910908a8a65887622f412f -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index a938c96..d93be20 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5266,7 +5266,7 @@
     },
     "osg": {
       "baseline": "3.6.5",
-      "port-version": 12
+      "port-version": 13
     },
     "osg-qt": {
       "baseline": "Qt5",
@@ -5274,7 +5274,7 @@
     },
     "osgearth": {
       "baseline": "3.2",
-      "port-version": 5
+      "port-version": 6
     },
     "osi": {
       "baseline": "0.108.6",
diff --git a/versions/o-/osg.json b/versions/o-/osg.json
index a90a66a..955d36e 100644
--- a/versions/o-/osg.json
+++ b/versions/o-/osg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "59614ba705c1b7d945c405d1b95cda1566d55904",
+      "version": "3.6.5",
+      "port-version": 13
+    },
     {
       "git-tree": "f438ec006b58ffd222b0f3bdce8d05b21611204e",
       "version": "3.6.5",
diff --git a/versions/o-/osgearth.json b/versions/o-/osgearth.json
index ef76793..201621c 100644
--- a/versions/o-/osgearth.json
+++ b/versions/o-/osgearth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "efc747ce157d5f42a19ea03005b0956f6f4a3cc5",
+      "version": "3.2",
+      "port-version": 6
+    },
     {
       "git-tree": "80311fbaa014b84f8cdb3faa8abbdc16e01837d5",
       "version": "3.2",

You have modified or added at least one vcpkg.json where you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Did you test the usage?

ports/osgearth/portfile.cmake Outdated Show resolved Hide resolved
ports/osg/portfile.cmake Outdated Show resolved Hide resolved
ports/osg/portfile.cmake Outdated Show resolved Hide resolved
ports/osg/unofficial-osg-config.cmake Show resolved Hide resolved
ports/osgearth/portfile.cmake Outdated Show resolved Hide resolved
@dg0yt
Copy link
Contributor Author

dg0yt commented Jun 6, 2022

Did you test the usage?

Usage of osg? It happens in osgearth. (There is also osg-qt.) Even with heavy patching in osgearth, it was far from perfect before these changes. I started this PR because the pending GDAL update is blocked by osg. I hope it is all okay now, but at least it is better base for drive-by fixes.

Usage of osgearth? It exports neither CMake config nor pkg-config. Pretty unusable, but this is not a regressions.

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 you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 7, 2022
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Some questions before merging. Otherwise LGTM, thanks again for the awesome cleanups!

ports/osg/vcpkg.json Show resolved Hide resolved
ports/osg/portfile.cmake Show resolved Hide resolved
ports/osg/unofficial-osg-config.cmake Outdated Show resolved Hide resolved
@@ -31,9 +31,29 @@
}
],
"default-features": [
"default-features",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you comment why this should be a default feature?

We should also remove tools, because they do not meet vcpkg's default features policy.

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 didn't want to remove tools in a micro update. In addition, building executables is the ultimate test to check if transitive usage requirements are properly carried. So this feature is precious in vcpkg CI.
(The focus of my PR was osg. This is osgearth, which is waiting for a separate update.)

Copy link
Contributor

Choose a reason for hiding this comment

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

If the needed purpose is to build things in vcpkg ci, we should add a test port for it in scripts/test_ports/ and not saddle every user with building things they don't need.

Please remove "tools" from the default features list; it's very easy for a consumer to add it back in as a dependency that's forward and backwards compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

ports/osgearth/find-package.patch Show resolved Hide resolved
@LilyWangLL LilyWangLL removed the info:reviewed Pull Request changes follow basic guidelines label Jun 9, 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 you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

@dg0yt dg0yt requested a review from ras0219-msft June 12, 2022 12:06
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Jun 14, 2022
@@ -31,9 +31,29 @@
}
],
"default-features": [
"default-features",
Copy link
Contributor

Choose a reason for hiding this comment

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

If the needed purpose is to build things in vcpkg ci, we should add a test port for it in scripts/test_ports/ and not saddle every user with building things they don't need.

Please remove "tools" from the default features list; it's very easy for a consumer to add it back in as a dependency that's forward and backwards compatible.

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 you should check the license field.

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

  • ports/osg/vcpkg.json
  • ports/osgearth/vcpkg.json

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

@LilyWangLL LilyWangLL added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jun 20, 2022
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 20, 2022
@JackBoosY JackBoosY requested a review from ras0219-msft June 20, 2022 02:30
@dan-shaw dan-shaw merged commit 0d27810 into microsoft:master Jun 20, 2022
@dg0yt dg0yt deleted the osg branch June 21, 2022 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants