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.

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

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

Merged
merged 3 commits into from
Nov 16, 2016
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
900711b
Automatically add references to core framework assemblies when target…
dsplaisted Nov 9, 2016
8c1e1af
Add the rest of default references that have .NET Standard types
dsplaisted Nov 11, 2016
46698d8
Add clarifying comment
dsplaisted Nov 16, 2016
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
23 changes: 23 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,29 @@ Copyright (c) .NET Foundation. All rights reserved.
<PublishDir Condition="'$(PublishDir)' == ''">$(OutputPath)$(PublishDirName)\</PublishDir>
</PropertyGroup>

<!-- For .NET Framework, reference core assemblies -->

<PropertyGroup>
<_TargetFrameworkVersionWithoutV>$(TargetFrameworkVersion)</_TargetFrameworkVersionWithoutV>
<_TargetFrameworkVersionWithoutV Condition="$(TargetFrameworkVersion.StartsWith('v'))">$(TargetFrameworkVersion.Substring(1))</_TargetFrameworkVersionWithoutV>
</PropertyGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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


<ItemGroup Condition=" '$(DisableImplicitFrameworkReferences)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETFramework'">
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

<Reference Include="System"/>
<Reference Include="System.Core"/>
<Reference Include="System.Data"/>
<Reference Include="System.Drawing"/>
<!-- When doing greater than/less than comparisons between strings, MSBuild will try to parse the strings as Version objects and compare them as
such if the parse succeeds. -->
<Reference Include="System.IO.Compression.FileSystem" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>
Copy link
Contributor

Choose a reason for hiding this comment

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

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

<Reference Include="System.IO.Compression" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>
<Reference Include="System.Net.Http" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.5' "/>
<Reference Include="System.Numerics" Condition=" '$(_TargetFrameworkVersionWithoutV)' >= '4.0' "/>
<Reference Include="System.Runtime.Serialization"/>
<Reference Include="System.Xml.Linq"/>
<Reference Include="System.Xml"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: sort these alphabetically.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

</ItemGroup>

<!-- Add conditional compilation symbols for the target framework (for example NET461, NETSTANDARD2_0, NETCOREAPP1_0) -->
<PropertyGroup Condition=" '$(DisableImplicitFrameworkDefines)' != 'true' and '$(TargetFrameworkIdentifier)' != '.NETPortable'">
<_FrameworkIdentifierForImplicitDefine>$(TargetFrameworkIdentifier.Replace('.', '').ToUpperInvariant())</_FrameworkIdentifierForImplicitDefine>
Expand Down
49 changes: 49 additions & 0 deletions test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildADesktopExe.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Xunit;
using FluentAssertions;
using static Microsoft.NET.TestFramework.Commands.MSBuildTest;
using Microsoft.NET.TestFramework.ProjectConstruction;

namespace Microsoft.NET.Build.Tests
{
Expand Down Expand Up @@ -110,6 +111,54 @@ public void It_respects_explicit_platform_target()
$"PlatformTarget=x64");
}

[Fact]
public void It_includes_default_framework_references()
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return;
}

var testProject = new TestProject()
{
Name = "DefaultReferences",
// TODO: Add net35 to the TargetFrameworks list once https://github.com/Microsoft/msbuild/issues/1333 is fixed
TargetFrameworks = "net40;net45;net461",
IsSdkProject = true,
IsExe = true
};

string sourceFile =
@"using System;

namespace DefaultReferences
{
public class TestClass
{
public static void Main(string [] args)
{
var uri = new System.Uri(""http://github.com/dotnet/corefx"");
var currentProcess = System.Diagnostics.Process.GetCurrentProcess();
}
}
}";
testProject.SourceFiles.Add("TestClass.cs", sourceFile);

var testAsset = _testAssetsManager.CreateTestProject(testProject)
.Restore("DefaultReferences");

var buildCommand = new BuildCommand(Stage0MSBuild, Path.Combine(testAsset.TestRoot, "DefaultReferences"));

buildCommand
.CaptureStdOut()
.Execute()
.Should()
.Pass()
.And
.NotHaveStdOutMatching("Could not resolve this reference", System.Text.RegularExpressions.RegexOptions.CultureInvariant | System.Text.RegularExpressions.RegexOptions.IgnoreCase);

}

