[vcpkg] Allow CI to pass in all relevant directories and remove use of symbolic links #11483

BillyONeal · 2020-05-21T02:52:56Z

Adds --x-foo-root= parameters for the directories to which we have applied symlinks in the past, such as installed, buildtrees, packages, etc.

This is intended to unblock #11365 which broke due the symlink workaround we previously used to get the work tree onto the VM temp disk not working with the current Scale Set Agents installation script.

Drive-by fixes:

Fixed help text having differing output and alignment caused by duplicating these declarations in the tool
Always use HelpTableFormatter to emit help tables.
Fixed parameter validation, filesystem validation, and path conversion being spewed out in between VcpkgCmdArguments, VcpkgPaths, and main(). Now VcpkgCmdArguments owns command line argument parsing and reading defaults from the environment, VcpkgPaths owns only path resolution bits. Theoretically this reduces the fidelity of the error messages we emit here, but I don't think in practice customized error messages for each type of missing input or output directory add much user value. We emit messages containing the bad path whenever something explodes anyway.

scripts/buildsystems/vcpkg.cmake

MVoz · 2020-05-22T17:53:29Z

merge?

BillyONeal · 2020-05-25T04:59:25Z

Depends on #11542 and #11545

Neumann-A · 2020-05-25T19:45:41Z

scripts/cmake/vcpkg_build_msbuild.cmake

