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

Wrong VcpkgInstalledDir with MSBuild, vcpkg manifest mode and custom triplet #21513

Closed
meastp opened this issue Nov 18, 2021 · 27 comments
Closed
Assignees
Labels
category:question This issue is a question info:manifests This PR or Issue pertains to the Manifests feature

Comments

@meastp
Copy link
Contributor

meastp commented Nov 18, 2021

Describe the bug
Building with MSBuild has stopped working. I recently updated vcpkg from an older version , and with a project in C:\myproj and a C:\myproj\vcpkg.json file (using manifest mode), I have found:

Using (in C:\myproj) C:\vcpkg.exe install --overlay-triplets=C:\myproj\triplets --triplet=x64-windows-v142 --host-triplet=x64-windows-v142 works as expected, with C:\myproc\vcpkg_installed\vcpkg and C:\myproj\vcpkg_installed\x64-windows-v142 are created.

However, when building within Visual Studio (with <VcpkgManifestInstall>true</VcpkgManifestInstall>), the folders in vcpkg_installed are different: C:\myproc\vcpkg_installed\x64-windows-v142\vcpkg and C:\myproj\vcpkg_installed\x64-windows-v142\x64-windows-v142 are created instead, which must be an error.

Build started...
1>------ Up-To-Date check: Project: myproj.vcxproj, Configuration: Debug x64 ------
1>Project is not up-to-date: build input 'c:\myproj\vcpkg_installed\x64-windows-v142\x64-windows-v142\include\boost\uuid\uuid.hpp' is missing.
...
1>Property reassignment: $(_ZVcpkgInstalledDir)="C:\myproj\vcpkg_installed\x64-windows-v142\" (previous value: "") at C:\vcpkg\scripts\buildsystems\msbuild\vcpkg.targets (54,9)
...
1>_ZVcpkgConfigSubdir            = debug\
1>_ZVcpkgConfigurationFileLocation = C:\myproj\vcpkg-configuration.json
1>_ZVcpkgCurrentInstalledDir     = C:\myproj\vcpkg_installed\x64-windows-v142\\x64-windows-v142\
1>_ZVcpkgExecutable              = C:\vcpkg\vcpkg.exe
1>_ZVcpkgHostTripletParameter    = "--host-triplet=x64-windows-v142"
1>_ZVcpkgHostTripletSuffix       = x64-windows-v142.
1>_ZVcpkgInstalledDir            = C:\myproj\vcpkg_installed\x64-windows-v142\
1>_ZVcpkgLinkage                 = 
1>_ZVcpkgManifestFileLocation    = C:\myproj\vcpkg.json
1>_ZVcpkgManifestRoot            = C:\myproj\
1>_ZVcpkgMSBuildStampFile        = C:\myproj\vcpkg_installed\x64-windows-v142\.msbuildstamp-x64-windows-v142.x64-windows-v142.stamp
1>_ZVcpkgNormalizedConfiguration = Debug
1>_ZVcpkgRoot                    = C:\vcpkg\

Environment

  • OS: Windows
  • Compiler:
Microsoft Visual Studio Enterprise 2019
Version 16.11.3
VisualStudio.16.Release/16.11.3+31702.278
Microsoft .NET Framework
Version 4.8.04084

Installed Version: Enterprise

Visual C++ 2019   00435-60000-00000-AA434
Microsoft Visual C++ 2019

If the culprit isn't immediately obvious to you who know the code, I can try to make a complete repro.

@autoantwort
Copy link
Contributor

which must be an error.

No, this is an intended change of #19767 to allow different triplets in one solution/project

@dg0yt
Copy link
Contributor

dg0yt commented Nov 18, 2021

1>_ZVcpkgCurrentInstalledDir     = C:\myproj\vcpkg_installed\x64-windows-v142\\x64-windows-v142\

This is intented, really?

@autoantwort
Copy link
Contributor

Yes

@dg0yt
Copy link
Contributor

dg0yt commented Nov 18, 2021

Does x64-windows-v142\\x64-windows-v142 include the host triplet, or why does it need x64-windows-v142 twice?

@autoantwort
Copy link
Contributor

Each triplet has its own installed dir under the triplet name. So the installed dir for a triplet is vcpkg_installed/<triplet>. And in this installed dir there are the normal <triplet>, <host_triplet>, vcpkg folders.
This allows you to have a solution with two projects with different triplets to build. Otherwise the projects uninstalls the packages for the triplet of the other project each time you build the projects

