Skip to content

Automatically reference assembly packs for .NET Framework#10981

Merged
sfoslund merged 3 commits into
dotnet:masterfrom
sfoslund:AutoRefTargetPack
Mar 27, 2020
Merged

Automatically reference assembly packs for .NET Framework#10981
sfoslund merged 3 commits into
dotnet:masterfrom
sfoslund:AutoRefTargetPack

Conversation

@sfoslund
Copy link
Copy Markdown
Member

Fixes #4009

@sfoslund sfoslund requested a review from a team March 24, 2020 22:32
Copy link
Copy Markdown
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

💯

Comment thread src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToUseVB.cs
</GetReferenceAssemblyPaths>

<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" IsImplicitlyDefined="true" Condition="'$(_FullFrameworkReferenceAssemblyPaths)' == ''"/>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's use a property here instead of hardcoding 1.0.0 for the version number. How about MicrosoftNETFrameworkReferenceAssembliesLatestPackageVersion?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you confirm that you tested restore in VS from a clean project?

My concern is: does VS use a design-time build to extract PackageReference when it does restore, or does it use only evaluated state, which wouldn't see this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

VS does use design-time builds. They would like to be able to restore based only on evaluation, but we add PackageReference and PackageDownload items in targets as a core part of the build for .NET Core, so it's not currently feasible for Sdk-style projects.

</GetReferenceAssemblyPaths>

<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" IsImplicitlyDefined="true" Condition="'$(_FullFrameworkReferenceAssemblyPaths)' == ''"/>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you confirm that you tested restore in VS from a clean project?

My concern is: does VS use a design-time build to extract PackageReference when it does restore, or does it use only evaluated state, which wouldn't see this?

</GetReferenceAssemblyPaths>

<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" IsImplicitlyDefined="true" Condition="'$(_FullFrameworkReferenceAssemblyPaths)' == ''"/>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if the project has an existing reference to this? Does this conflict or supersede?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It supersedes, I added test coverage in the latest update: fb3772a#diff-8ba7e75c46a5fe96b147fd79709da01eR116

@sfoslund sfoslund force-pushed the AutoRefTargetPack branch from fb3772a to c6b2f33 Compare March 26, 2020 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically reference .NET Framework reference assembly packages if needed

4 participants