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] Set default triplet depends on architecture (#1254) #11610

Closed
wants to merge 1 commit into from

Conversation

psionic12
Copy link

  1. Set the environment variable VCPKG_DEFAULT_TRIPLET if it is not set.
  2. Looking up this environment in CMake to make sure which _VCPKG_TARGET_TRIPLET_ARCH to use.

Comment on lines +370 to +373
# Set Environment Variable "VCPKG_DEFAULT_TRIPLET" depends on architecture
if(![Environment]::GetEnvironmentVariable("VCPKG_DEFAULT_TRIPLET", "User") -and ![Environment]::GetEnvironmentVariable("VCPKG_DEFAULT_TRIPLET", "Machine")) {
[Environment]::SetEnvironmentVariable("VCPKG_DEFAULT_TRIPLET", $PreferredToolArchitecture+"-windows", 'User')
}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this only set VCPKG_DEFAULT_TRIPLET for the PowerShell instance that runs the bootstrap script?

Copy link
Author

@psionic12 psionic12 Jul 3, 2020

Choose a reason for hiding this comment

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

No, it sets the environment variable for the user, if replace the "User" to "Process", then it's behavior is what you said. Indeed, it's better to set "Process" as well since the PowerShell instance you run the script will not notice the environment variables are changed.

@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Jul 3, 2020
@ras0219-msft
Copy link
Contributor

ras0219-msft commented Jul 9, 2020

Thanks for the PR!

We don't want to modify the user's outer environment at all in bootstrap; especially not only on one platform.

AFAICT the changes to vcpkg.cmake would cause incorrect behavior; if you have an issue which these changes are required to address, please either comment here or open an issue!

Unless there is something I've missed, I'm going to close this PR for now. Please let us know if there is anything remaining to address!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants