Skip to content

Commit

Permalink
Fix issue importing an SDK via Import element.
Browse files Browse the repository at this point in the history 
* This feature was regressed in dotnet#2002
* Update unit tests to verify functionality
* Update ProjectParse to parse values and construct an SdkReference object to be used by the evaluator.

Closes dotnet#2034
  • Loading branch information
@AndyGerlicher
AndyGerlicher committed May 1, 2017
1 parent c57e0bd commit eee8f6e
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 93 deletions.
2 changes: 2 additions & 0 deletions ref/net46/Microsoft.Build/Microsoft.Build.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ public partial class ProjectImportElement : Microsoft.Build.Construction.Project
{
internal ProjectImportElement() { }
public Microsoft.Build.Construction.ImplicitImportLocation ImplicitImportLocation { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public string MinimumVersion { get { throw null; } set { } }
public string Project { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation ProjectLocation { get { throw null; } }
public string Sdk { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } }
public string Version { get { throw null; } set { } }
protected override Microsoft.Build.Construction.ProjectElement CreateNewInstance(Microsoft.Build.Construction.ProjectRootElement owner) { throw null; }
}
[System.Diagnostics.DebuggerDisplayAttribute("#Imports={Count} Condition={Condition} Label={Label}")]
Expand Down
2 changes: 2 additions & 0 deletions ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ public partial class ProjectImportElement : Microsoft.Build.Construction.Project
{
internal ProjectImportElement() { }
public Microsoft.Build.Construction.ImplicitImportLocation ImplicitImportLocation { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public string MinimumVersion { get { throw null; } set { } }
public string Project { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation ProjectLocation { get { throw null; } }
public string Sdk { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } }
public string Version { get { throw null; } set { } }
protected override Microsoft.Build.Construction.ProjectElement CreateNewInstance(Microsoft.Build.Construction.ProjectRootElement owner) { throw null; }
}
[System.Diagnostics.DebuggerDisplayAttribute("#Imports={Count} Condition={Condition} Label={Label}")]
Expand Down
180 changes: 92 additions & 88 deletions src/Build.OM.UnitTests/Construction/ProjectSdkImplicitImport_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@ namespace Microsoft.Build.UnitTests.OM.Construction
/// </summary>
public class ProjectSdkImplicitImport_Tests : IDisposable
{
private const string ProjectTemplateSdkAsAttribute = @"
<Project Sdk=""{0}"">
{1}
</Project>";

private const string ProjectTemplateSdkAsElement = @"
<Project>
<Sdk Name=""{0}"" />
{1}
</Project>";

private const string ProjectTemplateSdkAsExplicitImport = @"
<Project>
<Import Project=""Sdk.props"" Sdk=""{0}"" />
{1}
<Import Project=""Sdk.targets"" Sdk=""{0}"" />
</Project>";

private const string SdkName = "MSBuildUnitTestSdk";
private readonly string _testSdkRoot;
private readonly string _testSdkDirectory;
Expand All @@ -37,29 +55,18 @@ public ProjectSdkImplicitImport_Tests()
}

[Theory]
[InlineData(@"
<Project Sdk=""{0}"">
<PropertyGroup>
<UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation>
</PropertyGroup>
</Project>
")]
[InlineData(@"
<Project>
<Sdk Name=""{0}"" />
<PropertyGroup>
<UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation>
</PropertyGroup>
</Project>
")]
public void SdkImportsAreInLogicalProject(string projectFormatString)
[InlineData(ProjectTemplateSdkAsAttribute, false)]
[InlineData(ProjectTemplateSdkAsElement, true)]
[InlineData(ProjectTemplateSdkAsExplicitImport, false)]
public void SdkImportsAreInLogicalProject(string projectFormatString, bool expectImportInLogicalProject)
{
string projectInnerContents = @"<PropertyGroup><UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation></PropertyGroup>";
File.WriteAllText(_sdkPropsPath, "<Project><PropertyGroup><InitialImportProperty>Hello</InitialImportProperty></PropertyGroup></Project>");
File.WriteAllText(_sdkTargetsPath, "<Project><PropertyGroup><FinalImportProperty>World</FinalImportProperty></PropertyGroup></Project>");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
string content = string.Format(projectFormatString, SdkName);
string content = string.Format(projectFormatString, SdkName, projectInnerContents);

ProjectRootElement projectRootElement = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));

Expand All @@ -68,42 +75,30 @@ public void SdkImportsAreInLogicalProject(string projectFormatString)
IList<ProjectElement> children = project.GetLogicalProject().ToList();

// <Sdk> style will have an extra ProjectElment.
var expected = projectFormatString.Contains("Sdk=") ? 6 : 7;
Assert.Equal(expected, children.Count);
Assert.Equal(expectImportInLogicalProject ? 7 : 6, children.Count);
}
}

[Theory]
[InlineData(@"
<Project Sdk=""{0}"">
<PropertyGroup>
<UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation>
</PropertyGroup>
</Project>
")]
[InlineData(@"
<Project>
<Sdk Name=""{0}"" />
<PropertyGroup>
<UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation>
</PropertyGroup>
</Project>
")]
public void SdkImportsAreInImportList(string projectFormatString)
[InlineData(ProjectTemplateSdkAsAttribute, false)]
[InlineData(ProjectTemplateSdkAsElement, false)]
[InlineData(ProjectTemplateSdkAsExplicitImport, true)]
public void SdkImportsAreInImportList(string projectFormatString, bool expectImportInLogicalProject)
{
string projectInnerContents = @"<PropertyGroup><UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation></PropertyGroup>";
File.WriteAllText(_sdkPropsPath, "<Project><PropertyGroup><InitialImportProperty>Hello</InitialImportProperty></PropertyGroup></Project>");
File.WriteAllText(_sdkTargetsPath, "<Project><PropertyGroup><FinalImportProperty>World</FinalImportProperty></PropertyGroup></Project>");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
string content = string.Format(projectFormatString, SdkName);
string content = string.Format(projectFormatString, SdkName, projectInnerContents);

ProjectRootElement projectRootElement = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));

var project = new Project(projectRootElement);

// The XML representation of the project should indicate there are no imports
Assert.Equal(0, projectRootElement.Imports.Count);
// The XML representation of the project should only indicate an import if they are not implicit.
Assert.Equal(expectImportInLogicalProject ? 2 : 0, projectRootElement.Imports.Count);

// The project representation should have imports
Assert.Equal(2, project.Imports.Count);
Expand All @@ -128,14 +123,24 @@ public void SdkImportsAreInImportList(string projectFormatString)
[Theory]
[InlineData(@"
<Project Sdk=""{0};{1};{2}"">
</Project >")]
</Project >", false)]
[InlineData(@"
<Project>
<Sdk Name=""{0}"" />
<Sdk Name=""{1}"" />
<Sdk Name=""{2}"" />
</Project>")]
public void SdkSupportsMultiple(string projectFormatString)
</Project>", false)]
[InlineData(@"
<Project>
<Import Project=""Sdk.props"" Sdk=""{0}"" />
<Import Project=""Sdk.props"" Sdk=""{1}"" />
<Import Project=""Sdk.props"" Sdk=""{2}"" />
<Import Project=""Sdk.targets"" Sdk=""{0}"" />
<Import Project=""Sdk.targets"" Sdk=""{1}"" />
<Import Project=""Sdk.targets"" Sdk=""{2}"" />
</Project>", true)]
public void SdkSupportsMultiple(string projectFormatString, bool expectImportInLogicalProject)
{
IList<string> sdkNames = new List<string>
{
Expand All @@ -161,7 +166,7 @@ public void SdkSupportsMultiple(string projectFormatString)
Project project = new Project(projectRootElement);

// The XML representation of the project should indicate there are no imports
Assert.Equal(0, projectRootElement.Imports.Count);
Assert.Equal(expectImportInLogicalProject ? 6 : 0, projectRootElement.Imports.Count);

// The project representation should have twice as many imports as SDKs
Assert.Equal(sdkNames.Count * 2, project.Imports.Count);
Expand All @@ -173,21 +178,12 @@ public void SdkSupportsMultiple(string projectFormatString)
}

[Theory]
[InlineData(@"<Project Sdk=""{0}"" ToolsVersion=""15.0"">
")]
[InlineData(@"<Project ToolsVersion=""15.0"">
<Sdk Name=""{0}"" />
")]
public void ProjectWithSdkImportsIsCloneable(string projectFileFirstLineFormat)
[InlineData(ProjectTemplateSdkAsAttribute)]
[InlineData(ProjectTemplateSdkAsElement)]
[InlineData(ProjectTemplateSdkAsExplicitImport)]
public void ProjectWithSdkImportsIsCloneable(string projectFormatString)
{
File.WriteAllText(_sdkPropsPath, "<Project />");
File.WriteAllText(_sdkTargetsPath, "<Project />");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
// Based on the new-console-project CLI template (but not matching exactly
// should not be a deal-breaker).
string content = $@"{string.Format(projectFileFirstLineFormat, SdkName)}
string projectInnerContents = @"
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp1.0</TargetFramework>
Expand All @@ -200,32 +196,28 @@ public void ProjectWithSdkImportsIsCloneable(string projectFileFirstLineFormat)
<ItemGroup>
<PackageReference Include=""Microsoft.NETCore.App"" Version=""1.0.1"" />
</ItemGroup>
</Project>";
</ItemGroup>";
File.WriteAllText(_sdkPropsPath, " < Project />");
File.WriteAllText(_sdkTargetsPath, "<Project />");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
// Based on the new-console-project CLI template (but not matching exactly
// should not be a deal-breaker).
string content = string.Format(projectFormatString, SdkName, projectInnerContents);
ProjectRootElement project = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));

project.DeepClone();
}
}

[Theory]
[InlineData(@"<Project Sdk=""{0}"" ToolsVersion=""15.0"">
")]
[InlineData(@"<Project ToolsVersion=""15.0"">
<Sdk Name=""{0}"" />
")]
public void ProjectWithSdkImportsIsRemoveable(string projectFileFirstLineFormat)
[InlineData(ProjectTemplateSdkAsAttribute)]
[InlineData(ProjectTemplateSdkAsElement)]
[InlineData(ProjectTemplateSdkAsExplicitImport)]
public void ProjectWithSdkImportsIsRemoveable(string projectFormatString)
{
File.WriteAllText(_sdkPropsPath, "<Project />");
File.WriteAllText(_sdkTargetsPath, "<Project />");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
// Based on the new-console-project CLI template (but not matching exactly
// should not be a deal-breaker).
string content = $@"{string.Format(projectFileFirstLineFormat, SdkName)}
string projectInnerContents = @"
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp1.0</TargetFramework>
Expand All @@ -238,10 +230,15 @@ public void ProjectWithSdkImportsIsRemoveable(string projectFileFirstLineFormat)
<ItemGroup>
<PackageReference Include=""Microsoft.NETCore.App"" Version=""1.0.1"" />
</ItemGroup>
</Project>";
</ItemGroup>";
File.WriteAllText(_sdkPropsPath, " < Project />");
File.WriteAllText(_sdkTargetsPath, "<Project />");

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
// Based on the new-console-project CLI template (but not matching exactly
// should not be a deal-breaker).
string content = string.Format(projectFormatString, SdkName, projectInnerContents);
ProjectRootElement project = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));
ProjectRootElement clone = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));

Expand Down Expand Up @@ -280,21 +277,28 @@ public void ProjectWithInvalidSdkName()
/// <summary>
/// Verifies that an empty SDK attribute works and nothing is imported.
/// </summary>
[Fact]
public void ProjectWithEmptySdkName()
[Theory]
[InlineData(ProjectTemplateSdkAsAttribute, false)]
[InlineData(ProjectTemplateSdkAsElement, true)]
[InlineData(ProjectTemplateSdkAsExplicitImport, true)]
public void ProjectWithEmptySdkName(string projectFormatString, bool throwsOnEvaluate)
{
string projectInnerContents =
@"<PropertyGroup><UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation></PropertyGroup>";

using (new Helpers.TemporaryEnvironment("MSBuildSDKsPath", _testSdkRoot))
{
string content = @"
<Project Sdk="""">
<PropertyGroup>
<UsedToTestIfImplicitImportsAreInTheCorrectLocation>null</UsedToTestIfImplicitImportsAreInTheCorrectLocation>
</PropertyGroup>
</Project>";

Project project = new Project(ProjectRootElement.Create(XmlReader.Create(new StringReader(content))));

Assert.Equal(0, project.Imports.Count);
string content = string.Format(projectFormatString, string.Empty, projectInnerContents);
if (throwsOnEvaluate)
{
Assert.Throws<InvalidProjectFileException>(
() => new Project(ProjectRootElement.Create(XmlReader.Create(new StringReader(content)))));
}
else
{
var project = new Project(ProjectRootElement.Create(XmlReader.Create(new StringReader(content))));
Assert.Equal(0, project.Imports.Count);
}
}
}

Expand Down
50 changes: 48 additions & 2 deletions src/Build/Construction/ProjectImportElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ public class ProjectImportElement : ProjectElement
/// <summary>
/// Initialize a parented ProjectImportElement
/// </summary>
internal ProjectImportElement(XmlElementWithLocation xmlElement, ProjectElementContainer parent, ProjectRootElement containingProject)
internal ProjectImportElement(XmlElementWithLocation xmlElement, ProjectElementContainer parent, ProjectRootElement containingProject, SdkReference sdkReference = null)
: base(xmlElement, parent, containingProject)
{
ErrorUtilities.VerifyThrowArgumentNull(parent, "parent");
ParsedSdkReference = sdkReference;
}

/// <summary>
Expand Down Expand Up @@ -75,12 +76,40 @@ public string Sdk
set
{
ErrorUtilities.VerifyThrowArgumentLength(value, XMakeAttributes.sdk);

if (!CheckUpdatedSdk()) return;
ProjectXmlUtilities.SetOrRemoveAttribute(XmlElement, XMakeAttributes.sdk, value);
MarkDirty("Set Import Sdk {0}", value);
}
}

/// <summary>
/// Gets or sets the version associated with this SDK import
/// </summary>
public string Version
{
get { return ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdkVersion); }
set
{
if (!CheckUpdatedSdk()) return;
ProjectXmlUtilities.SetOrRemoveAttribute(XmlElement, XMakeAttributes.sdkVersion, value);
MarkDirty("Set Import Version {0}", value);
}
}

/// <summary>
/// Gets or sets the minimum SDK version required by this import.
/// </summary>
public string MinimumVersion
{
get { return ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdkMinimumVersion); }
set
{
if (!CheckUpdatedSdk()) return;
ProjectXmlUtilities.SetOrRemoveAttribute(XmlElement, XMakeAttributes.sdkMinimumVersion, value);
MarkDirty("Set Import Minimum Version {0}", value);
}
}

/// <summary>
/// Location of the Sdk attribute
/// </summary>
Expand Down Expand Up @@ -139,5 +168,22 @@ protected override ProjectElement CreateNewInstance(ProjectRootElement owner)
{
return owner.CreateImportElement(this.Project);
}

private bool CheckUpdatedSdk()
{

SdkReference sdk = new SdkReference(
ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdk, true),
ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdkVersion, true),
ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdkMinimumVersion, true));

if (sdk.Equals(ParsedSdkReference))
{
return false;
}

ParsedSdkReference = sdk;
return true;
}
}
}
Loading

0 comments on commit eee8f6e

Please sign in to comment.