-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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] X_VCPKG_APPINSTALL_DEPS_INSTALL optionally install dependencies on install #14243
[vcpkg] X_VCPKG_APPINSTALL_DEPS_INSTALL optionally install dependencies on install #14243
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected for me with install command and CPack.
One small suggestion: you don't need to expand variables in if
expressions, this will be done automatically. Elsewhere in this file, the same approach is used, so it's best to do the same for consistency. :) I also tested with this changes, everything works fine for me.
Use variables directly instead of prefixing with `${}` as is done in rest of the file. Thanks @shatur95 Co-authored-by: Hennadii Chernyshchyk <[email protected]>
Co-authored-by: Hennadii Chernyshchyk <[email protected]>
cc @BillyONeal for review this PR. |
Given that this is an experimental feature, I'm up for merging it, with the understanding that this very well may be removed or modified, especially as we get closer to stabilization :) Thanks @sandercox for your contribution~! |
…pendencies on install (microsoft#14243)" This reverts commit 42456b7.
…stall dependencies on install (microsoft#14243)"" This reverts commit 062f105.
Continuing on the work from #14129 #13011 this adds an install command override again as proposed initially but behind an
if
check for the variableX_VCPKG_APPLOCAL_DEPS_INSTALL
marking it as experimental and allowing the feature to just work as I initially intended.I was already using the install function as used here in my own projects but I think this allows everybody to really use the functionality introduced by the
x_vcpkg_install_local_dependencies
function.As originally requested in #1653