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

Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet in .vcxproj, introduces other odd behaviors #12062

Closed
aggieNick02 opened this issue Jun 22, 2020 · 7 comments · Fixed by #12257
Assignees
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`)

Comments

@aggieNick02
Copy link
Contributor

aggieNick02 commented Jun 22, 2020

Commit 1451450 seems really problematic. While being able to edit these properties in the VS GUI is awesome, the issues it is causing are serious to enough to justify reverting until they are addressed.

If I have an existing project with VcpkgTriplet set in the .vcxproj as the documentation suggests, then before the commit, vcpkg respects that triplet. After, it does not. Additionally, there are other bits of wonky behavior I'm seeing, depending on machine and VS version.

Environment
Reproduced as follows. Imagine happens on VS2017 as well.

  • OS: Windows 1903, Windows 1909
  • Compiler: VS2015, VS2019

To Reproduce
Steps to reproduce:

  • Begin by creating a vcpkg install right before the commit. We'll install the "minhook" package just to have something to include. From powershell, run
git clone https:/Microsoft/vcpkg.git vcpkg_before
cd vcpkg_before
git checkout 14514508d8d30bdbd645b2bec89696aec25497f1~1
.\bootstrap-vcpkg.bat
.\vcpkg.exe integrate install
.\vcpkg.exe install minhook:x86-windows-static
  • Leave powershell open.
  • Now start visual studio (reproduced with 2015 and 2019), create a new c++ console application project with solution, and then edit the .vcxproj project file to have the line:
    <VcpkgTriplet>x86-windows-static</VcpkgTriplet>
    at the bottom of the
    <PropertyGroup Label="Globals">
    section. Reload the project when visual studio prompts. Then add
    #include "minhook.h"
    immediately before the main() function.
  • Build the solution. It will succeed.
  • Now close visual studio. We will now move to vcpkg with the problematic commit.
    Go back to powershell in vcpkg_before and run the following commands:
.\vcpkg.exe integrate remove
cd ..
git clone https:/Microsoft/vcpkg.git vcpkg_after
cd vcpkg_after
git checkout 14514508d8d30bdbd645b2bec89696aec25497f1
.\bootstrap-vcpkg.bat
.\vcpkg.exe integrate install
.\vcpkg.exe install minhook:x86-windows-static
  • Now launch visual studio and open the previous solution. Rebuild the solution. It fails.
  • Note that the build output specifies that it is using x86-windows even though the project file specifies x86-windows-static.
  • In VS2015 only, if you move the
    <VcpkgTriplet>x86-windows-static</VcpkgTriplet>
    line in the vcxproj to the "Configuration" PropertyGroup for Debug|Win32, it will then use "x86-windows-static", which is correct, but from the wrong location "C:\vckpg_after\installed\x86-windows", and still fail.
  • On a non-clean windows image (my dev machine with a slightly older VS2015 Update 3 (14.0.25425.01), the behavior is even wonkier. In that scenario, the triplet in the property page is defaulted to "$(PlatformTarget)-$(VcpkgOSTarget)" and building with vcpkg_after outputs
    "Using triplet "-windows" from "F:\vcpkg\installed\-windows\"

Expected behavior
Updating to the latest vcpkg should not break existing working projects

@aggieNick02 aggieNick02 changed the title Feature that adds vcpkj item to project settings in visual studio breaks existing VcpkgTriplet properties in projects, causes other major issues Feature that adds vcpkg item to project settings in visual studio breaks existing VcpkgTriplet properties in projects, causes other major issues Jun 22, 2020
@aggieNick02 aggieNick02 changed the title Feature that adds vcpkg item to project settings in visual studio breaks existing VcpkgTriplet properties in projects, causes other major issues Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet .vcxproj, introduces other odd behaviors Jun 22, 2020
@aggieNick02 aggieNick02 changed the title Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet .vcxproj, introduces other odd behaviors Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet .vcxproj, introduces other odd behaviors Jun 22, 2020
@aggieNick02 aggieNick02 changed the title Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet .vcxproj, introduces other odd behaviors Feature that adds vcpkg item to project settings in visual studio breaks builds that set VcpkgTriplet in .vcxproj, introduces other odd behaviors Jun 22, 2020
@aggieNick02
Copy link
Contributor Author

Sorry for the noise as I figured out git markdown and fixed typos. It should be stable now. Let me know if you want any files to repro or more details. I imagine this will break any project that prefers to link statically to vcpkg dependencies, so reverting may be prudent.

@Neumann-A @cbezault @BillyONeal @ras0219-msft

Also, once this new feature is in and stable, the documentation should probably updated to explain both the new functionality and its relation to setting VcpkgTriplet directly in the .vcxproj.

aggieNick02 referenced this issue Jun 22, 2020
Co-authored-by: Curtis J Bezault <[email protected]>
Co-authored-by: Billy Robert O'Neal III <[email protected]>
Co-authored-by: Robert Schumacher <[email protected]>
@BillyONeal
Copy link
Member

This is because we accidentally overwrite VcpkgTriplet even when it has already been set in your project file.

@BillyONeal
Copy link
Member

<VcpkgTriplet Condition="'$(VcpkgTriplet)'!='$(VcpkgUserTriplet)'">$(VcpkgUserTriplet)</VcpkgTriplet>
<-- Right here. I wanted to give Nicole's change ~2 days to land to avoid another merge mess as it heavily edits that file to add manifests.

@aggieNick02
Copy link
Contributor Author

That makes sense, thanks for pointing out where it is happening. I'm not sure how the wonky behavior in 2015 is happening (where the VcpkgTriplet is sometimes honored but the corresponding directory is wrong)... might be worth considering just disabling 2015 support if necessary.

@JackBoosY JackBoosY self-assigned this Jun 23, 2020
@JackBoosY JackBoosY added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Jun 23, 2020
@aggieNick02
Copy link
Contributor Author

Just as a heads up, the issue filed at #12086 is mentioned in this issue as an "odd behavior", and caused by the same commit.

@aggieNick02
Copy link
Contributor Author

Just checking in on this issue as it seems pretty critical. Any ETA? Is the current plan to rollback the problematic commit or to remedy it?

@ras0219
Copy link
Contributor

ras0219 commented Jul 3, 2020

I'm looking into this now.

ras0219 pushed a commit to ras0219/vcpkg that referenced this issue Jul 4, 2020
This PR also renames the VcpkgUserTriplet MSBuild variable to VcpkgTriplet to minimize user confusion compared to previous practice and documentation.
ras0219-msft added a commit that referenced this issue Jul 8, 2020
This PR also renames the VcpkgUserTriplet MSBuild variable to VcpkgTriplet to minimize user confusion compared to previous practice and documentation.

Co-authored-by: Robert Schumacher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants