-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[Windows] Add VCPKG_ROOT variable #6192
[Windows] Add VCPKG_ROOT variable #6192
Conversation
This change broke several of our CI builds on GitHub Actions and Azure Pipelines (we use the presence of VCPKG_ROOT being set to indicate that a user wants to opt-in to building dependencies with vcpkg rather than ones installed on the system). Some form of announcement about this change would have been nice.
To get things working again our solution has been to liberally add |
@nightlark thank you for the feedback and workaround! we will investigate the issue. |
@michaelbprice could you please comment on this? It seems it's not so safe to add that variable |
I've left a more detailed comment over at #6196 (comment). We'll see how this shakes out, but it seems to me that if tools expect to use My recommendation would be to change that opt-in behavior to depend on an environment variable more clearly within the control of the users' environment, by doing something like prepending an org identifier to the environment variable, such as |
In the sense that it's never safe to add any environment variable, as you cannot predict if someone downstream might already be using it for some other purpose. |
Recently GHA started defining VCPKG_ROOT environment variable (actions/runner-images#6192). So unset it to not let it autodetected and interfere with our build jobs.
Recently GHA started defining VCPKG_ROOT environment variable (actions/runner-images#6192). So unset it to not let it autodetected and interfere with our build jobs.
Description
Add VCPKG_ROOT variable in additional to VCPKG_INSTALLATION_ROOT + simple style fix
Related issue:
#4241
Check list