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

[vcpkg_fail_port_install] Deprecate function #21489

Conversation

JackBoosY
Copy link
Contributor

Since we have now used keyword supports to abort the unsupported triplet build when analyzing the port paragraph, deprecate this function.

Related vcpkg-tool changes: microsoft/vcpkg-tool#184.

@JackBoosY JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team. labels Nov 17, 2021
@autoantwort
Copy link
Contributor

But maybe instead of printing the message for every end user suggest the change when creating PRs like in #20142

@autoantwort
Copy link
Contributor

I have created #21502 as an alternative

@PhoebeHui PhoebeHui marked this pull request as draft November 25, 2021 06:59
@JackBoosY JackBoosY closed this Dec 17, 2021
@BillyONeal
Copy link
Member

If this is resurrected it should respond to Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL like vcpkg_common_functions.

@JackBoosY JackBoosY reopened this Jan 27, 2022
…jack/depreciate-vcpkg-fail-port-install
@JackBoosY
Copy link
Contributor Author

JackBoosY commented Jan 27, 2022

Depends on #21502.
We should remove all vcpkg_fail_port_install calls first.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jan 27, 2022
@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jan 27, 2022
@BillyONeal
Copy link
Member

Depends on #21502. We should remove all vcpkg_fail_port_install calls first.

PS C:\Dev\vcpkg> git fetch --all --prune
Fetching origin
Fetching BillyONeal
PS C:\Dev\vcpkg> git switch -d origin/master
HEAD is now at 07e508359 [brpc] Fix build failed when protobuf update to 3.19.3 (#22685)
PS C:\Dev\vcpkg> findstr /snipl /c:"vcpkg_fail_port_install" ports\*
PS C:\Dev\vcpkg>

That is done.

@PhoebeHui
Copy link
Contributor

See https://github.com/microsoft/vcpkg/search?q=vcpkg_fail_port_install&type=,

Could you remove it in other three files as well?

@JackBoosY JackBoosY marked this pull request as ready for review January 28, 2022 05:21
scripts/ports.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_fail_port_install.cmake Outdated Show resolved Hide resolved
#]===]

message("${Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL}" "vcpkg_fail_port_install has been removed and all values should be moved by adding `supports` field to manifest file or directly adding `${PORT}:${FAILED_TRIPLET}=fail` to _scripts/ci.baseline.txt_.\nPlease remove `vcpkg_fail_port_install(...)`.\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be

    z_vcpkg_deprecation_message(".... has been deprecated in favor of ....")

as the first directive inside the function, not at global scope.

Copy link
Member

Choose a reason for hiding this comment

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

That is not consistent with how it works for vcpkg_common_functions. I observe z_vcpkg_deprecation_message is not hooked up to Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL and thus not to --prohibit-backcompat-features which is undesirable here.

JackBoosY and others added 2 commits January 30, 2022 00:35
Co-authored-by: autoantwort <41973254+autoantwort@users.noreply.github.com>
@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jan 30, 2022
@@ -40,6 +42,8 @@ Library linkage for which the build should fail early.
#]===]

function(vcpkg_fail_port_install)
z_vcpkg_deprecation_message("vcpkg_fail_port_install has been removed and all values should be moved by adding `supports` field to manifest file or directly adding `${PORT}:${FAILED_TRIPLET}=fail` to _scripts/ci.baseline.txt_.\nPlease remove `vcpkg_fail_port_install(...)`.\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

I was incorrect about this needing to be z_vcpkg_deprecation_message -- it should be the originally proposed message("${Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL}" ...) form (though this is the right place to put it).

Copy link
Member

Choose a reason for hiding this comment

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

Done!

@BillyONeal BillyONeal merged commit dd7550b into microsoft:master Feb 3, 2022
@JackBoosY JackBoosY deleted the dev/jack/depreciate-vcpkg-fail-port-install branch February 5, 2022 07:35
ekilmer added a commit to ekilmer/vcpkg that referenced this pull request Feb 5, 2022
* master: (221 commits)
  [vcpkg-tool] update to 2022-02-03 (microsoft#22924)
  [opencv4] Disable building cpufeatures since it conflict with libwebp (microsoft#22844)
  [rhasheq] New port (microsoft#22905)
  [sfml] Add arm64 patch to allow SFML to compile on apple silicon (microsoft#22937)
  [popsift] Fix missing Thrust include, already merged upstream. (microsoft#22929)
  [python3][python2] Use MKDIR_P to create directories to avoid race conditions (microsoft#22902)
  Added libe57format port (microsoft#22909)
  update polyhook2 (microsoft#22906)
  [botan] Fix debug info (microsoft#22911)
  [opentelemetry-cpp] update version to v1.2.0 (microsoft#22925)
  [docs] document VCPKG_INSTALLED_DIR variable (microsoft#22695)
  [c89stringutils] New port (microsoft#22904)
  [randomstr] New port (microsoft#22921)
  [docs] Add Authoring-script-ports.md (microsoft#22396)
  [vcpkg_fail_port_install] Deprecate function (microsoft#21489)
  [vcpkg-cmake-config] add missing TOOLS_PATH (microsoft#22863)
  Ace Build Windows - Missing files in include package (microsoft#22880)
  [opendnp3] Disable FetchContent in favor of predownloading (microsoft#22894)
  libraqm update to 0.9.0 (microsoft#22907)
  [google-cloud-cpp] update to latest release (v1.36.0) (microsoft#22897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants