Skip to content

Commit

Permalink
Implementation of implicit imports during evaluation instead of durin…
Browse files Browse the repository at this point in the history 
…g parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes dotnet#1447
  • Loading branch information
@jeffkl
jeffkl committed Dec 16, 2016
1 parent 39c4880 commit 697c31f
Show file tree
Hide file tree
Showing 12 changed files with 230 additions and 154 deletions.
4 changes: 3 additions & 1 deletion ref/net46/Microsoft.Build/Microsoft.Build.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ internal ProjectElement() { }
public virtual string Condition { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public virtual Microsoft.Build.Construction.ElementLocation ConditionLocation { get { throw null; } }
public Microsoft.Build.Construction.ProjectRootElement ContainingProject { get { throw null; } }
public bool IsImplicit { get { throw null; } }
public string Label { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public Microsoft.Build.Construction.ElementLocation LabelLocation { get { throw null; } }
public Microsoft.Build.Construction.ElementLocation Location { get { throw null; } }
Expand Down Expand Up @@ -86,6 +85,7 @@ public partial class ProjectImportElement : Microsoft.Build.Construction.Project
internal ProjectImportElement() { }
public string Project { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation ProjectLocation { get { throw null; } }
public string Sdk { 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 Expand Up @@ -253,6 +253,8 @@ internal ProjectRootElement() { }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectPropertyGroupElement> PropertyGroups { get { throw null; } }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectPropertyGroupElement> PropertyGroupsReversed { get { throw null; } }
public string RawXml { get { throw null; } }
public string Sdk { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectTargetElement> Targets { get { throw null; } }
public System.DateTime TimeLastChanged { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } }
public string ToolsVersion { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
Expand Down
4 changes: 3 additions & 1 deletion ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ internal ProjectElement() { }
public virtual string Condition { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public virtual Microsoft.Build.Construction.ElementLocation ConditionLocation { get { throw null; } }
public Microsoft.Build.Construction.ProjectRootElement ContainingProject { get { throw null; } }
public bool IsImplicit { get { throw null; } }
public string Label { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public Microsoft.Build.Construction.ElementLocation LabelLocation { get { throw null; } }
public Microsoft.Build.Construction.ElementLocation Location { get { throw null; } }
Expand Down Expand Up @@ -86,6 +85,7 @@ public partial class ProjectImportElement : Microsoft.Build.Construction.Project
internal ProjectImportElement() { }
public string Project { get { throw null; } set { } }
public Microsoft.Build.Construction.ElementLocation ProjectLocation { get { throw null; } }
public string Sdk { 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 Expand Up @@ -253,6 +253,8 @@ internal ProjectRootElement() { }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectPropertyGroupElement> PropertyGroups { get { throw null; } }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectPropertyGroupElement> PropertyGroupsReversed { get { throw null; } }
public string RawXml { get { throw null; } }
public string Sdk { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } }
public System.Collections.Generic.ICollection<Microsoft.Build.Construction.ProjectTargetElement> Targets { get { throw null; } }
public System.DateTime TimeLastChanged { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } }
public string ToolsVersion { [System.Diagnostics.DebuggerStepThroughAttribute]get { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute]set { } }
Expand Down
1 change: 0 additions & 1 deletion src/Shared/XMakeAttributes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ internal static class XMakeAttributes
internal const string taskName = "TaskName";
internal const string continueOnError = "ContinueOnError";
internal const string project = "Project";
internal const string @implicit = "_Implicit";
internal const string taskParameter = "TaskParameter";
internal const string itemName = "ItemName";
internal const string propertyName = "PropertyName";
Expand Down
5 changes: 0 additions & 5 deletions src/XMakeBuildEngine/Construction/ProjectElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,6 @@ public ElementLocation Location
get { return XmlElement.Location; }
}

/// <summary>
/// Gets the Implicit state of the element: true if the element was not in the read XML.
/// </summary>
public bool IsImplicit => XmlElement.HasAttribute(XMakeAttributes.@implicit);

/// <summary>
/// Gets the name of the associated element.
/// Useful for display in some circumstances.
Expand Down
17 changes: 2 additions & 15 deletions src/XMakeBuildEngine/Construction/ProjectElementContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,11 +455,6 @@ internal void UpdateElementValue(ProjectElement child)
/// </remarks>
internal void AddToXml(ProjectElement child)
{
if (child.IsImplicit)
{
return;
}

if (child.ExpressedAsAttribute)
{
// todo children represented as attributes need to be placed in order too
Expand All @@ -485,7 +480,7 @@ internal void AddToXml(ProjectElement child)
// If none is found, then the node being added is inserted as the only node of its kind

ProjectElement referenceSibling;
Predicate<ProjectElement> siblingIsExplicitElement = _ => _.ExpressedAsAttribute == false && _.IsImplicit == false;
Predicate<ProjectElement> siblingIsExplicitElement = _ => _.ExpressedAsAttribute == false;

if (TrySearchLeftSiblings(child.PreviousSibling, siblingIsExplicitElement, out referenceSibling))
{
Expand Down Expand Up @@ -567,11 +562,6 @@ private string GetElementIndentation(XmlElementWithLocation xmlElement)

internal void RemoveFromXml(ProjectElement child)
{
if (child.IsImplicit)
{
return;
}

if (child.ExpressedAsAttribute)
{
XmlElement.RemoveAttribute(child.XmlElement.Name);
Expand Down Expand Up @@ -630,10 +620,7 @@ private void AddInitialChild(ProjectElement child)

_count++;

if (!child.IsImplicit)
{
MarkDirty("Add child element named '{0}'", child.ElementName);
}
MarkDirty("Add child element named '{0}'", child.ElementName);
}

/// <summary>
Expand Down
44 changes: 44 additions & 0 deletions src/XMakeBuildEngine/Construction/ProjectImportElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,36 @@ public string Project
}
}

/// <summary>
/// Gets or sets the SDK that contains the import.
/// </summary>
public string Sdk
{
get
{
return
FileUtilities.FixFilePath(ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdk));
}

set
{
ErrorUtilities.VerifyThrowArgumentLength(value, XMakeAttributes.sdk);

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

/// <summary>
/// Location of the project attribute
/// </summary>
public ElementLocation ProjectLocation => XmlElement.GetAttributeLocation(XMakeAttributes.project);

/// <summary>
/// Gets or sets the <see cref="ImplicitImportLocation"/> of the import.
/// </summary>
internal ImplicitImportLocation ImplicitImportLocation { get; set; } = ImplicitImportLocation.None;

/// <summary>
/// Creates an unparented ProjectImportElement, wrapping an unparented XmlElement.
/// Validates the project value.
Expand Down Expand Up @@ -92,4 +117,23 @@ protected override ProjectElement CreateNewInstance(ProjectRootElement owner)
return owner.CreateImportElement(this.Project);
}
}

/// <summary>
/// Represents the location of an implicit import.
/// </summary>
internal enum ImplicitImportLocation
{
/// <summary>
/// The import is not implicit.
/// </summary>
None,
/// <summary>
/// The import should be at the top.
/// </summary>
Top,
/// <summary>
/// The import should be at the bottom.
/// </summary>
Bottom
}
}
70 changes: 33 additions & 37 deletions src/XMakeBuildEngine/Construction/ProjectRootElement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,6 @@ public class ProjectRootElement : ProjectElementContainer
/// </summary>
private BuildEventContext _buildEventContext;

/// <summary>
/// Xpath expression that will find any element with the implicit attribute
/// </summary>
private static readonly string ImplicitAttributeXpath = $"//*[@{XMakeAttributes.@implicit}]";

/// <summary>
/// Initialize a ProjectRootElement instance from a XmlReader.
/// May throw InvalidProjectFileException.
Expand Down Expand Up @@ -654,6 +649,25 @@ public string InitialTargets
}
}

/// <summary>
///
/// </summary>
public string Sdk
{
[DebuggerStepThrough]
get
{
return ProjectXmlUtilities.GetAttributeValue(XmlElement, XMakeAttributes.sdk);
}

[DebuggerStepThrough]
set
{
ProjectXmlUtilities.SetOrRemoveAttribute(XmlElement, XMakeAttributes.sdk, value);
MarkDirty("Set project Sdk to '{0}'", value);
}
}

/// <summary>
/// Gets or sets the value of TreatAsLocalProperty. If there is no tag, returns empty string.
/// If the value being set is null or empty, removes the attribute.
Expand Down Expand Up @@ -710,10 +724,8 @@ public string RawXml
{
using (ProjectWriter projectWriter = new ProjectWriter(stringWriter))
{
var xmlWithNoImplicits = RemoveImplicits();

projectWriter.Initialize(xmlWithNoImplicits);
xmlWithNoImplicits.Save(projectWriter);
projectWriter.Initialize(XmlDocument);
XmlDocument.Save(projectWriter);
}

return stringWriter.ToString();
Expand Down Expand Up @@ -848,6 +860,14 @@ public ElementLocation InitialTargetsLocation
get { return XmlElement.GetAttributeLocation(XMakeAttributes.initialTargets); }
}

/// <summary>
///
/// </summary>
public ElementLocation SdkLocation
{
get { return XmlElement.GetAttributeLocation(XMakeAttributes.sdk); }
}

/// <summary>
/// Location of the TreatAsLocalProperty attribute, if any
/// </summary>
Expand Down Expand Up @@ -1752,10 +1772,8 @@ public void Save(Encoding saveEncoding)
{
using (ProjectWriter projectWriter = new ProjectWriter(_projectFileLocation.File, saveEncoding))
{
var xmlWithNoImplicits = RemoveImplicits();

projectWriter.Initialize(xmlWithNoImplicits);
xmlWithNoImplicits.Save(projectWriter);
projectWriter.Initialize(XmlDocument);
XmlDocument.Save(projectWriter);
}

_encoding = saveEncoding;
Expand Down Expand Up @@ -1784,26 +1802,6 @@ public void Save(Encoding saveEncoding)
#endif
}

private XmlDocument RemoveImplicits()
{
if (XmlDocument.SelectSingleNode(ImplicitAttributeXpath) == null)
{
return XmlDocument;
}

var xmlWithNoImplicits = (XmlDocument) XmlDocument.CloneNode(deep: true);

var implicitElements =
xmlWithNoImplicits.SelectNodes(ImplicitAttributeXpath);

foreach (XmlNode implicitElement in implicitElements)
{
implicitElement.ParentNode.RemoveChild(implicitElement);
}

return xmlWithNoImplicits;
}

/// <summary>
/// Save the project to the file system, if dirty or the path is different.
/// Creates any necessary directories.
Expand Down Expand Up @@ -1837,10 +1835,8 @@ public void Save(TextWriter writer)
{
using (ProjectWriter projectWriter = new ProjectWriter(writer))
{
var xmlWithNoImplicits = RemoveImplicits();

projectWriter.Initialize(xmlWithNoImplicits);
xmlWithNoImplicits.Save(projectWriter);
projectWriter.Initialize(XmlDocument);
XmlDocument.Save(projectWriter);
}

_versionOnDisk = Version;
Expand Down
60 changes: 57 additions & 3 deletions src/XMakeBuildEngine/Evaluation/Evaluator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,42 @@ element is ProjectOtherwiseElement
DebuggerManager.BakeStates(Path.GetFileNameWithoutExtension(currentProjectOrImport.FullPath));
}
#endif
IList<ProjectImportElement> implicitImports = new List<ProjectImportElement>();

if (!String.IsNullOrWhiteSpace(currentProjectOrImport.Sdk))
{
// SDK imports are added implicitly where they are evaluated at the top and bottom as if they are in the XML
//
foreach (string sdk in currentProjectOrImport.Sdk.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries)
.Select(i => i.Trim())
.Where(i => !String.IsNullOrWhiteSpace(i)))
{
int slashIndex = sdk.LastIndexOf("/", StringComparison.Ordinal);
string sdkName = slashIndex > 0 ? sdk.Substring(0, slashIndex) : sdk;

// TODO: do something other than just ignore the version

if (sdkName.Contains("/"))
{
ProjectErrorUtilities.ThrowInvalidProject(currentProjectOrImport.SdkLocation, "InvalidSdkFormat");
}

ProjectImportElement initialImport = ProjectImportElement.CreateDisconnected("Sdk.props", currentProjectOrImport);
initialImport.Sdk = sdkName;
initialImport.ImplicitImportLocation = ImplicitImportLocation.Top;
implicitImports.Add(initialImport);

ProjectImportElement finalImport = ProjectImportElement.CreateDisconnected("Sdk.targets", currentProjectOrImport);
finalImport.Sdk = sdkName;
finalImport.ImplicitImportLocation = ImplicitImportLocation.Bottom;
implicitImports.Add(finalImport);
}
}

foreach (var import in implicitImports.Where(i => i.ImplicitImportLocation == ImplicitImportLocation.Top))
{
EvaluateImportElement(currentProjectOrImport.DirectoryPath, import);
}

foreach (ProjectElement element in currentProjectOrImport.Children)
{
Expand Down Expand Up @@ -1143,6 +1179,11 @@ child is ProjectItemElement ||
ErrorUtilities.ThrowInternalError("Unexpected child type");
}

foreach (var import in implicitImports.Where(i => i.ImplicitImportLocation == ImplicitImportLocation.Bottom))
{
EvaluateImportElement(currentProjectOrImport.DirectoryPath, import);
}

#if FEATURE_MSBUILD_DEBUGGER
if (DebuggerManager.DebuggingEnabled)
{
Expand Down Expand Up @@ -2240,7 +2281,14 @@ private List<ProjectRootElement> ExpandAndLoadImports(string directoryOfImportin
continue;
}

var newExpandedImportPath = importElement.Project.Replace(extensionPropertyRefAsString, extensionPathExpanded);
string project = importElement.Project;
if (!String.IsNullOrWhiteSpace(importElement.Sdk))
{
project = Path.Combine(BuildEnvironmentHelper.Instance.MSBuildSDKsPath, importElement.Sdk, "Sdk", project);
}


var newExpandedImportPath = project.Replace(extensionPropertyRefAsString, extensionPathExpanded);
_loggingService.LogComment(_buildEventContext, MessageImportance.Low, "TryingExtensionsPath", newExpandedImportPath, extensionPathExpanded);

List<ProjectRootElement> projects;
Expand Down Expand Up @@ -2307,7 +2355,13 @@ private LoadImportsResult ExpandAndLoadImportsFromUnescapedImportExpressionCondi
return LoadImportsResult.ConditionWasFalse;
}

return ExpandAndLoadImportsFromUnescapedImportExpression(directoryOfImportingFile, importElement, importElement.Project, throwOnFileNotExistsError, out projects);
string project = importElement.Project;
if (!String.IsNullOrWhiteSpace(importElement.Sdk))
{
project = Path.Combine(BuildEnvironmentHelper.Instance.MSBuildSDKsPath, importElement.Sdk, "Sdk", project);
}

return ExpandAndLoadImportsFromUnescapedImportExpression(directoryOfImportingFile, importElement, project, throwOnFileNotExistsError, out projects);
}

/// <summary>
Expand Down Expand Up @@ -2410,7 +2464,7 @@ private LoadImportsResult ExpandAndLoadImportsFromUnescapedImportExpression(stri
{
parenthesizedProjectLocation = "[" + _projectRootElement.FullPath + "]";
}

// TODO: Detect if the duplicate import came from an SDK attribute
_loggingService.LogWarning(_buildEventContext, null, new BuildEventFileInfo(importLocationInProject), "DuplicateImport", importFileUnescaped, previouslyImportedAt.Location.LocationString, parenthesizedProjectLocation);
duplicateImport = true;
}
Expand Down
Loading

0 comments on commit 697c31f

Please sign in to comment.