@meastp
Copy link
Contributor Author

meastp commented Nov 19, 2021

The two methods presented differ in behaviour. Is this also intended? It does mean that I can not use vcpkg install from powershell to pre-seed for a later msbuild invocation...

@dg0yt
Copy link
Contributor

dg0yt commented Nov 19, 2021

It does mean that I can not use vcpkg install from powershell to pre-seed for a later msbuild invocation...

You do pre-fill the artifact cache. But only if vcpkg install actually targets the same configuration.

@meastp
Copy link
Contributor Author

meastp commented Nov 19, 2021

@dg0yt This is what I tried - look at the invocation from my the issue description. The (binary) files are placed in different directories... So, they are not used when invoking MSBuild, because MSBuild now expects the binary files in vcpkg_installed\x64-windows-v142\x64-windows-v142\bin while when invoking vcpkg install from powershell places the files in vcpkg_installed\x64-windows-v142\bin which is different...

@dg0yt
Copy link
Contributor

dg0yt commented Nov 19, 2021

By "artifact cache" I don't meant the installed tree, but the local cache containing zipped archives. vcpkg tries to restore from cache before building packages. So you do save at least the build time (but not the disk space).
The difficulty is to match the actual target configuration. If there is a difference, then there is a cache miss.

I can't really comment on VS particularities, but the situation in CMake is similar: a vcpkg install next to the manifest does create a different installed tree. It simply cannot know the binary directory and configuration which is used when vcpkg is invoked from the cmake toolchain.
This isn't sufficiently covered in the documentation.

@meastp
Copy link
Contributor Author

meastp commented Nov 19, 2021

This might be a big problem - today I use <VcpkgManifestInstall>false</VcpkgManifestInstall> and pre-install the dependencies/vcpkg_installed-directory using powershell because the MSBuild Manifest install features disables multi-processor builds (at least it did before) and caused unnecessary long build times. So the disk space is not the only issue at all!

I can't really comment on VS particularities, but the situation in CMake is similar: a vcpkg install next to the manifest does create a different installed tree. It simply cannot know the binary directory and configuration which is used when vcpkg is invoked from the cmake toolchain.

That's a problem. :( Are there parameters I can use when invoking vcpkg install from powershell that I'm not using atm? (see first post)? Because I know the complete config, I should be able to pass it to vcpkg install manually...

@meastp
Copy link
Contributor Author

meastp commented Nov 19, 2021

The disabling of multi-processor builds is due to the parameter --x-wait-for-lock being passed to vcpkg install by MSBuild.

I would have no problem using manifest install in MSBuild if multi-processor builds worked, but as it is now, the build runs sequentially and is a lot slower. This is the reason i prefer pre-seeding using powershell vcpkg install.

@JackBoosY JackBoosY added the info:manifests This PR or Issue pertains to the Manifests feature label Nov 19, 2021
@autoantwort
Copy link
Contributor

It does mean that I can not use vcpkg install from powershell to pre-seed for a later msbuild invocation...

You can, but you have to set the installed dir properly (--x-install-root=...)

because the MSBuild Manifest install features disables multi-processor builds (at least it did before) and caused unnecessary long build times.

This was also improved

the build runs sequentially and is a lot slower

No this is not the case. Only at the start they are all blocked until vcpkg has installed the dependencies. So the total time should be roughly the same

@autoantwort
Copy link
Contributor

Do you build in visual Studio or for a ci? With the updated integration vcpkg is not invoked at all when the dependencies are already installed.

@meastp
Copy link
Contributor Author

meastp commented Nov 22, 2021

@autoantwort I build both in Visual Studio (on my developer machine) and in CI (using Azure's Microsoft-Hosted Pipeline Agent(s)). I'd love to just use manifest install in both cases, but only if the build is not sequental. Will investigate, thanks :)

@mdubepsi
Copy link

mdubepsi commented Jan 26, 2022

Hi, I'm trying to update from vcpkg pinned at around 2021 July 1st to latest and I'm running into the same issue, where, in manifest mode, the triplets folder is added twice to the install location. Since commit 97e7ac8, the $(VcpkgTriplet) is added twice, at lines 54 and 65 below:

