Skip to content
This repository has been archived by the owner on May 17, 2024. It is now read-only.

Remove known troublesome imports and update tests #341

Merged
merged 4 commits into from
Dec 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/MSBuild.Abstractions/BaselineProject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private static string AdjustTargetTFM(ProjectStyle projectStyle, ProjectOutputTy
return MSBuildFacts.Net5Windows;
}

if (projectStyle is not ProjectStyle.MSTest && outputType is ProjectOutputType.Library)
if (projectStyle is not ProjectStyle.MSTest && projectStyle is not ProjectStyle.Web && outputType is ProjectOutputType.Library)
{
return MSBuildFacts.NetStandard20;
}
Expand Down
40 changes: 35 additions & 5 deletions src/MSBuild.Abstractions/MSBuildConversionWorkspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ public MSBuildConversionWorkspace(ImmutableArray<string> paths, bool noBackup, s
continue;
}

// This is a hack, but the only way to handle this is to re-architect try-convert along these lines:
// 1. Launch a .NET Framework process using the VS-deployed MSBuild to evaluate a project
// 2. Serialize the evaluation model
// 3. Use the .NET Core process to load MSBuild, but this time from .NET SDK
// 4. Deserialize the evaluation model
// 5. Do conversions
RemoveTargetsNotLoadableByNETSDKMSBuild(path);

var root = new MSBuildProjectRootElement(ProjectRootElement.Open(path, collection, preserveFormatting: true));
if (IsSupportedProjectType(root, forceWeb))
{
Expand Down Expand Up @@ -89,6 +97,25 @@ public ImmutableDictionary<string, ImmutableDictionary<string, string>> Determin
return builder.ToImmutable();
}

private void RemoveTargetsNotLoadableByNETSDKMSBuild(string path)
{
var projectFile = File.ReadAllText(path);
if (projectFile is { Length:>0 })
{
var replacement =
projectFile
// Legacy web project specify these two targets paths. They aren't loadable by the .NET SDK msbuild process, so we just remove them.
// They aren't actually useful as an end result when converting web projects. When people conver to .NET Core they will just use the Web SDK attribute.
.Replace("<Import Project=\"$(VSToolsPath)\\WebApplications\\Microsoft.WebApplication.targets\" Condition=\"'$(VSToolsPath)' != ''\" />", "")
.Replace("<Import Project=\"$(MSBuildExtensionsPath32)\\Microsoft\\VisualStudio\\v10.0\\WebApplications\\Microsoft.WebApplication.targets\" Condition=\"false\" />", "")

// Legacy F# projects specify this import. It's not loadable by the .NET SDK MSBuild, and .NET Core-based F# projects don't use it. So we just remove it.
.Replace("<Import Project=\"$(FSharpTargetsPath)\" />", "");

File.WriteAllText(path, replacement);
}
}

/// <summary>
/// Clear out the project's construction model and add a simple SDK-based project to get a baseline.
/// We need to use the same name as the original csproj and same path so that all the default that derive
Expand All @@ -97,7 +124,7 @@ public ImmutableDictionary<string, ImmutableDictionary<string, string>> Determin
private bool TryCreateSdkBaselineProject(string projectFilePath, IProject project, IProjectRootElement root, ImmutableDictionary<string, ImmutableDictionary<string, string>> configurations, string tfm, bool keepCurrentTFMs, [NotNullWhen(true)] out BaselineProject? baselineProject)
{
var projectStyle = GetProjectStyle(root);
var outputType = GetProjectOutputType(root);
var outputType = GetProjectOutputType(root, projectStyle);
var rootElement = ProjectRootElement.Open(projectFilePath);

rootElement.RemoveAllChildren();
Expand Down Expand Up @@ -194,12 +221,15 @@ private bool IsSupportedOutputType(ProjectOutputType type) =>
_ => false
};

private ProjectOutputType GetProjectOutputType(IProjectRootElement root)
private ProjectOutputType GetProjectOutputType(IProjectRootElement root) =>
GetProjectOutputType(root, GetProjectStyle(root));

