[vcpkg.cmake] guard against inf recursion. #23195

Neumann-A · 2022-02-20T22:43:47Z

requires cmake 3.18
closes #16454

github-actions

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 5cf60186a241e84e8232641ee973395d4fde90e1 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 6b3147e..165fa5e 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7542,7 +7542,7 @@
     },
     "zlib": {
       "baseline": "1.2.11",
-      "port-version": 13
+      "port-version": 14
     },
     "zlib-ng": {
       "baseline": "2.0.5",
diff --git a/versions/z-/zlib.json b/versions/z-/zlib.json
index 7c0d68f..cc5ac06 100644
--- a/versions/z-/zlib.json
+++ b/versions/z-/zlib.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "6e9612a1beb3167f554ef45373ab272b56596f90",
+      "version": "1.2.11",
+      "port-version": 14
+    },
     {
       "git-tree": "92cfe30c807d343c6359d272242f0765ad906740",
       "version": "1.2.11",

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

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

ports/zlib/vcpkg.json
scripts/test_ports/inf-recursion-test/vcpkg.json

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

scripts/buildsystems/vcpkg.cmake

ports/zlib/vcpkg.json

Co-authored-by: autoantwort <[email protected]>

github-actions

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 5cf60186a241e84e8232641ee973395d4fde90e1 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 6b3147e..165fa5e 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7542,7 +7542,7 @@
     },
     "zlib": {
       "baseline": "1.2.11",
-      "port-version": 13
+      "port-version": 14
     },
     "zlib-ng": {
       "baseline": "2.0.5",
diff --git a/versions/z-/zlib.json b/versions/z-/zlib.json
index 7c0d68f..f0f409f 100644
--- a/versions/z-/zlib.json
+++ b/versions/z-/zlib.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "27564e1e248f9c40b856871f6b112eb4c4971dc1",
+      "version": "1.2.11",
+      "port-version": 14
+    },
     {
       "git-tree": "92cfe30c807d343c6359d272242f0765ad906740",
       "version": "1.2.11",

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

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

scripts/test_ports/inf-recursion-test/vcpkg.json

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

Neumann-A · 2022-02-21T08:46:46Z

@JackBoosY: Why the tag depends:different-pr?

JackBoosY · 2022-02-21T08:51:19Z

Ah sorry for that. Now we use cmake 3.22.

github-actions

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

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

scripts/test_ports/inf-recursion-test/vcpkg.json

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

github-actions

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

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

scripts/test_ports/inf-recursion-test/vcpkg.json

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

Neumann-A · 2022-02-21T21:12:26Z

Hmm the license bot is buggy for test ports

JackBoosY · 2022-02-22T01:56:00Z

Ready for review now?

Neumann-A · 2022-02-22T07:51:42Z

@JackBoosY Yes

JackBoosY · 2022-02-22T08:24:42Z

I'd like this PR and we should wait for other team members to review it.

scripts/buildsystems/vcpkg.cmake

strega-nil-ms

I need to review further, but that's the obvious change I see right now.

github-actions

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

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

scripts/test_ports/inf-recursion-test/vcpkg.json

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

strega-nil-ms

Those are the only two things, then I'm approve (well, also have a vcpkg_underlying_add_executable and such)

ports/zlib/vcpkg-cmake-wrapper.cmake

scripts/buildsystems/vcpkg.cmake

Co-authored-by: nicole mazzuca <[email protected]>

github-actions

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:

scripts/test_ports/inf-recursion-test/vcpkg.json

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

github-actions

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:

scripts/test_ports/inf-recursion-test/vcpkg.json

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

JackBoosY · 2022-07-11T03:02:41Z

So what's the status here?

Neumann-A · 2022-07-11T05:30:51Z

@JackBoosY Should be finished. The zlib issue seems to be gone since CI is green (?, never had a clue why it was happening any way and wasn't locally reproduciable)

JackBoosY · 2022-07-11T07:27:09Z

@BillyONeal Can you please review again?

Thanks.

Neumann-A · 2022-10-25T08:49:42Z

I see the merge conflict; I ignore the merge conflict unless somebody is showing a willingness to merge. or should I invent a new 3.24 toolchain ?

Cheney-W · 2022-11-11T10:15:11Z

We should not do this: defending against the "before project()" case when all in-the-wild examples have been after project() seems like unnecessary complexity.

Neumann-A · 2022-11-11T11:05:52Z

all in-the-wild examples have been after project()

Do you have a list of examples? The point here is also that for those examples we could patch them to be before project() (or also do the correct thing which is applied here.)

BillyONeal · 2023-01-17T19:00:56Z

all in-the-wild examples have been after project()

Do you have a list of examples? The point here is also that for those examples we could patch them to be before project() (or also do the correct thing which is applied here.)

The ones listed in the rationale for the change (e.g. #16454) are that way.

BillyONeal · 2024-06-22T00:29:48Z

Given that this change is complex and we haven't seen an in the wild example it will fix, I'm going to close this. Please feel to reopen it should an in the wild bug it would fix appears.

Thanks for your contribution!

Neumann-A · 2024-06-22T21:04:11Z

@BillyONeal You still want to have z_vcpkg_underlying_find_package to someday support dependency providers. You want wrappers to stop using _find_package directly

Neumann-A · 2024-07-02T20:17:37Z

@BillyONeal: Just saying: The overrides in vcpkg.cmake makes using vcpkg.cmake with my current employer in the current form impossible. I am more than capable enough to workaround that but other employees in other companies might not be able to do so.

It should be a priority to get rid of the plain _find_package calls in the wrappers.

BillyONeal · 2024-07-03T22:36:54Z

@JavierMatosD @ras0219-msft @vicroms @AugP and I discussed. We make the following observations:

The expected use case is:
1. Define VCPKG_OVERRIDE_FIND_PACKAGE_NAME to something else
2. You call macro(find_package ...)
3. In your overridden macro, you call "something else"
  but we understand that this enforces that vcpkg is in the 'bottom' of the stack without excessive workarounds.
We would be OK with adding a new feature for "the name of the thing we call that is not _find_package" but:
- It needs infrastructure for the wrappers to call to avoid needing cmake_language(...)
- We need a post build check for vcpkg-cmake-wrapper to fail if _find_package is present

Neumann-A · 2024-07-06T14:47:03Z

Define VCPKG_OVERRIDE_FIND_PACKAGE_NAME to something else

That variable is currently broken and correctly supporting it requires cmake_language call.

We need a post build check for vcpkg-cmake-wrapper to fail if _find_package is present

Seems like a hen/egg problem. First requires the proper infrastructure in vcpkg.cmake and then can add the post build check into vcpkg-tool otherwise there is no way to fix the CI issues appearing due to the post build check.

[vcpkg.cmake] guard against inf recursion.

3f3413f

github-actions bot requested changes Feb 20, 2022

View reviewed changes

Neumann-A commented Feb 20, 2022

View reviewed changes

scripts/buildsystems/vcpkg.cmake Show resolved Hide resolved

autoantwort reviewed Feb 21, 2022

View reviewed changes

ports/zlib/vcpkg.json Outdated Show resolved Hide resolved

JonLiu1993 assigned JackBoosY Feb 21, 2022

JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed depends:different-pr This PR or Issue depends on a PR which has been filed labels Feb 21, 2022

Add license

46b670f

Co-authored-by: autoantwort <[email protected]>

github-actions bot requested changes Feb 21, 2022

View reviewed changes

JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 21, 2022

version update

8c29e00

github-actions bot approved these changes Feb 21, 2022

View reviewed changes

add license

9a96d8d

github-actions bot approved these changes Feb 21, 2022

View reviewed changes

Neumann-A marked this pull request as ready for review February 22, 2022 05:57

JackBoosY added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Feb 22, 2022

strega-nil-ms reviewed Mar 28, 2022

View reviewed changes

scripts/buildsystems/vcpkg.cmake Outdated Show resolved Hide resolved

strega-nil-ms suggested changes Mar 28, 2022

View reviewed changes

JackBoosY added the requires:author-response label Mar 29, 2022

Neumann-A added 2 commits March 30, 2022 21:16

Merge remote-tracking branch 'upstream/master' into avoid_inf_recursion

59dd304

apply cr

8592b56

github-actions bot approved these changes Mar 30, 2022

View reviewed changes

strega-nil-ms suggested changes Mar 31, 2022

View reviewed changes

ports/zlib/vcpkg-cmake-wrapper.cmake Outdated Show resolved Hide resolved

scripts/buildsystems/vcpkg.cmake Outdated Show resolved Hide resolved

ON/OFF instead of TRUE/FALSE

3a8cc5d

Co-authored-by: nicole mazzuca <[email protected]>

github-actions bot approved these changes May 12, 2022

View reviewed changes

ras0219-msft dismissed stale reviews from JackBoosY and strega-nil-ms via 3ed937b June 23, 2022 19:58

Neumann-A mentioned this pull request Jul 8, 2022

Using VCPKG_OVERRIDE_FIND_PACKAGE_NAME results in calls to unknown command _find_package #25649

Open

Merge branch 'microsoft:master' into avoid_inf_recursion

8afd64b

github-actions bot approved these changes Jul 8, 2022

View reviewed changes

JackBoosY linked an issue Jul 9, 2022 that may be closed by this pull request

Using VCPKG_OVERRIDE_FIND_PACKAGE_NAME results in calls to unknown command _find_package #25649

Open

JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 19, 2022

JavierMatosD added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 19, 2022

JackBoosY assigned Cheney-W and unassigned JackBoosY Oct 14, 2022

Cheney-W removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Nov 11, 2022

Cheney-W added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly and removed category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels May 17, 2023

Osyotr mentioned this pull request Sep 2, 2023

Webkit cmake config: Maximum recursion depth of 1000 exceeded #33507

Open

BillyONeal closed this Jun 22, 2024

Neumann-A mentioned this pull request Jul 6, 2024

Dont directly use _find_package #30972

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg.cmake] guard against inf recursion. #23195

[vcpkg.cmake] guard against inf recursion. #23195

Neumann-A commented Feb 20, 2022 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

Neumann-A commented Feb 21, 2022

JackBoosY commented Feb 21, 2022

github-actions bot left a comment

github-actions bot left a comment

Neumann-A commented Feb 21, 2022

JackBoosY commented Feb 22, 2022

Neumann-A commented Feb 22, 2022

JackBoosY commented Feb 22, 2022

strega-nil-ms left a comment

github-actions bot left a comment

strega-nil-ms left a comment

github-actions bot left a comment

github-actions bot left a comment

JackBoosY commented Jul 11, 2022

Neumann-A commented Jul 11, 2022

JackBoosY commented Jul 11, 2022

Neumann-A commented Oct 25, 2022

Cheney-W commented Nov 11, 2022

Neumann-A commented Nov 11, 2022

BillyONeal commented Jan 17, 2023 •

edited

Loading

BillyONeal commented Jun 22, 2024

Neumann-A commented Jun 22, 2024

Neumann-A commented Jul 2, 2024

BillyONeal commented Jul 3, 2024

Neumann-A commented Jul 6, 2024

[vcpkg.cmake] guard against inf recursion. #23195

[vcpkg.cmake] guard against inf recursion. #23195

Conversation

Neumann-A commented Feb 20, 2022 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

Neumann-A commented Feb 21, 2022

JackBoosY commented Feb 21, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

Neumann-A commented Feb 21, 2022

JackBoosY commented Feb 22, 2022

Neumann-A commented Feb 22, 2022

JackBoosY commented Feb 22, 2022

strega-nil-ms left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

strega-nil-ms left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

JackBoosY commented Jul 11, 2022

Neumann-A commented Jul 11, 2022

JackBoosY commented Jul 11, 2022

Neumann-A commented Oct 25, 2022

Cheney-W commented Nov 11, 2022

Neumann-A commented Nov 11, 2022

BillyONeal commented Jan 17, 2023 • edited Loading

BillyONeal commented Jun 22, 2024

Neumann-A commented Jun 22, 2024

Neumann-A commented Jul 2, 2024

BillyONeal commented Jul 3, 2024

Neumann-A commented Jul 6, 2024

Neumann-A commented Feb 20, 2022 •

edited

Loading

BillyONeal commented Jan 17, 2023 •

edited

Loading