[Fact]
public void It_generates_binding_redirects_if_needed()
{
Expand Down
6 changes: 6 additions & 0 deletions test/Microsoft.NET.Build.Tests/GivenThatWeWantToReferenceAProject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ public void It_checks_for_valid_references(string referencerTarget, bool referen
buildCommand = buildCommand.CaptureStdOut();
}

// Suppress ResolveAssemblyReference warning output due to https://github.com/Microsoft/msbuild/issues/1329
if (buildSucceeds && referencerProject.IsExe && referencerProject.ShortTargetFrameworkIdentifiers.Contains("net"))
{
buildCommand = buildCommand.CaptureStdOut();
}

var result = buildCommand.Execute();

if (buildSucceeds)
Expand Down
13 changes: 12 additions & 1 deletion test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPreserveCompilationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.NET.TestFramework.Commands;
using Xunit;
using static Microsoft.NET.TestFramework.Commands.MSBuildTest;
using System.Xml.Linq;

namespace Microsoft.NET.Publish.Tests
{
Expand All @@ -24,7 +25,17 @@ public void It_publishes_the_project_with_a_refs_folder_and_correct_deps_file()
{
var testAsset = _testAssetsManager
.CopyTestAsset("CompilationContext", "PreserveCompilationContext")
.WithSource();
.WithSource()
.WithProjectChanges(project =>
{
// Workaround for https://github.com/dotnet/sdk/issues/367

var ns = XNamespace.Get("http://schemas.microsoft.com/developer/msbuild/2003");
var propertyGroup = project.Root.Elements(ns + "PropertyGroup").FirstOrDefault();
propertyGroup.Should().NotBeNull();

propertyGroup.Add(new XElement(ns + "DisableImplicitFrameworkReferences", "true"));
});

testAsset.Restore("TestApp");
testAsset.Restore("TestLibrary");
Expand Down
7 changes: 7 additions & 0 deletions test/Microsoft.NET.TestFramework/Assertions/CommandResultAssertions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ public AndConstraint<CommandResultAssertions> HaveStdOutMatching(string pattern,
return new AndConstraint<CommandResultAssertions>(this);
}

public AndConstraint<CommandResultAssertions> NotHaveStdOutMatching(string pattern, RegexOptions options = RegexOptions.None)
{
Execute.Assertion.ForCondition(!Regex.Match(_commandResult.StdOut, pattern, options).Success)
.FailWith(AppendDiagnosticsTo($"The command output matched a pattern it should not have. Pattern: {pattern}{Environment.NewLine}"));
return new AndConstraint<CommandResultAssertions>(this);
}

public AndConstraint<CommandResultAssertions> HaveStdErr()
{
Execute.Assertion.ForCondition(!string.IsNullOrEmpty(_commandResult.StdErr))
Expand Down
50 changes: 31 additions & 19 deletions test/Microsoft.NET.TestFramework/ProjectConstruction/TestProject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class TestProject

public List<TestProject> ReferencedProjects { get; } = new List<TestProject>();

public Dictionary<string, string> SourceFiles { get; } = new Dictionary<string, string>();

private static string GetShortTargetFrameworkIdentifier(string targetFramework)
{
int identifierLength = 0;
Expand Down Expand Up @@ -212,12 +214,14 @@ internal void Create(TestAsset targetTestAsset, string testProjectsSourceFolder)
projectXml.Save(file);
}

string source;

if (this.IsExe)
if (SourceFiles.Count == 0)
{
source =
@"using System;
string source;

if (this.IsExe)
{
source =
@"using System;

class Program
{
Expand All @@ -226,19 +230,19 @@ static void Main(string[] args)
Console.WriteLine(""Hello World!"");
";

foreach (var dependency in this.ReferencedProjects)
{
source += $" Console.WriteLine({dependency.Name}.{dependency.Name}Class.Name);" + Environment.NewLine;
}
foreach (var dependency in this.ReferencedProjects)
{
source += $" Console.WriteLine({dependency.Name}.{dependency.Name}Class.Name);" + Environment.NewLine;
}

source +=
@" }
source +=
@" }
}";
}
else
{
source =
$@"using System;
}
else
{
source =
$@"using System;

namespace {this.Name}
{{
Expand All @@ -247,10 +251,18 @@ public class {this.Name}Class
public static string Name {{ get {{ return ""{this.Name}""; }} }}
}}
}}";
}
string sourcePath = Path.Combine(targetFolder, this.Name + ".cs");
}
string sourcePath = Path.Combine(targetFolder, this.Name + ".cs");

File.WriteAllText(sourcePath, source);
File.WriteAllText(sourcePath, source);
}
else
{
foreach (var kvp in SourceFiles)
{
File.WriteAllText(Path.Combine(targetFolder, kvp.Key), kvp.Value);
}
}
}

public override string ToString()
Expand Down