<Choose>
<When Condition="'$(VcpkgEnableManifest)' == 'true'">
<PropertyGroup>
<_ZVcpkgInstalledDir Condition="'$(_ZVcpkgInstalledDir)' == ''">$(_ZVcpkgManifestRoot)vcpkg_installed\$(VcpkgTriplet)\</_ZVcpkgInstalledDir>
</PropertyGroup>
</When>
<Otherwise>
<PropertyGroup>
<_ZVcpkgInstalledDir Condition="'$(_ZVcpkgInstalledDir)' == ''">$(_ZVcpkgRoot)installed\</_ZVcpkgInstalledDir>
</PropertyGroup>
</Otherwise>
</Choose>
<PropertyGroup>
<_ZVcpkgCurrentInstalledDir>$(_ZVcpkgInstalledDir)$(VcpkgTriplet)\</_ZVcpkgCurrentInstalledDir>
<_ZVcpkgNormalizedConfiguration Condition="$(VcpkgConfiguration.StartsWith('Debug'))">Debug</_ZVcpkgNormalizedConfiguration>
<_ZVcpkgNormalizedConfiguration Condition="$(VcpkgConfiguration.StartsWith('Release')) or '$(VcpkgConfiguration)' == 'RelWithDebInfo' or '$(VcpkgConfiguration)' == 'MinSizeRel'">Release</_ZVcpkgNormalizedConfiguration>
<_ZVcpkgConfigSubdir Condition="'$(_ZVcpkgNormalizedConfiguration)' == 'Debug'">debug\</_ZVcpkgConfigSubdir>
<_ZVcpkgExecutable>$(_ZVcpkgRoot)vcpkg.exe</_ZVcpkgExecutable>
<ExternalIncludePath>$(ExternalIncludePath);$(_ZVcpkgCurrentInstalledDir)include</ExternalIncludePath>
</PropertyGroup>

Btw, I'm not using a custom triplet.

Many comments above state that this is intended, but I don't see how adding twice the same information, by doubling $(VcpkgTriplet), would help something. Maybe I'm missing some details.

@autoantwort
Copy link
Contributor

autoantwort commented Jan 26, 2022

Many comments above state that this is intended, but I don't see how adding twice the same information, by doubling $(VcpkgTriplet), would help something. Maybe I'm missing some details.

It fixes #16083 (or makes the workaround the default)

@mdubepsi
Copy link

Ok, thanks. I guess having manifest behave the same as non-manifest, where triplets accumulate in the 'installed' folder involves deeper change to vcpkg_tool and this thread is then about that.

Not wanting to hijack the thread, but this change (to fix #16083) seems to have changed the behavior regarding $(VcpkgInstalledDir) which was populated before and left empty by default now. @autoantwort maybe you can pinpoint me to other details that could help me understand. I would happily try to help around those issues, but I'm starting way further than you at having the big picture. Thanks.

@autoantwort
Copy link
Contributor

Yeah $(VcpkgInstalledDir) is not prefilled anymore. If it is empty is uses the default.

What is your problem or what are you trying to solve?

@autoantwort
Copy link
Contributor

If you want to see what changed, have a look at #19767. If you want to know why it changed see #18906.

@mdubepsi
Copy link

mdubepsi commented Jan 26, 2022

I'm trying to solve that updating vcpkg (from ~July 1st, 2021 to latest) breaks our scripts because of two reasons:

  1. $(VcpkgInstalledDir) is not filled anymore. We use it to copy DLLs or access some files from some ports;
  2. Install location in manifest mode is moved in a second folder (as reported by this issue). This exacerbate the first issue, because if $(VcpkgInstalledDir) was set, we would probably not have noticed the folder tree changes (by doubling $(VcpkgTriplet)).

For the first, having $(VcpkgInstalledDir) populated can be patched on our side, but I think being able to have it built-in is good.

For the second, even if we don't currently build for both x86 and x64, I can appreciate the fix for #16083 (as when I noticed that behavior, I though this was by design that install folder would get cleared when switching between triplets in manifest mode). In non manifest mode, both x86 and x64 can lie in the same 'installed' folder, so it would be perfect if manifest mode would behave the same, but I guess this is complicated.

If you tell me that both those are dead ends, I'll just live with it, else I can look at proposing patches/MRs. We can currently postpone our upgrade too.

Edit: got your message about issues explaining the how and why. I'll have a look.

@mdubepsi
Copy link

With a simple change in vcpkg-tool, I was able to make it work in manifest mode similarly to non-manifest mode by not considering package from other triplets:

mdubepsi/vcpkg-tool@a816a75

It must be combined to a change in vcpkg itself:

mdubepsi@38164c9

With those, I'm able to switch between configurations (x64, win32(x86)) in Visual Studio without vcpkg to remove packages from the other triplet without having to have nested install in vcpkg_installed\$(VcpkgTriplet)\$(VcpkgTriplet)\

@meastp can you give it a try?

@autoantwort
Copy link
Contributor

autoantwort commented Jan 27, 2022

I think the simplest solution is to change VcpkgInstalledDir to $(VcpkgManifestRoot)vcpkg_installed to get the old bahavior

@mdubepsi
Copy link

mdubepsi commented Jan 27, 2022

This is indeed a simpler solution, involving no change in code, but I think having vcpkg-tool behave the same in both modes, by supporting to have many triplets in the installed folder (as it already do in non-manifest mode), would be a more robust solution, working out of the box, without having to change some optional parameter. This would also fix the current issue where the outcome is different depending if install is made from within Visual Studio or outside.

I think VcpkgInstalledDir should be an option to change the install directory, not to work around a non-orthogonal behavior. Those having trouble with multiple configurations in #16083 could have worked around the problem by changing VcpkgInstalledDir to something like $(VcpkgManifestRoot)vcpkg_installed\$(PlatformTarget), but again, this would be counter-intuitive and possibly not even undocumented.

Of course, this assumes the change doesn't break anything else and is acceptable as an upstream change.

@autoantwort
Copy link
Contributor

I think having vcpkg-tool behave the same in both modes, by supporting to have many triplets in the installed folder (as it already do in non-manifest mode), would be a more robust solution, working out of the box, without having to change some optional parameter.

I don't think that this will happen. The idea of manifest mode is that the installed dir only contains the dependencies specified in your vcpkg.json file for a specific triplet.

I think VcpkgInstalledDir should be an option to change the install directory, not to work around a non-orthogonal behavior. Those having trouble with multiple configurations in #16083 could have worked around the problem by changing VcpkgInstalledDir to something like $(VcpkgManifestRoot)vcpkg_installed$(PlatformTarget).

This behavior change is only a problem for some exiting users that do a upgrade, for all new users the new default is better.

@mdubepsi
Copy link

mdubepsi commented Jan 27, 2022

May be better as I probably don't see the full extend of it, but also uglier as having x64-windows\s64-windows and x86-windows\x86-windows looks like a workaround at first sight.

I appreciate vcpkg a lot, I'm happy to finally get some time to try to contribute. Is there a way to have this change: mdubepsi@38164c9 considered, reviewed and discussed outside this thread?

I don't think that this will happen. The idea of manifest mode is that the installed dir only contains the dependencies specified in your vcpkg.json file for a specific triplet.

Maybe this answer should have been given to those asking to support multiple configurations without having to replace all packages when switching between x86 and x64? As I said, I noticed this behavior and told myself that this was probably by design at that time.

@autoantwort
Copy link
Contributor

autoantwort commented Jan 27, 2022

having x64-windows\s64-windows and x86-windows\x86-windows looks like a workaround at first sight.

This is effectively the same as for cmake. But there it is <folder_for_my_x86-windows_build>/vcpkg_installed/x86-windows

I appreciate vcpkg a lot, I'm happy to finally get some time to try to contribute. Is there a way to have this change: mdubepsi/vcpkg@38164c9 considered, reviewed and discussed outside this thread?

You can create a new PR for that.

Maybe this answer should have been given to those asking to support multiple configurations without having to replace all packages when switching between x86 and x64?

Maybe, but switching between x86 and x64 now fully works without a problem.

@meastp
Copy link
Contributor Author

meastp commented Jan 27, 2022

@mdubepsi @autoantwort FYI: I removed the separate cmd line vcpkg install ... step to pre-populate vcpkg_installed (since this won't work anyway, due to the differing directory layouts) and I'm now just building with Visual Studio with manifest install enabled.

The original reason why I added the pre-populate step was because of a slowdown/lock with VS manifest install which now appear to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:question This issue is a question info:manifests This PR or Issue pertains to the Manifests feature
Projects
None yet
Development

No branches or pull requests

5 participants