Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix bug when multi-TFM project reference is outside OmniSharp's target path #1195

Merged
merged 1 commit into from
May 21, 2018
Merged

Fix bug when multi-TFM project reference is outside OmniSharp's target path #1195

Show file tree
Hide file tree
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 build.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
"TwoProjectsWithSolution",
"ProjectWithGeneratedFile",
"CSharpAndFSharp",
"ProjectWithMismatchedFileName"
"ProjectWithMismatchedFileName",
"ProjectWithMultiTFMLib"
],
"LegacyTestAssets": [
"LegacyNUnitTestProject",
Expand Down
11 changes: 6 additions & 5 deletions src/OmniSharp.MSBuild/ProjectLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ private MSB.Evaluation.Project EvaluateProjectFileCore(string filePath)

toolsVersion = GetLegalToolsetVersion(toolsVersion, projectCollection.Toolsets);

return projectCollection.LoadProject(filePath, toolsVersion);
var project = projectCollection.LoadProject(filePath, toolsVersion);

SetTargetFrameworkIfNeeded(project);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every project goes through this code path, where we first evaluate the project with MSBuild. At this point, we need to set the <TargetFramework/> property if there are multiple <TargetFrameworks/> to ensure that the project evaluates with a TFM. Otherwise, several MSBuild properties (like TargetFrameworkMoniker) are missing.


return project;
}

private static void SetTargetFrameworkIfNeeded(MSB.Evaluation.Project evaluatedProject)
Expand All @@ -127,10 +131,7 @@ private static void SetTargetFrameworkIfNeeded(MSB.Evaluation.Project evaluatedP
// do better and potentially allow OmniSharp hosts to select a target framework.
targetFramework = targetFrameworks[0];
Copy link
Member

Choose a reason for hiding this comment

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

how could we allow this to be overrideable in the future?
allow the user to set preferred TFM via MSBuildOptions in the config?

Copy link
Member

Choose a reason for hiding this comment

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

We need create a singleton service or something that we can toggle the tfm for a given project, and then also enhance services to also understand that...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we need the same thing for setting the active solution/project configuration as well.

evaluatedProject.SetProperty(PropertyNames.TargetFramework, targetFramework);
}
else if (!string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length == 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check is not needed here. We used to pass targetFrameworks as an out parameter but that was removed long ago. So, this is essentially unnecessary code.

{
targetFrameworks = ImmutableArray.Create(targetFramework);
evaluatedProject.ReevaluateIfNecessary();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, we tell the project to re-evaluate itself if it's dirty, which will be the case because we set the <TargetFramework> property.

}
}

Expand Down
12 changes: 12 additions & 0 deletions test-assets/test-projects/ProjectWithMultiTFMLib/App/App.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">

<ItemGroup>
<ProjectReference Include="..\Lib\Lib.csproj" />
</ItemGroup>

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp1.1</TargetFramework>
</PropertyGroup>

</Project>
12 changes: 12 additions & 0 deletions test-assets/test-projects/ProjectWithMultiTFMLib/App/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;

namespace App
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
}
}
}
8 changes: 8 additions & 0 deletions test-assets/test-projects/ProjectWithMultiTFMLib/Lib/Class1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System;

namespace Lib
{
public class Class1
{
}
}
7 changes: 7 additions & 0 deletions test-assets/test-projects/ProjectWithMultiTFMLib/Lib/Lib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netstandard1.3;netstandard2.0</TargetFrameworks>
</PropertyGroup>

</Project>
48 changes: 48 additions & 0 deletions test-assets/test-projects/ProjectWithMultiTFMLib/ProjectWithMultiTFMLib.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.26124.0
MinimumVisualStudioVersion = 15.0.26124.0
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "App", "App\App.csproj", "{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Lib", "Lib\Lib.csproj", "{CE41561B-5D13-4688-8686-EEFF744BE8B5}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Debug|x64 = Debug|x64
Debug|x86 = Debug|x86
Release|Any CPU = Release|Any CPU
Release|x64 = Release|x64
Release|x86 = Release|x86
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|x64.ActiveCfg = Debug|x64
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|x64.Build.0 = Debug|x64
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|x86.ActiveCfg = Debug|x86
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Debug|x86.Build.0 = Debug|x86
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|Any CPU.Build.0 = Release|Any CPU
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|x64.ActiveCfg = Release|x64
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|x64.Build.0 = Release|x64
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|x86.ActiveCfg = Release|x86
{632DFE45-B56E-4158-8F27-45E2BA0BAFCF}.Release|x86.Build.0 = Release|x86
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|Any CPU.Build.0 = Debug|Any CPU
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|x64.ActiveCfg = Debug|x64
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|x64.Build.0 = Debug|x64
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|x86.ActiveCfg = Debug|x86
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Debug|x86.Build.0 = Debug|x86
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|Any CPU.ActiveCfg = Release|Any CPU
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|Any CPU.Build.0 = Release|Any CPU
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|x64.ActiveCfg = Release|x64
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|x64.Build.0 = Release|x64
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|x86.ActiveCfg = Release|x86
{CE41561B-5D13-4688-8686-EEFF744BE8B5}.Release|x86.Build.0 = Release|x86
EndGlobalSection
EndGlobal
21 changes: 21 additions & 0 deletions tests/OmniSharp.MSBuild.Tests/WorkspaceInformationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,27 @@ public async Task TestProjectWithReferencedProjectOutsideOfOmniSharp()
}
}

[Fact]
public async Task TestProjectWithMultiTFMReferencedProjectOutsideOfOmniSharp()
{
using (var testProject = await TestAssets.Instance.GetTestProjectAsync("ProjectWithMultiTFMLib"))
using (var host = CreateOmniSharpHost(Path.Combine(testProject.Directory, "App")))
{
var workspaceInfo = await GetWorkspaceInfoAsync(host);

Assert.NotNull(workspaceInfo.Projects);
Assert.Equal(2, workspaceInfo.Projects.Count);

var project1 = workspaceInfo.Projects[0];
Assert.Equal("App.csproj", Path.GetFileName(project1.Path));

var project2 = workspaceInfo.Projects[1];
Assert.Equal("Lib.csproj", Path.GetFileName(project2.Path));
Assert.Equal(".NETStandard,Version=v1.3", project2.TargetFramework);
Assert.Equal(2, project2.TargetFrameworks.Count);
}
}

[ConditionalFact(typeof(WindowsOnly))]
public async Task AntlrGeneratedFiles()
{
Expand Down