private ProjectOutputType GetProjectOutputType(IProjectRootElement root, ProjectStyle projectStyle)
{
if (root.Imports.Any(i => i.Project.Contains(WebFacts.WebApplicationTargets, StringComparison.OrdinalIgnoreCase)))
if (projectStyle == ProjectStyle.Web)
{
// ASP.NET Core apps use an EXE output type even though ASP.NET apps use Library
// Note that this specifically looks for WebApplicationTargets (rather than a System.Web reference) since
// ASP.NET Core apps use an EXE output type even though legacy ASP.NET apps use Library
// Note that this specifically checks the project style only (rather than a System.Web reference) since
// ASP.NET libraries may reference System.Web and should still use a Library output types. Only ASP.NET
// apps should convert with Exe output type.
return ProjectOutputType.Exe;
Expand Down
12 changes: 5 additions & 7 deletions src/MSBuild.Abstractions/MSBuildHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,11 @@ public static bool ConditionToDimensionValues(string condition, out ImmutableDic
/// <summary>
/// Given a TFM string, determines if that TFM has an explicit System.ValueTuple reference.
/// </summary>
public static bool FrameworkHasAValueTuple(string tfm)
{
return !tfm.ContainsIgnoreCase(MSBuildFacts.NetstandardPrelude)
&& !tfm.ContainsIgnoreCase(MSBuildFacts.NetcoreappPrelude)
&& (tfm.StartsWith("net", StringComparison.OrdinalIgnoreCase)
&& tfm.StartsWith(MSBuildFacts.LowestFrameworkVersionWithSystemValueTuple));
}
public static bool FrameworkHasAValueTuple(string tfm) =>
tfm.StartsWith(MSBuildFacts.NetstandardPrelude, StringComparison.OrdinalIgnoreCase)
|| tfm.StartsWith(MSBuildFacts.NetcoreappPrelude, StringComparison.OrdinalIgnoreCase)
|| tfm.StartsWith(MSBuildFacts.Net5, StringComparison.OrdinalIgnoreCase)
|| tfm.StartsWith(MSBuildFacts.LowestFrameworkVersionWithSystemValueTuple, StringComparison.OrdinalIgnoreCase);

/// <summary>
/// Gets all Reference items from a given item group.
Expand Down
4 changes: 3 additions & 1 deletion src/MSBuild.Abstractions/ProjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ public static string GetTargetFramework(this IProject project)
var tfi = project.GetPropertyValue(MSBuildFacts.LegacyTargetFrameworkPropertyNodeName);
if (string.IsNullOrWhiteSpace(tfi))
{
throw new InvalidOperationException($"{MSBuildFacts.LegacyTargetFrameworkPropertyNodeName} is not set!");
// For some befuddling reason, legacy F# projects don't actually have a TFI.
// We just assume it's .NET Framework though, because like, that's just what it's gonna be anyways.
tfi = ".NETFramework";
}

var tfv = project.GetPropertyValue(MSBuildFacts.LegacyTargetFrameworkVersionNodeName);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net472</TargetFramework>
<TargetFramework>netcoreapp3.1</TargetFramework>
<OutputType>Exe</OutputType>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
namespace SmokeTests.LegacyFSharpConsole.AssemblyInfo

open System.Reflection
open System.Runtime.CompilerServices
open System.Runtime.InteropServices

// General Information about an assembly is controlled through the following
// set of attributes. Change these attribute values to modify the information
// associated with an assembly.
[<assembly: AssemblyTitle("SmokeTests.LegacyFSharpConsole")>]
[<assembly: AssemblyDescription("")>]
[<assembly: AssemblyConfiguration("")>]
[<assembly: AssemblyCompany("")>]
[<assembly: AssemblyProduct("SmokeTests.LegacyFSharpConsole")>]
[<assembly: AssemblyCopyright("Copyright © 2019")>]
[<assembly: AssemblyTrademark("")>]
[<assembly: AssemblyCulture("")>]

// Setting ComVisible to false makes the types in this assembly not visible
// to COM components. If you need to access a type in this assembly from
// COM, set the ComVisible attribute to true on that type.
[<assembly: ComVisible(false)>]

// The following GUID is for the ID of the typelib if this project is exposed to COM
[<assembly: Guid("df33a7b9-e0dd-4e09-b9d9-c6d0b7d5a648")>]

// Version information for an assembly consists of the following four values:
//
// Major Version
// Minor Version
// Build Number
// Revision
//
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [<assembly: AssemblyVersion("1.0.*")>]
[<assembly: AssemblyVersion("1.0.0.0")>]
[<assembly: AssemblyFileVersion("1.0.0.0")>]

do
()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Learn more about F# at http://fsharp.org

open System

[<EntryPoint>]
let main argv =
printfn "Hello World from F#!"
0 // return an integer exit code
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net472</TargetFramework>
<TargetFramework>net5.0</TargetFramework>
<OutputType>Exe</OutputType>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>
Expand Down
10 changes: 5 additions & 5 deletions tests/end-to-end/Smoke.Tests/BasicConversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ public BasicSmokeTests(SolutionPathFixture solutionPathFixture, MSBuildFixture m
solutionPathFixture.SetCurrentDirectory();
}

[Fact(Skip = "Legacy F# support is not installed on any build machines")]
[Fact]
public void ConvertsLegacyFSharpConsoleToNetCoreApp31()
{
var projectToConvertPath = GetFSharpProjectPath("SmokeTests.LegacyFSharpConsole");
var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleBaseline");
var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleCoreBaseline");
AssertConversionWorks(projectToConvertPath, projectBaselinePath, "netcoreapp3.1");
}

[Fact(Skip = "Legacy F# support is not installed on any build machines")]
[Fact]
public void ConvertsLegacyFSharpConsoleToNet50()
{
var projectToConvertPath = GetFSharpProjectPath("SmokeTests.LegacyFSharpConsole");
var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleBaseline");
AssertConversionWorks(projectToConvertPath, projectBaselinePath, "netcoreapp3.1");
var projectBaselinePath = GetFSharpProjectPath("SmokeTests.FSharpConsoleNet5Baseline");
AssertConversionWorks(projectToConvertPath, projectBaselinePath, "net5.0");
}

[Fact]
Expand Down
2 changes: 0 additions & 2 deletions tests/unit/ProjectConversion.Tests/ProjectExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ public void GetTargetFramework(string targetFramework, string targetFrameworkIde
}

[Theory]
[InlineData(null, null, "v4.6")]
[InlineData(null, ".NETCoreApp", null)]
[InlineData(null, "Unknown", null)]
public void GetTargetFramework_Throws(string targetFramework, string targetFrameworkIdentifier, string targetFrameworkVersion)
{
var properties = new Dictionary<string, string>();
Expand Down