From 13ad21e244122f386a9e7cb7b62ad9c253e5de0c Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 6 Dec 2016 11:33:34 -0800 Subject: [PATCH 1/6] Implementation of implicit imports during evaluation instead of during 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 #1447 --- ref/net46/Microsoft.Build/Microsoft.Build.cs | 5 +- .../Microsoft.Build/Microsoft.Build.cs | 5 +- src/Shared/XMakeAttributes.cs | 1 - .../Construction/ProjectElement.cs | 5 -- .../Construction/ProjectElementContainer.cs | 17 +---- .../Construction/ProjectImportElement.cs | 49 +++++++++++++ .../Construction/ProjectRootElement.cs | 70 +++++++++--------- src/XMakeBuildEngine/Evaluation/Evaluator.cs | 60 +++++++++++++++- .../Evaluation/Preprocessor.cs | 71 +++++++++++++++---- .../Evaluation/ProjectParser.cs | 67 +---------------- .../Evaluation/Preprocessor_Tests.cs | 8 ++- .../ProjectSdkImplicitImport_Tests.cs | 33 ++++++--- 12 files changed, 237 insertions(+), 154 deletions(-) diff --git a/ref/net46/Microsoft.Build/Microsoft.Build.cs b/ref/net46/Microsoft.Build/Microsoft.Build.cs index 5ba0171f72e..f1a9dd64273 100644 --- a/ref/net46/Microsoft.Build/Microsoft.Build.cs +++ b/ref/net46/Microsoft.Build/Microsoft.Build.cs @@ -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; } } @@ -86,6 +85,8 @@ 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 { } } + public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } } protected override Microsoft.Build.Construction.ProjectElement CreateNewInstance(Microsoft.Build.Construction.ProjectRootElement owner) { throw null; } } [System.Diagnostics.DebuggerDisplayAttribute("#Imports={Count} Condition={Condition} Label={Label}")] @@ -253,6 +254,8 @@ internal ProjectRootElement() { } public System.Collections.Generic.ICollection PropertyGroups { get { throw null; } } public System.Collections.Generic.ICollection 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 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 { } } diff --git a/ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs b/ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs index 5a847c6be83..5adfb077a8f 100644 --- a/ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs +++ b/ref/netstandard1.3/Microsoft.Build/Microsoft.Build.cs @@ -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; } } @@ -86,6 +85,8 @@ 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 { } } + public Microsoft.Build.Construction.ElementLocation SdkLocation { get { throw null; } } protected override Microsoft.Build.Construction.ProjectElement CreateNewInstance(Microsoft.Build.Construction.ProjectRootElement owner) { throw null; } } [System.Diagnostics.DebuggerDisplayAttribute("#Imports={Count} Condition={Condition} Label={Label}")] @@ -253,6 +254,8 @@ internal ProjectRootElement() { } public System.Collections.Generic.ICollection PropertyGroups { get { throw null; } } public System.Collections.Generic.ICollection 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 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 { } } diff --git a/src/Shared/XMakeAttributes.cs b/src/Shared/XMakeAttributes.cs index 3df2a54acf8..b34759d7653 100644 --- a/src/Shared/XMakeAttributes.cs +++ b/src/Shared/XMakeAttributes.cs @@ -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"; diff --git a/src/XMakeBuildEngine/Construction/ProjectElement.cs b/src/XMakeBuildEngine/Construction/ProjectElement.cs index 2cd34d3a166..9dc045eee1b 100644 --- a/src/XMakeBuildEngine/Construction/ProjectElement.cs +++ b/src/XMakeBuildEngine/Construction/ProjectElement.cs @@ -292,11 +292,6 @@ public ElementLocation Location get { return XmlElement.Location; } } - /// - /// Gets the Implicit state of the element: true if the element was not in the read XML. - /// - public bool IsImplicit => XmlElement.HasAttribute(XMakeAttributes.@implicit); - /// /// Gets the name of the associated element. /// Useful for display in some circumstances. diff --git a/src/XMakeBuildEngine/Construction/ProjectElementContainer.cs b/src/XMakeBuildEngine/Construction/ProjectElementContainer.cs index 1212a65c7b5..baf9ffc8338 100644 --- a/src/XMakeBuildEngine/Construction/ProjectElementContainer.cs +++ b/src/XMakeBuildEngine/Construction/ProjectElementContainer.cs @@ -455,11 +455,6 @@ internal void UpdateElementValue(ProjectElement child) /// internal void AddToXml(ProjectElement child) { - if (child.IsImplicit) - { - return; - } - if (child.ExpressedAsAttribute) { // todo children represented as attributes need to be placed in order too @@ -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 siblingIsExplicitElement = _ => _.ExpressedAsAttribute == false && _.IsImplicit == false; + Predicate siblingIsExplicitElement = _ => _.ExpressedAsAttribute == false; if (TrySearchLeftSiblings(child.PreviousSibling, siblingIsExplicitElement, out referenceSibling)) { @@ -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); @@ -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); } /// diff --git a/src/XMakeBuildEngine/Construction/ProjectImportElement.cs b/src/XMakeBuildEngine/Construction/ProjectImportElement.cs index 1bf6cfc9269..1d3c9cb8c5c 100644 --- a/src/XMakeBuildEngine/Construction/ProjectImportElement.cs +++ b/src/XMakeBuildEngine/Construction/ProjectImportElement.cs @@ -61,6 +61,36 @@ public string Project /// public ElementLocation ProjectLocation => XmlElement.GetAttributeLocation(XMakeAttributes.project); + /// + /// Gets or sets the SDK that contains the import. + /// + 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); + } + } + + /// + /// Location of the Sdk attribute + /// + public ElementLocation SdkLocation => XmlElement.GetAttributeLocation(XMakeAttributes.sdk); + + /// + /// Gets or sets the of the import. + /// + internal ImplicitImportLocation ImplicitImportLocation { get; set; } = ImplicitImportLocation.None; + /// /// Creates an unparented ProjectImportElement, wrapping an unparented XmlElement. /// Validates the project value. @@ -92,4 +122,23 @@ protected override ProjectElement CreateNewInstance(ProjectRootElement owner) return owner.CreateImportElement(this.Project); } } + + /// + /// Represents the location of an implicit import. + /// + internal enum ImplicitImportLocation + { + /// + /// The import is not implicit. + /// + None, + /// + /// The import should be at the top. + /// + Top, + /// + /// The import should be at the bottom. + /// + Bottom + } } diff --git a/src/XMakeBuildEngine/Construction/ProjectRootElement.cs b/src/XMakeBuildEngine/Construction/ProjectRootElement.cs index 2af810ef0f3..4a6ca348de1 100644 --- a/src/XMakeBuildEngine/Construction/ProjectRootElement.cs +++ b/src/XMakeBuildEngine/Construction/ProjectRootElement.cs @@ -157,11 +157,6 @@ public class ProjectRootElement : ProjectElementContainer /// private BuildEventContext _buildEventContext; - /// - /// Xpath expression that will find any element with the implicit attribute - /// - private static readonly string ImplicitAttributeXpath = $"//*[@{XMakeAttributes.@implicit}]"; - /// /// Initialize a ProjectRootElement instance from a XmlReader. /// May throw InvalidProjectFileException. @@ -654,6 +649,25 @@ public string InitialTargets } } + /// + /// + /// + 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); + } + } + /// /// 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. @@ -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(); @@ -848,6 +860,14 @@ public ElementLocation InitialTargetsLocation get { return XmlElement.GetAttributeLocation(XMakeAttributes.initialTargets); } } + /// + /// + /// + public ElementLocation SdkLocation + { + get { return XmlElement.GetAttributeLocation(XMakeAttributes.sdk); } + } + /// /// Location of the TreatAsLocalProperty attribute, if any /// @@ -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; @@ -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; - } - /// /// Save the project to the file system, if dirty or the path is different. /// Creates any necessary directories. @@ -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; diff --git a/src/XMakeBuildEngine/Evaluation/Evaluator.cs b/src/XMakeBuildEngine/Evaluation/Evaluator.cs index c1411a730d4..cb2887c7622 100644 --- a/src/XMakeBuildEngine/Evaluation/Evaluator.cs +++ b/src/XMakeBuildEngine/Evaluation/Evaluator.cs @@ -987,6 +987,42 @@ element is ProjectOtherwiseElement DebuggerManager.BakeStates(Path.GetFileNameWithoutExtension(currentProjectOrImport.FullPath)); } #endif + IList implicitImports = new List(); + + 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) { @@ -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) { @@ -2240,7 +2281,14 @@ private List 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 projects; @@ -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); } /// @@ -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; } diff --git a/src/XMakeBuildEngine/Evaluation/Preprocessor.cs b/src/XMakeBuildEngine/Evaluation/Preprocessor.cs index 462d81094c9..8734b65fe82 100644 --- a/src/XMakeBuildEngine/Evaluation/Preprocessor.cs +++ b/src/XMakeBuildEngine/Evaluation/Preprocessor.cs @@ -35,17 +35,17 @@ internal class Preprocessor /// /// Project to preprocess /// - private Project _project; + private readonly Project _project; /// /// Table to resolve import tags /// - private Dictionary> _importTable; + private readonly Dictionary> _importTable; /// /// Stack of file paths pushed as we follow imports /// - private Stack _filePaths = new Stack(); + private readonly Stack _filePaths = new Stack(); /// /// Constructor @@ -56,15 +56,15 @@ private Preprocessor(Project project) IList imports = project.Imports; - _importTable = new Dictionary>(imports.Count); + _importTable = new Dictionary>(imports.Count); foreach (ResolvedImport entry in imports) { IList list; - if (!_importTable.TryGetValue(entry.ImportingElement.XmlElement, out list)) + if (!_importTable.TryGetValue(entry.ImportingElement.XmlElement.OuterXml, out list)) { list = new List(); - _importTable[entry.ImportingElement.XmlElement] = list; + _importTable[entry.ImportingElement.XmlElement.OuterXml] = list; } list.Add(entry.ImportedProject); @@ -91,9 +91,44 @@ private XmlDocument Preprocess() { XmlDocument outerDocument = _project.Xml.XmlDocument; - XmlDocument destinationDocument = (XmlDocument)outerDocument.CloneNode(false /* shallow */); + int implicitImportCount = _project.Imports.Count(i => i.ImportingElement.ImplicitImportLocation != ImplicitImportLocation.None); + // At the time of adding this feature, cloning is buggy. The implicit imports are added to the XML document and removed after + // processing. This variable keeps track of the nodes that were added + IList addedNodes = new List(implicitImportCount); + XmlElement documentElement = outerDocument.DocumentElement; - // TODO: Remove Sdk attribute from destination document and put it in a comment (so that if you do a build on a preprocessed file it won't try to import the Sdk imports twice) + if (implicitImportCount > 0 && documentElement != null) + { + // Top implicit imports need to be added in the correct order by adding the first one at the top and each one after the first + // one. This variable keeps track of the last import that was added. + XmlNode lastImplicitImportAdded = null; + + // Add the implicit top imports + // + foreach (var import in _project.Imports.Where(i => i.ImportingElement.ImplicitImportLocation == ImplicitImportLocation.Top)) + { + XmlNode node = outerDocument.ImportNode(import.ImportingElement.XmlElement, false); + if (lastImplicitImportAdded == null) + { + documentElement.InsertBefore(node, documentElement.FirstChild); + lastImplicitImportAdded = node; + } + else + { + documentElement.InsertAfter(node, lastImplicitImportAdded); + } + addedNodes.Add(node); + } + + // Add the implicit bottom imports + // + foreach (var import in _project.Imports.Where(i => i.ImportingElement.ImplicitImportLocation == ImplicitImportLocation.Bottom)) + { + addedNodes.Add(documentElement.InsertAfter(outerDocument.ImportNode(import.ImportingElement.XmlElement, false), documentElement.LastChild)); + } + } + + XmlDocument destinationDocument = (XmlDocument)outerDocument.CloneNode(false /* shallow */); _filePaths.Push(_project.FullPath); @@ -104,6 +139,13 @@ private XmlDocument Preprocess() CloneChildrenResolvingImports(outerDocument, destinationDocument); + // Remove the nodes that were added as implicit imports + // + foreach (XmlNode addedNode in addedNodes) + { + documentElement?.RemoveChild(addedNode); + } + return destinationDocument; } @@ -174,7 +216,7 @@ private void CloneChildrenResolvingImports(XmlNode source, XmlNode destination) string sdk = importSdk.Length > 0 ? $" {XMakeAttributes.sdk}=\"{importSdk}\"" : String.Empty; IList resolvedList; - if (!_importTable.TryGetValue(child as XmlElement, out resolvedList)) + if (!_importTable.TryGetValue(((XmlElement)child).OuterXml, out resolvedList)) { // Import didn't resolve to anything; just display as a comment and move on string closedImportTag = @@ -192,10 +234,10 @@ private void CloneChildrenResolvingImports(XmlNode source, XmlNode destination) string importTag = $" "; - if (((XmlElement)child).GetAttribute(XMakeAttributes.@implicit).Length > 0) + if (!String.IsNullOrWhiteSpace(importSdk) && _project.Xml.Sdk.IndexOf(importSdk, StringComparison.OrdinalIgnoreCase) >= 0) { - importTag = - $" Import of \"{importProject.Replace("--", "__")}\" from Sdk \"{importSdk}\" was implied by the {XMakeElements.project} element's {XMakeAttributes.sdk} attribute."; + importTag += + $"\r\n This import was added implicitly because of the {XMakeElements.project} element's {XMakeAttributes.sdk} attribute specified \"{importSdk}\"."; } destination.AppendChild(destinationDocument.CreateComment( @@ -236,6 +278,11 @@ private void CloneChildrenResolvingImports(XmlNode source, XmlNode destination) // Node doesn't need special treatment, clone and append XmlNode clone = destinationDocument.ImportNode(child, false /* shallow */); // ImportNode does a clone but unlike CloneNode it works across XmlDocuments + if (clone.NodeType == XmlNodeType.Element && String.Equals(XMakeElements.project, child.Name, StringComparison.Ordinal) && clone.Attributes?[XMakeAttributes.sdk] != null) + { + clone.Attributes.Remove(clone.Attributes[XMakeAttributes.sdk]); + } + destination.AppendChild(clone); CloneChildrenResolvingImports(child, clone); diff --git a/src/XMakeBuildEngine/Evaluation/ProjectParser.cs b/src/XMakeBuildEngine/Evaluation/ProjectParser.cs index 53b43c2faf3..0e1846855d8 100644 --- a/src/XMakeBuildEngine/Evaluation/ProjectParser.cs +++ b/src/XMakeBuildEngine/Evaluation/ProjectParser.cs @@ -47,7 +47,7 @@ internal class ProjectParser /// /// Valid attributes on import element /// - private readonly static string[] s_validAttributesOnImport = new string[] { XMakeAttributes.condition, XMakeAttributes.label, XMakeAttributes.project, XMakeAttributes.sdk, XMakeAttributes.@implicit }; + private readonly static string[] s_validAttributesOnImport = new string[] { XMakeAttributes.condition, XMakeAttributes.label, XMakeAttributes.project, XMakeAttributes.sdk }; /// /// Valid attributes on usingtask element @@ -187,61 +187,6 @@ private void ParseProjectElement(XmlElementWithLocation element) // so we have to set it now _project.SetProjectRootElementFromParser(element, _project); - if (element.HasAttribute(XMakeAttributes.sdk)) - { - - var sdksString = element.GetAttribute(XMakeAttributes.sdk); - - var sdks = sdksString.Contains(";") ? sdksString.Split(';') : new[] {sdksString}; - - foreach (var sdk in sdks) - { - var 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(element.GetAttributeLocation(XMakeAttributes.sdk), - "InvalidSdkFormat"); - } - - // TODO: paths should just be Sdk.props/targets; Sdk-aware imports should do the rest of the path. - var initialImportPath = Path.Combine(BuildEnvironmentHelper.Instance.MSBuildSDKsPath, - sdkName, "Sdk", "Sdk.props"); - var finalImportPath = Path.Combine(BuildEnvironmentHelper.Instance.MSBuildSDKsPath, - sdkName, "Sdk", "Sdk.targets"); - - // TODO: don't require all SDKs to have both props and targets - // if (File.Exists(initialImportPath)) - { - var implicitImportElement = element.OwnerDocument.CreateElement(XMakeElements.import); - - // TODO: make this - implicitImportElement.SetAttribute(XMakeAttributes.project, - initialImportPath); - implicitImportElement.SetAttribute(XMakeAttributes.@implicit, $"Sdk = {sdkName}"); - implicitImportElement.SetAttribute(XMakeAttributes.sdk, sdkName); - - element.PrependChild(implicitImportElement); - } - - // TODO: don't require all SDKs to have both props and targets - // if (File.Exists(finalImportPath)) - { - var implicitImportElement = element.OwnerDocument.CreateElement(XMakeElements.import); - - // TODO: make this - implicitImportElement.SetAttribute(XMakeAttributes.project, - finalImportPath); - implicitImportElement.SetAttribute(XMakeAttributes.@implicit, $"Sdk = {sdkName}"); - implicitImportElement.SetAttribute(XMakeAttributes.sdk, sdkName); - - element.AppendChild(implicitImportElement); - } - } - } ParseProjectRootElementChildren(element); } @@ -573,16 +518,6 @@ private ProjectImportElement ParseProjectImportElement(XmlElementWithLocation el ProjectXmlUtilities.VerifyThrowProjectNoChildElements(element); - if (element.HasAttribute(XMakeAttributes.@implicit)) - { - // the implicit attribute is only allowed on Import, but only if MSBuild itself - // put it there. If a user specified it in a project, error just like we would - // when encountering an unknown attribute. - ProjectXmlUtilities.VerifyThrowProjectInvalidAttribute( - element.Location.Line == 0 && element.Location.Column == 0, - element.GetAttributeWithLocation(XMakeAttributes.@implicit)); - } - return new ProjectImportElement(element, parent, _project); } diff --git a/src/XMakeBuildEngine/UnitTests/Evaluation/Preprocessor_Tests.cs b/src/XMakeBuildEngine/UnitTests/Evaluation/Preprocessor_Tests.cs index 71acdfd07e5..9aa3094302f 100644 --- a/src/XMakeBuildEngine/UnitTests/Evaluation/Preprocessor_Tests.cs +++ b/src/XMakeBuildEngine/UnitTests/Evaluation/Preprocessor_Tests.cs @@ -885,10 +885,11 @@ public void SdkImportsAreInPreprocessedOutput() string expected = ObjectModelHelpers.CleanupFileContents( $@" - +