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_from_git] new options TAG and X_OUT_REF #15049

Merged
merged 5 commits into from Jan 11, 2021

Conversation

Neumann-A
Copy link
Contributor

  • TAG github tag to checkout
  • OUT_REF github commit id related to tag or ref
    (useable for automatic updates of ports if used with a version tag)

I use this in my two Qt PRs to directly checkout from code.qt.io and enable automatic updates of the refs on tag change

- TAG github tag to checkout
- OUT_REF github commit id related to tag or ref
(useable for automatic updates of ports if used with a version tag)
@ras0219-msft
Copy link
Contributor

We explicitly ask for REFs and not tags in the shared helpers in order to defend against tag republishing. If this functionality is just for qt6 updates, it seems more appropriate to keep private to the qt ports.

@Neumann-A
Copy link
Contributor Author

@ras0219-msft: I hope qt is stable enough to not do that. This PR does not change the behavior it still requires REF. It just checks out by TAG if available and compare the result against the given REF. So tag republishing will still be detected. If a port is doing tag republishing regularly the port simply needs to be updated to remove the TAG option without any other changes.

@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly requires:discussion labels Dec 11, 2020
@ras0219
Copy link
Contributor

ras0219 commented Dec 13, 2020

Thanks for the explanation; I didn't read the code closely enough. I can potentially see use in the TAG field as an additional check on top of the REF field.

Could you link to exactly how you've used OUT_REF?

@Neumann-A
Copy link
Contributor Author

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.

Thanks for the explanation and pointer. LGTM given the changes I've suggested.

scripts/cmake/vcpkg_from_git.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_from_git.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_from_git.cmake Outdated Show resolved Hide resolved
This was referenced Jan 7, 2021
@Neumann-A Neumann-A changed the title [vcpkg_from_git] new options TAG and OUT_REF [vcpkg_from_git] new options TAG and X_OUT_REF Jan 8, 2021
@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY
Copy link
Contributor

Should be good now.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jan 11, 2021
@@ -41,7 +48,7 @@ Relative paths are based on the port directory.
include(vcpkg_execute_in_download_mode)

function(vcpkg_from_git)
set(oneValueArgs OUT_SOURCE_PATH URL REF)
set(oneValueArgs OUT_SOURCE_PATH URL REF TAG X_OUT_REF)
Copy link
Member

Choose a reason for hiding this comment

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

@BillyONeal please remind me.
As this is internal and documented should it be prefixed with X_ or Z_?

@vicroms vicroms merged commit 0b16dbc into microsoft:master Jan 11, 2021
@Neumann-A Neumann-A deleted the from_git_support_tags branch January 11, 2021 08:29
@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Jan 11, 2021
@ras0219-msft ras0219-msft mentioned this pull request Jun 9, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants