Skip to content

Don't include base FrameworkReferences when creating NuGet packages #3102

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Don't include base FrameworkReferences when creating NuGet packages #3102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Copyright (c) .NET Foundation. All rights reserved.
Publish="true" />
</ItemGroup>
<ItemGroup Condition=" '$(DisableImplicitFrameworkReferences)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETStandard' And '$(_TargetFrameworkVersionWithoutV)' >= '2.1'">
<FrameworkReference Include="NETStandard.Library" IsImplicitlyDefined="true" />
<FrameworkReference Include="NETStandard.Library" IsImplicitlyDefined="true" Pack="false"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a new FrameworkReference?

What does it mean compared to the rest of the frameworks?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're (going to be) using a targeting pack for .NET Standard, so the FrameworkReference is how we bring that in.

With Pack=false I think this should not affect NuGet or NuGet packages (just like the base Microsoft.NETCore.App FrameworkReference will be handled).

Basically, FrameworkReferences which are represented by a target framework moniker should have Pack=false and not be represented in NuGet packages or flow transitively. Does Pack=false have this behavior with the FrameworkReference work?

Copy link
Contributor

@nkolev92 nkolev92 Apr 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The last part is new to me.
Restore does not account for Pack=false. It flows everything by default.

Basically the way it's currently working is targeted more for the optional shared frameworks.

Do we need to reconsider that? Do we need to add a PrivateAssets like switch to FrameworkReference items?

Copy link
Member Author

Choose a reason for hiding this comment

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

@nkolev92 Yes, we need to be able to prevent the NETStandard.Library FrameworkReference from flowing. Does Pack=false prevent it from being added to the NuGet package? To me it would make sense if that also prevented it from flowing through the projects, but we can also add a PrivateAssets-type flag to it if need be.

Copy link
Contributor

@nkolev92 nkolev92 Apr 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Restore does not read the "pack" attribute and it probably shouldn't.
pack=false will prevent it from being included in the package only.

We should discuss this in a separate meeting or a Monday sync-up.

The existence of a netstandard framework reference also kills our transitivity with FrameworkReference story.

https://github.com/NuGet/Home/wiki/%5BSpec%5D-FrameworkReference-in-NuGet#what-happens-in-the-future-ifwhen-frameworkreference-are-added-to-other-frameworks

We need to rethink that as well.

Issue: NuGet/Home#7988

//cc @rrelyea

</ItemGroup>

<ItemGroup Condition=" '$(DisableImplicitFrameworkReferences)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
Expand All @@ -65,6 +65,7 @@ Copyright (c) .NET Foundation. All rights reserved.
Publish="true" />

<FrameworkReference Include="Microsoft.NETCore.App" IsImplicitlyDefined="true"
Pack="false"
Condition="('$(_TargetFrameworkVersionWithoutV)' != '') And ('$(_TargetFrameworkVersionWithoutV)' >= '3.0')"/>

</ItemGroup>
Expand Down