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

Use documented item group in FindInvalidProjectReferences #10220

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

jkoritzinsky
Copy link
Member

Context

Today, GetReferenceTargetPlatformMonikers and FindInvalidProjectReferences use TargetPathWithTargetPlatformMoniker instead of the documented _ProjectReferenceTargetPlatformMonikers item group for the project reference gathering/validation.

This causes a violation in the ProjectReference protocol, leading to multiple items being returned from GetTargetPath and the default (Build) target. Returning multiple items from these targets can cause usability problems and unexpected behaviors.

Below is a simplified use case of the one that hit this bug:

A/A.csproj:

<Project Sdk="Microsoft.NET.Sdk">
      <PropertyGroup>
          <TargetFramework>net8.0</TargetFramework>
      </PropertyGroup>
</Project>

B/B.csproj:

<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
       <FindInvalidProjectReferences>true</FindInvalidProjectReferences>
       <TargetFramework>net8.0-windows10.0.22621.0</TargetFramework>
       <TargetPlatformMinVersion>10.0.17763.0</TargetPlatformMinVersion>
    </PropertyGroup>
   <ItemGroup>
       <ProjectReference Include="../A/A.csproj" />
   </ItemGroup>
</Project>

C/C.csproj

<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
       <OutputType>Exe</OutputType>
       <TargetFramework>net8.0-windows10.0.22621.0</TargetFramework>
       <TargetPlatformMinVersion>10.0.17763.0</TargetPlatformMinVersion>
    </PropertyGroup>
   <ItemGroup>
       <ProjectReference Include="../A/A.csproj" Aliases="A" />
       <ProjectReference Include="../B/B.csproj" Aliases="B" />
   </ItemGroup>
</Project>

C/Program.cs

extern alias A; // Csc errors here with "alias not found"
                       // because the reference to B.csproj added items for A.dll and B.dll with the B alias metadata
                       // which overrode the item returned by A.csproj, the A.dll with the A alias metadata
Console.WriteLine("Hello world");

Changes Made

Use the documented item group instead of reusing TargetPathWithTargetPlatformMoniker.

Testing

Local hacking. I'd like to add an end-to-end test, but I'm not sure where is the best place to do so here.

Notes

This was discovered in a project that uses the Microsoft.WindowsAppSdk, which sets FindInvalidProjectReferences to true in some scenarios.

The user was able to work around this bug by changing the order of the ProjectReference items in their equivalent of C.csproj.

Use the documented item group instead of reusing `TargetPathWithTargetPlatformMoniker`.
@Sergio0694
Copy link
Contributor

For context, I hit this in Sergio0694/ComputeSharp#813, and it completely broke my build due to multiple projects failing due to this issue (in one case due to missing/incorrect reference aliases, in another case because ReferenceOutputAssembly was being ignored). Special thanks to Jeremy for investigating this and both figuring out a workaround as well as creating this PR 🙂

Copy link
Contributor

@Nirmal4G Nirmal4G left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that this could be a simple copy-paste mistake since git blame seems to stop at v15!

@rainersigwald
Copy link
Member

rainersigwald commented Jul 31, 2024

I assume that this could be a simple copy-paste mistake since git blame seems to stop at v15!

Internal link to change from (exactly!) 11 years ago: https://vstfdevdiv.corp.microsoft.com/DevDiv2/DevDiv/_versionControl/changeset/1033157

Add designtime or buildtime warning when referencing Dev12/WinBlue projects from Dev11/Win8 C++ projects

No details in the changeset to indicate that this was intentional.

@MichalPavlik MichalPavlik merged commit 027c64a into main Aug 5, 2024
10 checks passed
@MichalPavlik MichalPavlik deleted the find-invalid-project-references branch August 5, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants