Automatically add references to core framework assemblies when targeting .NET Framework #379

dsplaisted · 2016-11-10T20:47:44Z

Right now, if you create an ASP.NET Core application targeting .NET Framework, you won't get references to any framework assemblies except mscorlib. This means a bunch of types (Uri, Linq, etc) won't be available to you.

We could add these to the templates, but in the interest of keeping the .csproj files concise I think it's better to reference them automatically in the SDK targets. That's what this PR does.

The list of assemblies comes from what you get by default when creating a new .NET Framework Class Library. For parity with .NET Standard and .NET Core projects, I'd suggest we should reference all assemblies that contain types that are in .NET Standard 2.0. @weshaggard @ericstj @terrajobst can you provide a list of assemblies you would need to reference in .NET Framework to get all the types that are in .NET Standard 2.0?

…ing .NET Framework

davidfowl · 2016-11-10T20:54:44Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <_TargetFrameworkMinorVersion Condition="$(_TargetFrameworkVersionSegmentCount) > 1">$(TargetFrameworkVersion.Split('.')[1])</_TargetFrameworkMinorVersion>
+  </PropertyGroup>
+
+  <ItemGroup Condition=" '$(DisableImplicitFrameworkReferences)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETFramework'">


This is bigger than the original set of default references in project.json/DNX. How were these chosen? Based on the .NET Framework default template today? Can we make this a bit smaller?

Based on the .NET Framework default template today?

Yes, that's what I based it on. However, I'm suggesting that we make it even bigger by default, to include all the types that are in .NET Standard 2.0. That way you'll have access to all .NET Standard 2.0 types by default whether you are targeting .NET Core or .NET Framework.

nguerrera

Migration currently adds the default refs from project.json. If we land on a superset of that, then we should adjust the migration. File a bug on CLI for that.

nguerrera · 2016-11-11T02:31:25Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <_TargetFrameworkMajorVersion>$(TargetFrameworkVersion.Split('.')[0])</_TargetFrameworkMajorVersion>
+    <_TargetFrameworkMajorVersion Condition="$(_TargetFrameworkMajorVersion.StartsWith('v'))">$(TargetFrameworkVersion.Substring(1))</_TargetFrameworkMajorVersion>
+    <_TargetFrameworkMinorVersion Condition="$(_TargetFrameworkVersionSegmentCount) > 1">$(TargetFrameworkVersion.Split('.')[1])</_TargetFrameworkMinorVersion>
+  </PropertyGroup>


I don't think this string manipulation is necessary. IIRC, @rainersigwald mentioned elsewhere that MSBuild can will automagically compare System.Version-parseable strings and compare them as such.

That is true, though System.Version.TryParse will fail on a v-prefixed string so you'll probably need to do that still.

@rainersigwald What's the syntax to compare version numbers in MSBuild? Or do strings that parse as version numbers automatically get compared as such?

The latter: strings that parse as version numbers get compared that way with the usual comparison operators.

nguerrera · 2016-11-11T02:34:51Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <Reference Include="Microsoft.CSharp"/>
+    <Reference Include="System.Data"/>
+    <Reference Include="System.Net.Http" Condition="($(_TargetFrameworkMajorVersion) > 4) Or ($(_TargetFrameworkMajorVersion) == 4 And $(_TargetFrameworkMinorVersion) >= 5)"/>
+    <Reference Include="System.Xml"/>


nit: sort these alphabetically.

nguerrera · 2016-11-11T02:35:34Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <Reference Include="System.Data.DataSetExtensions"/>
+    <Reference Include="Microsoft.CSharp"/>
+    <Reference Include="System.Data"/>
+    <Reference Include="System.Net.Http" Condition="($(_TargetFrameworkMajorVersion) > 4) Or ($(_TargetFrameworkMajorVersion) == 4 And $(_TargetFrameworkMinorVersion) >= 5)"/>


There are other conditions needed for version < 4, right?

Tests won't pass with version < 4 but that's only because of dotnet/msbuild#1333 combined with tests always running with core msbuild as of now. We keep getting requests for it, so we shouldn't discount version < 4 here.

Also update the test to make sure there aren't any warnings about references that can't be found

dsplaisted · 2016-11-12T00:08:14Z

I've updated the list of references to be based on what is necessary to make the types in .NET Standard available, based on this list: https://github.com/dotnet/standard/tree/master/platforms/net461

From that list, I've excluded System.ComponentModel.Composition, which is only there to support a type forward for Lazy<T, TMetadata>, and System.Web, which is only there for HttpUtility, but has a whole bunch of other stuff we don't want to bring in.

I've also added conditions to the references so that warnings won't be generated when you target .NET Framework 3.5 or 4.0, which don't have all of these assemblies. I've also sorted them.

dsplaisted · 2016-11-14T20:37:31Z

@nguerrera This is ready for you to re-review now

nguerrera · 2016-11-14T20:54:48Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <Reference Include="System.Core"/>
+    <Reference Include="System.Data"/>
+    <Reference Include="System.Drawing"/>
+    <Reference Include="System.IO.Compression.FileSystem" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>


Maybe worth a comment that these comparisons actually do the right thing in msbuild

nguerrera · 2016-11-14T20:55:21Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

+    <Reference Include="System.Numerics" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.0' "/>
+    <Reference Include="System.Runtime.Serialization"/>
+    <Reference Include="System.Xml.Linq"/>
+    <Reference Include="System.Xml"/>


project.json had Microsoft.CSharp, should we do that if we're in a C# project?

I don't have a strong opinion, but I left it out because it's not currently included in .NET Standard.

nguerrera · 2016-11-15T18:40:06Z

Also, is there a bug on the CLI to remove now redundant references from migration. What will be the behavior for migrated projects from the current preview when they get the next one and start to have dup references?

dsplaisted · 2016-11-16T01:13:55Z

Also, is there a bug on the CLI to remove now redundant references from migration. What will be the behavior for migrated projects from the current preview when they get the next one and start to have dup references?

I filed dotnet/cli#4695 to remove the redundant references. Duplicate references don't appear to be a problem, I think MSBuild ends up merging them.

Update BuildTools

* Tags become choices, lots of help changes * Rebased on rel/sv2017/post-rtw * Post-Rebase cleanup of outputs for various situations. Moved tags to symbols on 2.0 templates * Reverted to tags in 1.x templates and Item templates * Removed extra space from tags secifications in 1.x templates * Reverted to tags in 2.0 templates * Fixed misspelling of 'language' in 2.0 template.json files * Changes based on review comments (not including case-insensitive reads yet) * Rebased on rel/vs2017/post-rtw * Made reading template config & cache info be case-insensitive. * Made cache parameter properties get only * Cleanup of help & ItemplateInfo * Minor cleanup, mostly removing no longer needed async tasks * More cleanup * Tags become choices, lots of help changes * Rebased on rel/sv2017/post-rtw * Post-Rebase cleanup of outputs for various situations. Moved tags to symbols on 2.0 templates * Reverted to tags in 1.x templates and Item templates * Removed extra space from tags secifications in 1.x templates * Reverted to tags in 2.0 templates * Fixed misspelling of 'language' in 2.0 template.json files * Cleaning up UI interactions. Better experience for: problems with extra args files (dotnet#327, dotnet#329); default not in choice list dotnet#271 * Added support for split template configuration files (dotnet#150) * Cleanup of nulls in the template cache * fixed a merge mistake * More fixes from merge issues * Make fallback host files work correctly * Added wildcard to host matched. Fixed a bug with wildcard matching in zips * Add filter switch, hide all switch, add logic for accepting explicitly set filters * Change filter to type * Give instructions to call --help instead of showing help on error * Remove unused using * Fixes dotnet#380 and dotnet#379 * Respond to review feedback * Make the message shown for the invalid params match between help and non-help cases * Set IsPackable=false for new test projects (dotnet/templating#376). The rationale for this is that creating NuGet packages from tests is probably not the more-common use case and so by default these project should not participate when "dotnet pack" is run at the solution level. This doesn't prevent users from opting into "dotnet pack", it simply won't be the default. * Make IsPackable configurable for test project template (dotnet/templating#377). * Use '-p' as the short form for '--enable-pack' in MSTest v1 project template. * Implement "--enable-pack" parameter for test project templates (dotnet/templating#377). This parameter, if set to "true", marks the project as packable (i.e. participates in solution-level calls to "dotnet pack", producing a NuGet package). * Build for non-Windows platforms Refactor to use latest cli Add RestoreSource for dotnet-new3 project Build with framework version and runtime id Fix test cases * Move testbase location to local directory * Updating the identity of the 2.0 templates * Better matching across templates with same group identity. * Added precedence to all template.json files * Reading & writing template Precedence * Template matching uses Precedence to help disambiguate templates * fix for dotnet#395 * Updated package refs in test projects * fix for exclude, include, & copy only configuration in sources * Mikes test harness for testing dotnet new commands. Tests for precedence disambiguation * Fixes per Mike's CR * renamed the mvc precendence tests * Add 3PN for nuget.exe * Allow runtime to be passed in * Handling reading different cache formats, and upgrading the cache aut… (dotnet#407) * Handling reading different cache formats, and upgrading the cache automatically as needed * Cleanup for cache versioning * Improvements to cache versioning. Language display fix. * Cache rebuild enhancement. Only scan mount points for templates and langpacks

Automatically add references to core framework assemblies when target…

900711b

…ing .NET Framework

dnfclas added the cla-already-signed label Nov 10, 2016

davidfowl reviewed Nov 10, 2016

View reviewed changes

nguerrera suggested changes Nov 11, 2016

View reviewed changes

nguerrera reviewed Nov 11, 2016

View reviewed changes

Add the rest of default references that have .NET Standard types

8c1e1af

Also update the test to make sure there aren't any warnings about references that can't be found

dsplaisted closed this Nov 14, 2016

dsplaisted reopened this Nov 14, 2016

dnfclas added the cla-already-signed label Nov 14, 2016

srivatsn approved these changes Nov 14, 2016

View reviewed changes

nguerrera approved these changes Nov 15, 2016

View reviewed changes

Add clarifying comment

46698d8

dsplaisted merged commit 0908e35 into dotnet:master Nov 16, 2016

dsplaisted deleted the AutomaticFrameworkReferences branch November 16, 2016 01:29

nguerrera mentioned this pull request Dec 19, 2016

Targeting net20 results in broken reference warnings #532

Closed

sbomer pushed a commit to sbomer/sdk that referenced this pull request Sep 19, 2017

Merge pull request dotnet#379 from ellismg/update-buildtools

3baeac8

Update BuildTools

dsplaisted mentioned this pull request Jan 31, 2020

dotnet migrate should not add Framework references that the SDK will include by default #7139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically add references to core framework assemblies when targeting .NET Framework #379

Automatically add references to core framework assemblies when targeting .NET Framework #379

dsplaisted commented Nov 10, 2016

davidfowl Nov 10, 2016

dsplaisted Nov 10, 2016

nguerrera left a comment

nguerrera Nov 11, 2016

rainersigwald Nov 11, 2016

dsplaisted Nov 11, 2016

rainersigwald Nov 11, 2016

nguerrera Nov 11, 2016

nguerrera Nov 11, 2016

nguerrera Nov 11, 2016

dsplaisted commented Nov 12, 2016

dsplaisted commented Nov 14, 2016

nguerrera Nov 14, 2016

nguerrera Nov 14, 2016

dsplaisted Nov 16, 2016

nguerrera commented Nov 15, 2016

dsplaisted commented Nov 16, 2016

Automatically add references to core framework assemblies when targeting .NET Framework #379

Automatically add references to core framework assemblies when targeting .NET Framework #379

Conversation

dsplaisted commented Nov 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguerrera left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsplaisted commented Nov 12, 2016

dsplaisted commented Nov 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguerrera commented Nov 15, 2016

dsplaisted commented Nov 16, 2016