@@ -109,7 +109,7 @@ function(vcpkg_build_msbuild)
        list(
            APPEND _csc_OPTIONS
            /p:ForceImportBeforeCppTargets=${SCRIPTS}/buildsystems/msbuild/vcpkg.targets
-            "/p:VcpkgTriplet=${TARGET_TRIPLET}"
+            "/p:VcpkgRoot=${CURRENT_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}"


That is wrong.
You probably want

"/p:VcpkgTriplet=${TARGET_TRIPLET}" "/p:VcpkgRoot=${CURRENT_INSTALLED_DIR}"

~~nevermind just checked the targets VcpkgRoot is named wrongly~~
still wrong

VcpkgRoot should be renamed to VcpkgInstalledTripletRoot instead.... (naming is hard) (or VcpkgTripletInstalledDir)

Is ${CURRENT_INSTALLED_DIR} installed/triplet or just installed?

azure-pipelines · 2020-05-29T11:39:27Z

Azure Pipelines successfully started running 1 pipeline(s).

strega-nil · 2020-05-29T17:08:22Z

scripts/cmake/vcpkg_build_msbuild.cmake

@@ -110,6 +110,7 @@ function(vcpkg_build_msbuild)
            APPEND _csc_OPTIONS
            /p:ForceImportBeforeCppTargets=${SCRIPTS}/buildsystems/msbuild/vcpkg.targets
            "/p:VcpkgTriplet=${TARGET_TRIPLET}"
+            "/p:VcpkgRoot=${CURRENT_INSTALLED_DIR}"


is VcpkgRoot the equivalent of CURRENT_INSTALLED_DIR? I would expect this variable to be something like where the vcpkg executable lives.

Yes, it is like CURRENT_INSTALLED_DIR -- the name is bad because I didn't write the script consuming the variable (it's $/scripts/buildsystems/msbuild/vcpkg.targets). I don't want to edit that here because I don't know exactly how it interacts with vcpkg integrate install. Would filing an issue about it and addressing in a subsequent PR be OK? (I don't want to grow this PR even bigger :) )

Hmmm I guess I could make a dependent change which changes that name, stand by...

so should we merge 11653 first?

scripts/cmake/vcpkg_install_msbuild.cmake

# Conflicts: # toolsrc/include/vcpkg/vcpkgcmdarguments.h # toolsrc/src/vcpkg.cpp # toolsrc/src/vcpkgcmdarguments.cpp

toolsrc/src/vcpkg/vcpkgcmdarguments.cpp

JackBoosY

Please update qt5-base, shiva-sfml, spirv-tools version info. See documentation.

BillyONeal · 2020-06-03T00:33:50Z

Please update qt5-base, shiva-sfml, spirv-tools version info. See documentation.

Done, thanks!

…edDir. I could have ensured we always passed the trailing slash, but defending against it here makes it easier for users if they want to use /p themselves.

Maximus5 · 2020-06-04T10:44:58Z

scripts/buildsystems/msbuild/vcpkg.targets

+    allow us to override a console provided property here. -->
+    <VcpkgRootSanitized>$(VcpkgRoot)</VcpkgRootSanitized>
+    <VcpkgRootSanitized Condition="!$(VcpkgRootSanitized.EndsWith('\'))">$(VcpkgRootSanitized)\</VcpkgRootSanitized>
+    <VcpkgCurrentInstalledDirSanitized>$(VcpkgCurrentInstalledDir)</VcpkgCurrentInstalledDirSanitized>


Looks like there is a problem here.
The VcpkgCurrentInstalledDir initialization was removed few lines above and it's empty now.

…f symbolic links (microsoft#11483)

BillyONeal marked this pull request as draft May 21, 2020 02:53

BillyONeal force-pushed the directories branch 2 times, most recently from 687bcd4 to 206c8cf Compare May 21, 2020 03:12

This was referenced May 21, 2020

[msys] use fresh msys, fix update core #11471

Closed

[vcpkg-acquire-msys] Update pacman before any other package. #11443

Merged

BillyONeal force-pushed the directories branch 6 times, most recently from b996a00 to 544a0e5 Compare May 22, 2020 08:13

BillyONeal marked this pull request as ready for review May 22, 2020 08:13

BillyONeal requested review from ras0219-msft, strega-nil, vicroms and JackBoosY May 22, 2020 08:13

Neumann-A reviewed May 22, 2020

View reviewed changes

scripts/buildsystems/vcpkg.cmake Show resolved Hide resolved

BillyONeal added depends:different-pr This PR or Issue depends on a PR which has been filed info:internal This PR or Issue was filed by the vcpkg team. labels May 25, 2020

BillyONeal mentioned this pull request May 25, 2020

[vcpkg metrics] Allow someone to opt out after build #11542

Merged

Neumann-A reviewed May 25, 2020

View reviewed changes

strega-nil reviewed May 29, 2020

View reviewed changes

scripts/cmake/vcpkg_install_msbuild.cmake Outdated Show resolved Hide resolved

BillyONeal added depends:different-pr This PR or Issue depends on a PR which has been filed and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels May 29, 2020

BillyONeal added 3 commits May 29, 2020 16:49

Merge remote-tracking branch 'origin/master' into directories

5004548

# Conflicts: # toolsrc/include/vcpkg/vcpkgcmdarguments.h # toolsrc/src/vcpkg.cpp # toolsrc/src/vcpkgcmdarguments.cpp

Fix parse_switch calls.

fd6d850

Fix MSBuild variable name.

a1a89f2

BillyONeal requested a review from strega-nil May 29, 2020 23:50

strega-nil reviewed May 30, 2020

View reviewed changes

toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved

Be less SHOUTY_SNAKES

4ee716e

strega-nil approved these changes May 30, 2020

View reviewed changes

JackBoosY suggested changes Jun 1, 2020

View reviewed changes

BillyONeal added 2 commits June 2, 2020 17:31

Merge remote-tracking branch 'origin/master' into directories

e700138

Bump version number on edited packages.

04685e8

BillyONeal requested a review from JackBoosY June 3, 2020 00:33

JackBoosY approved these changes Jun 3, 2020

View reviewed changes

Defend against no trailing slash of VcpkgRoot and VcpkgCurrentInstall…

1df9751

…edDir. I could have ensured we always passed the trailing slash, but defending against it here makes it easier for users if they want to use /p themselves.

BillyONeal force-pushed the directories branch from 8cb8d9e to 1df9751 Compare June 3, 2020 06:36

BillyONeal merged commit 4fb2256 into microsoft:master Jun 4, 2020

BillyONeal deleted the directories branch June 4, 2020 02:31

Maximus5 reviewed Jun 4, 2020

View reviewed changes

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021

[vcpkg] Allow CI to pass in all relevant directories and remove use o…

ec09fa2

…f symbolic links (microsoft#11483)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg] Allow CI to pass in all relevant directories and remove use of symbolic links #11483

[vcpkg] Allow CI to pass in all relevant directories and remove use of symbolic links #11483

BillyONeal commented May 21, 2020

MVoz commented May 22, 2020

BillyONeal commented May 25, 2020 •

edited

Loading

Neumann-A May 25, 2020 •

edited

Loading

Neumann-A May 25, 2020 •

edited

Loading

BillyONeal May 25, 2020

strega-nil May 28, 2020

azure-pipelines bot commented May 29, 2020

strega-nil May 29, 2020

BillyONeal May 29, 2020

BillyONeal May 29, 2020

BillyONeal May 29, 2020

strega-nil May 29, 2020

BillyONeal May 29, 2020

JackBoosY left a comment

BillyONeal commented Jun 3, 2020

Maximus5 Jun 4, 2020

[vcpkg] Allow CI to pass in all relevant directories and remove use of symbolic links #11483

[vcpkg] Allow CI to pass in all relevant directories and remove use of symbolic links #11483

Conversation

BillyONeal commented May 21, 2020

MVoz commented May 22, 2020

BillyONeal commented May 25, 2020 • edited Loading

Neumann-A May 25, 2020 • edited Loading

Choose a reason for hiding this comment

Neumann-A May 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azure-pipelines bot commented May 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JackBoosY left a comment

Choose a reason for hiding this comment

BillyONeal commented Jun 3, 2020

Choose a reason for hiding this comment

BillyONeal commented May 25, 2020 •

edited

Loading

Neumann-A May 25, 2020 •

edited

Loading

Neumann-A May 25, 2020 •

edited

Loading