diff --git a/src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs b/src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs index df1eb401b16..f0aa2674689 100644 --- a/src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs @@ -1897,8 +1897,8 @@ public static IEnumerable TaskRegistryTranslationTestData "dotv", new Dictionary { - {"a", new ProjectImportPathMatch("a", "b;c")}, - {"d", new ProjectImportPathMatch("d", "e;f")} + {"a", new ProjectImportPathMatch("a", new List {"b", "c"})}, + {"d", new ProjectImportPathMatch("d", new List {"e", "f"})} } ); diff --git a/src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs b/src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs index 11db7b58526..76842cfa17b 100644 --- a/src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs +++ b/src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs @@ -526,7 +526,7 @@ public void ExtensionPathsTest_Basic1() - + @@ -557,7 +557,7 @@ public void ExtensionPathsTest_Basic1() Assert.Equal(allPaths.GetElement(1).OS, "osx"); Assert.Equal(allPaths.GetElement(1).PropertyElements.Count, 2); - Assert.Equal(allPaths.GetElement(1).PropertyElements.GetElement("MSBuildExtensionsPath").Value, @"/tmp/foo;$(FallbackPaths)"); + Assert.Equal(allPaths.GetElement(1).PropertyElements.GetElement("MSBuildExtensionsPath").Value, @"/tmp/foo"); Assert.Equal(allPaths.GetElement(1).PropertyElements.GetElement("MSBuildExtensionsPath32").Value, @"/tmp/foo32;/tmp/bar32"); Assert.Equal(allPaths.GetElement(2).OS, "unix"); @@ -578,7 +578,7 @@ public void ExtensionPathsTest_Basic1() } else if (NativeMethodsShared.IsOSX) { - CheckPathsTable(pathsTable, "MSBuildExtensionsPath", new string[] {"/tmp/foo", "$(FallbackPaths)"}); + CheckPathsTable(pathsTable, "MSBuildExtensionsPath", new string[] {"/tmp/foo"}); CheckPathsTable(pathsTable, "MSBuildExtensionsPath32", new string[] {"/tmp/foo32", "/tmp/bar32"}); } else @@ -591,16 +591,11 @@ private void CheckPathsTable(Dictionary pathsTab { Assert.True(pathsTable.ContainsKey(kind)); var paths = pathsTable[kind]; + Assert.Equal(paths.SearchPaths.Count, expectedPaths.Length); - // Property expansion would be done in the context of an - // actual project. So, without that, pathsTable would - // have the unexpanded strings. - var searchPaths = paths.GetExpandedSearchPaths(p => p); - Assert.Equal(searchPaths.Count, expectedPaths.Length); - - for (int i = 0; i < searchPaths.Count; i ++) + for (int i = 0; i < paths.SearchPaths.Count; i ++) { - Assert.Equal(searchPaths[i], expectedPaths[i]); + Assert.Equal(paths.SearchPaths[i], expectedPaths[i]); } } diff --git a/src/Build.UnitTests/Definition/Toolset_Tests.cs b/src/Build.UnitTests/Definition/Toolset_Tests.cs index 161fbcdd9a1..50efa1deda0 100644 --- a/src/Build.UnitTests/Definition/Toolset_Tests.cs +++ b/src/Build.UnitTests/Definition/Toolset_Tests.cs @@ -117,7 +117,7 @@ public void ValidateToolsetTranslation() Toolset t = new Toolset("4.0", "c:\\bar", buildProperties, environmentProperties, globalProperties, subToolsets, "c:\\foo", "4.0", new Dictionary { - ["MSBuildExtensionsPath"] = new ProjectImportPathMatch("MSBuildExtensionsPath", @"c:\foo") + ["MSBuildExtensionsPath"] = new ProjectImportPathMatch("MSBuildExtensionsPath", new List {@"c:\foo"}) }); ((INodePacketTranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); @@ -162,7 +162,7 @@ public void ValidateToolsetTranslation() Assert.NotNull(t2.ImportPropertySearchPathsTable); Assert.Equal(1, t2.ImportPropertySearchPathsTable.Count); - Assert.Equal(@"c:\foo", t2.ImportPropertySearchPathsTable["MSBuildExtensionsPath"].GetExpandedSearchPaths(p => p)[0]); + Assert.Equal(@"c:\foo", t2.ImportPropertySearchPathsTable["MSBuildExtensionsPath"].SearchPaths[0]); } [Fact] diff --git a/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs b/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs index 5afe2ce71d0..5f948abacfb 100644 --- a/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs +++ b/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs @@ -515,31 +515,16 @@ public void ImportFromExtensionsPathAnd32And64() } } - [Theory] - [InlineData( - "/a/b;$(FallbackPaths);/x/y", - @"$(FallbackExpandDir1);/a/b - /xyz/tmp;$(IndirectRefProperty)", null, false)] - // has fallback paths, but won't point to the valid one - [InlineData( - "/a/b;$(NonExistantProperty)", - "", - typeof (InvalidProjectFileException), true)] - // no fallback paths after expansion, so should ignore fallback-paths code path - [InlineData( - "$(NonExistantProperty)", - @"", typeof(InvalidProjectFileException), false)] - // no fallback paths after expansion, so should ignore fallback-paths code path - [InlineData( - "", - @"", typeof(InvalidProjectFileException), false)] - public void ExpandExtensionsPathFallback(string fallbackPathsInAppConfig, string propertiesInMainProject, Type expectedException, bool exceptionHasFallbacks) + // Fall-back path that has a property in it: $(FallbackExpandDir1) + [Fact] + public void ExpandExtensionsPathFallback() { string extnTargetsFileContentTemplate = @" + "; var configFileContents = @" @@ -553,7 +538,7 @@ public void ExpandExtensionsPathFallback(string fallbackPathsInAppConfig, string - + @@ -565,57 +550,24 @@ public void ExpandExtensionsPathFallback(string fallbackPathsInAppConfig, string try { - string mainTargetsFileContent = @" - - - {0} - - - - - - - - "; - - mainTargetsFileContent = string.Format(mainTargetsFileContent, propertiesInMainProject, "MSBuildExtensionsPath"); - mainProjectPath = ObjectModelHelpers.CreateFileInTempProjectDirectory("main.proj", mainTargetsFileContent); - extnDir1 = GetNewExtensionsPathAndCreateFile("extensions1", Path.Combine("foo", "extn.proj"), extnTargetsFileContentTemplate); + mainProjectPath = ObjectModelHelpers.CreateFileInTempProjectDirectory("main.proj", + GetMainTargetFileContent()); + ToolsetConfigurationReaderTestHelper.WriteConfigFile(configFileContents); var reader = GetStandardConfigurationReader(); - var logger = new MockLogger(); - try { - var projectCollection = new ProjectCollection(new Dictionary {["FallbackExpandDir1"] = extnDir1}); - - projectCollection.ResetToolsetsForTests(reader); - projectCollection.RegisterLogger(logger); - - var project = projectCollection.LoadProject(mainProjectPath); - Assert.True(project.Build("Main")); - logger.AssertLogContains("Running FromExtn"); + var projectCollection = new ProjectCollection(new Dictionary {["FallbackExpandDir1"] = extnDir1}); - Assert.True(expectedException == null, $"Didn't get any exception. Expected: {expectedException}"); - } - catch (Exception ex) - { - if (expectedException == null) - throw; - - Assert.Equal(ex.GetType(), expectedException); + projectCollection.ResetToolsetsForTests(reader); + var logger = new MockLogger(); + projectCollection.RegisterLogger(logger); - if (exceptionHasFallbacks) - { - logger.AssertLogContains("in the fallback search path"); - } - else - { - logger.AssertLogDoesntContain("in the fallback search path"); - } - } + var project = projectCollection.LoadProject(mainProjectPath); + Assert.True(project.Build("Main")); + logger.AssertLogContains("Running FromExtn"); } finally { diff --git a/src/Build/Definition/ProjectImportPathMatch.cs b/src/Build/Definition/ProjectImportPathMatch.cs index d59cbe9efaf..9a6979a7576 100644 --- a/src/Build/Definition/ProjectImportPathMatch.cs +++ b/src/Build/Definition/ProjectImportPathMatch.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Linq; using Microsoft.Build.BackEnd; using Microsoft.Build.Shared; @@ -14,31 +13,19 @@ namespace Microsoft.Build.Evaluation /// internal class ProjectImportPathMatch : INodePacketTranslatable { - /// - /// Character used to separate search paths specified for MSBuildExtensionsPath* in - /// the config file - /// - private static char s_separatorForExtensionsPathSearchPaths = ';'; - /// /// ProjectImportPathMatch instance representing no fall-back /// - public static readonly ProjectImportPathMatch None = new ProjectImportPathMatch(string.Empty, string.Empty); + public static readonly ProjectImportPathMatch None = new ProjectImportPathMatch(string.Empty, new List()); - internal ProjectImportPathMatch(string propertyName, string propertyValue) + internal ProjectImportPathMatch(string propertyName, List searchPaths) { ErrorUtilities.VerifyThrowArgumentNull(propertyName, nameof(propertyName)); - ErrorUtilities.VerifyThrowArgumentNull(propertyValue, nameof(propertyValue)); + ErrorUtilities.VerifyThrowArgumentNull(searchPaths, nameof(searchPaths)); PropertyName = propertyName; - PropertyValue = propertyValue; + SearchPaths = searchPaths; MsBuildPropertyFormat = $"$({PropertyName})"; - - // cache the result for @propertyValue="" case also - if (!ProjectImportPathMatch.HasPropertyReference(propertyValue) || string.IsNullOrEmpty(propertyValue)) - { - SearchPathsWithNoExpansionRequired = ProjectImportPathMatch.SplitSearchPaths(propertyValue); - } } public ProjectImportPathMatch(INodePacketTranslator translator) @@ -51,11 +38,6 @@ public ProjectImportPathMatch(INodePacketTranslator translator) /// public string PropertyName; - /// - /// String representation of the property value - /// - public string PropertyValue; - /// /// Returns the corresponding property name - eg. "$(MSBuildExtensionsPath32)" /// @@ -64,44 +46,15 @@ public ProjectImportPathMatch(INodePacketTranslator translator) /// /// Enumeration of the search paths for the property. /// - private List SearchPathsWithNoExpansionRequired; + public List SearchPaths; public void Translate(INodePacketTranslator translator) { translator.Translate(ref PropertyName); - translator.Translate(ref PropertyValue); translator.Translate(ref MsBuildPropertyFormat); - translator.Translate(ref SearchPathsWithNoExpansionRequired); + translator.Translate(ref SearchPaths); } - /// - /// Gets the list of search paths with any property references expanded using @expandPropertyReferences - /// Func that expands properties in a string - /// List of expanded search paths - /// - public IList GetExpandedSearchPaths(Func expandPropertyReferences) - { - ErrorUtilities.VerifyThrowArgumentNull(expandPropertyReferences, nameof(expandPropertyReferences)); - - /// If SearchPathsWithNoExpansionRequired is not-null, then it means that the PropertyValue - /// did not have any property reference and so we just return the list we prepared during - /// construction. - return SearchPathsWithNoExpansionRequired ?? ProjectImportPathMatch.SplitSearchPaths(expandPropertyReferences(PropertyValue)); - } - - /// - /// Splits @fullString on @s_separatorForExtensionsPathSearchPaths - /// - /// FIXME: handle ; in path on Unix - private static List SplitSearchPaths(string fullString) => fullString - .Split(new[] {s_separatorForExtensionsPathSearchPaths}, StringSplitOptions.RemoveEmptyEntries) - .Distinct().ToList(); - - /// - /// Returns true if @value might have a property reference - /// - private static bool HasPropertyReference(string value) => value.Contains("$("); - /// /// Factory for serialization. /// @@ -110,4 +63,4 @@ internal static ProjectImportPathMatch FactoryForDeserialization(INodePacketTran return new ProjectImportPathMatch(translator); } } -} +} \ No newline at end of file diff --git a/src/Build/Definition/ToolsetConfigurationReader.cs b/src/Build/Definition/ToolsetConfigurationReader.cs index 149425f211c..6dfef1c027e 100644 --- a/src/Build/Definition/ToolsetConfigurationReader.cs +++ b/src/Build/Definition/ToolsetConfigurationReader.cs @@ -36,6 +36,12 @@ internal class ToolsetConfigurationReader : ToolsetReader /// private bool _configurationReadAttempted = false; + /// + /// Character used to separate search paths specified for MSBuildExtensionsPath* in + /// the config file + /// + private char _separatorForExtensionsPathSearchPaths = ';'; + /// /// Cached values of tools version -> project import search paths table /// @@ -229,7 +235,13 @@ private Dictionary ComputeDistinctListOfSearchPa continue; } - pathsTable.Add(property.Name, new ProjectImportPathMatch(property.Name, property.Value)); + //FIXME: handle ; in path on Unix + var paths = property.Value + .Split(new[] {_separatorForExtensionsPathSearchPaths}, StringSplitOptions.RemoveEmptyEntries) + .Distinct() + .Where(path => !string.IsNullOrEmpty(path)); + + pathsTable.Add(property.Name, new ProjectImportPathMatch(property.Name, paths.ToList())); } return pathsTable; @@ -255,4 +267,4 @@ private static Configuration ReadApplicationConfiguration() return ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None); } } -} +} \ No newline at end of file diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index e49de3cd4de..99a221f887f 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1926,11 +1926,9 @@ private List ExpandAndLoadImports(string directoryOfImportin var fallbackSearchPathMatch = _data.Toolset.GetProjectImportSearchPaths(importElement.Project); sdkResult = null; - IList onlyFallbackSearchPaths = fallbackSearchPathMatch.GetExpandedSearchPaths(fallbackPath => _data.ExpandString(fallbackPath)); - // no reference or we need to lookup only the default path, // so, use the Import path - if (fallbackSearchPathMatch.Equals(ProjectImportPathMatch.None) || onlyFallbackSearchPaths.Count == 0) + if (fallbackSearchPathMatch.Equals(ProjectImportPathMatch.None)) { List projects; ExpandAndLoadImportsFromUnescapedImportExpressionConditioned(directoryOfImportingFile, importElement, out projects, out sdkResult); @@ -1982,16 +1980,15 @@ private List ExpandAndLoadImports(string directoryOfImportin // Adding the value of $(MSBuildExtensionsPath*) property to the list of search paths var prop = _data.GetProperty(fallbackSearchPathMatch.PropertyName); - var expandedPathsToSearch = new List(); - expandedPathsToSearch.Add(prop?.EvaluatedValue); // The actual value of the property, with no fallbacks - - expandedPathsToSearch.AddRange(onlyFallbackSearchPaths); - + var pathsToSearch = new string[fallbackSearchPathMatch.SearchPaths.Count + 1]; + pathsToSearch[0] = prop?.EvaluatedValue; // The actual value of the property, with no fallbacks + fallbackSearchPathMatch.SearchPaths.CopyTo(pathsToSearch, 1); // The list of fallbacks, in order + string extensionPropertyRefAsString = fallbackSearchPathMatch.MsBuildPropertyFormat; _evaluationLoggingContext.LogComment(MessageImportance.Low, "SearchPathsForMSBuildExtensionsPath", extensionPropertyRefAsString, - String.Join(";", expandedPathsToSearch)); + String.Join(";", pathsToSearch)); bool atleastOneExactFilePathWasLookedAtAndNotFound = false; @@ -2003,13 +2000,15 @@ private List ExpandAndLoadImports(string directoryOfImportin // Try every extension search path, till we get a Hit: // 1. 1 or more project files loaded // 2. 1 or more project files *found* but ignored (like circular, self imports) - foreach (var extensionPathExpanded in expandedPathsToSearch) + foreach (var extensionPath in pathsToSearch) { // In the rare case that the property we've enabled for search paths hasn't been defined // we will skip it, but continue with other paths in the fallback order. - if (string.IsNullOrEmpty(extensionPathExpanded)) + if (string.IsNullOrEmpty(extensionPath)) continue; + string extensionPathExpanded = _data.ExpandString(extensionPath); + if (!_fallbackSearchPathsCache.DirectoryExists(extensionPathExpanded)) { continue; @@ -2068,7 +2067,7 @@ private List ExpandAndLoadImports(string directoryOfImportin atleastOneExactFilePathWasLookedAtAndNotFound && (_loadSettings & ProjectLoadSettings.IgnoreMissingImports) == 0) { - ThrowForImportedProjectWithSearchPathsNotFound(fallbackSearchPathMatch, onlyFallbackSearchPaths, importElement); + ThrowForImportedProjectWithSearchPathsNotFound(fallbackSearchPathMatch, importElement); } return allProjects; @@ -2650,7 +2649,7 @@ private void RecordEvaluatedItemElement(ProjectItemElement itemElement) /// MSBuildExtensionsPath reference kind found in the Project attribute of the Import element /// The importing element for this import /// - private void ThrowForImportedProjectWithSearchPathsNotFound(ProjectImportPathMatch searchPathMatch, IList onlyFallbackSearchPaths, ProjectImportElement importElement) + private void ThrowForImportedProjectWithSearchPathsNotFound(ProjectImportPathMatch searchPathMatch, ProjectImportElement importElement) { var extensionsPathProp = _data.GetProperty(searchPathMatch.PropertyName); string importExpandedWithDefaultPath; @@ -2675,6 +2674,8 @@ private void ThrowForImportedProjectWithSearchPathsNotFound(ProjectImportPathMat relativeProjectPath = importElement.Project; } + var onlyFallbackSearchPaths = searchPathMatch.SearchPaths.Select(s => _data.ExpandString(s)).ToList(); + string stringifiedListOfSearchPaths = StringifyList(onlyFallbackSearchPaths); #if FEATURE_SYSTEM_CONFIGURATION diff --git a/src/MSBuild/app.config b/src/MSBuild/app.config index cf0adc8ea41..64aeb0cfabe 100644 --- a/src/MSBuild/app.config +++ b/src/MSBuild/app.config @@ -100,9 +100,9 @@ - - - + + +