Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

Commit

Permalink
Revert "Allow overriding fallback paths for MSBuildExtensionsPath via…
Browse files Browse the repository at this point in the history 
… a property (#59)"

This reverts commit c4e70a6.
  • Loading branch information
@akoeplinger
akoeplinger committed Oct 11, 2018
1 parent 17218b6 commit 54f30ab
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 150 deletions.
4 changes: 2 additions & 2 deletions src/Build.UnitTests/BackEnd/TaskRegistry_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1897,8 +1897,8 @@ public static IEnumerable<object[]> TaskRegistryTranslationTestData
"dotv",
new Dictionary<string, ProjectImportPathMatch>
{
{"a", new ProjectImportPathMatch("a", "b;c")},
{"d", new ProjectImportPathMatch("d", "e;f")}
{"a", new ProjectImportPathMatch("a", new List<string> {"b", "c"})},
{"d", new ProjectImportPathMatch("d", new List<string> {"e", "f"})}
}
);

Expand Down
17 changes: 6 additions & 11 deletions src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ public void ExtensionPathsTest_Basic1()
<property name=""MSBuildExtensionsPath64"" value=""c:\foo64;c:\bar64""/>
</searchPaths>
<searchPaths os=""osx"">
<property name=""MSBuildExtensionsPath"" value=""/tmp/foo;$(FallbackPaths)""/>
<property name=""MSBuildExtensionsPath"" value=""/tmp/foo""/>
<property name=""MSBuildExtensionsPath32"" value=""/tmp/foo32;/tmp/bar32""/>
</searchPaths>
<searchPaths os=""unix"">
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand All @@ -591,16 +591,11 @@ private void CheckPathsTable(Dictionary<string, ProjectImportPathMatch> 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]);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Build.UnitTests/Definition/Toolset_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, ProjectImportPathMatch>
{
["MSBuildExtensionsPath"] = new ProjectImportPathMatch("MSBuildExtensionsPath", @"c:\foo")
["MSBuildExtensionsPath"] = new ProjectImportPathMatch("MSBuildExtensionsPath", new List<string> {@"c:\foo"})
});

((INodePacketTranslatable)t).Translate(TranslationHelpers.GetWriteTranslator());
Expand Down Expand Up @@ -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]
Expand Down
78 changes: 15 additions & 63 deletions src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,31 +515,16 @@ public void ImportFromExtensionsPathAnd32And64()
}
}

[Theory]
[InlineData(
"/a/b;$(FallbackPaths);/x/y",
@"<IndirectRefProperty>$(FallbackExpandDir1);/a/b</IndirectRefProperty>
<FallbackPaths>/xyz/tmp;$(IndirectRefProperty)</FallbackPaths>", 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 = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Target Name='FromExtn'>
<Message Text='Running FromExtn'/>
</Target>
<Import Project='$(MSBuildExtensionsPath)\\foo\\extn.proj' Condition=""Exists('$(MSBuildExtensionsPath)\foo\extn.proj')"" />
</Project>";

var configFileContents = @"
Expand All @@ -553,7 +538,7 @@ public void ExpandExtensionsPathFallback(string fallbackPathsInAppConfig, string
<property name=""MSBuildBinPath"" value="".""/>
<projectImportSearchPaths>
<searchPaths os=""" + NativeMethodsShared.GetOSNameForExtensionsPath() + @""">
<property name=""MSBuildExtensionsPath"" value=""" + fallbackPathsInAppConfig + @""" />
<property name=""MSBuildExtensionsPath"" value=""$(FallbackExpandDir1)"" />
</searchPaths>
</projectImportSearchPaths>
</toolset>
Expand All @@ -565,57 +550,24 @@ public void ExpandExtensionsPathFallback(string fallbackPathsInAppConfig, string

try
{
string mainTargetsFileContent = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<PropertyGroup>
{0}
</PropertyGroup>
<Target Name='Main' DependsOnTargets='FromExtn'>
<Message Text='PropertyFromExtn1: $(PropertyFromExtn1)'/>
</Target>
<Import Project='$({1})\foo\extn.proj'/>
</Project>";

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<string, string> {["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<string, string> {["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
{
Expand Down
61 changes: 7 additions & 54 deletions src/Build/Definition/ProjectImportPathMatch.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Shared;

Expand All @@ -14,31 +13,19 @@ namespace Microsoft.Build.Evaluation
/// </summary>
internal class ProjectImportPathMatch : INodePacketTranslatable
{
/// <summary>
/// Character used to separate search paths specified for MSBuildExtensionsPath* in
/// the config file
/// </summary>
private static char s_separatorForExtensionsPathSearchPaths = ';';

/// <summary>
/// ProjectImportPathMatch instance representing no fall-back
/// </summary>
public static readonly ProjectImportPathMatch None = new ProjectImportPathMatch(string.Empty, string.Empty);
public static readonly ProjectImportPathMatch None = new ProjectImportPathMatch(string.Empty, new List<string>());

internal ProjectImportPathMatch(string propertyName, string propertyValue)
internal ProjectImportPathMatch(string propertyName, List<string> 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)
Expand All @@ -51,11 +38,6 @@ public ProjectImportPathMatch(INodePacketTranslator translator)
/// </summary>
public string PropertyName;

/// <summary>
/// String representation of the property value
/// </summary>
public string PropertyValue;

/// <summary>
/// Returns the corresponding property name - eg. "$(MSBuildExtensionsPath32)"
/// </summary>
Expand All @@ -64,44 +46,15 @@ public ProjectImportPathMatch(INodePacketTranslator translator)
/// <summary>
/// Enumeration of the search paths for the property.
/// </summary>
private List<string> SearchPathsWithNoExpansionRequired;
public List<string> 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);
}

/// <summary>
/// Gets the list of search paths with any property references expanded using @expandPropertyReferences
/// <param name="expandPropertyReferences">Func that expands properties in a string</param>
/// <returns>List of expanded search paths</returns>
/// </summary>
public IList<string> GetExpandedSearchPaths(Func<string, string> 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));
}

/// <summary>
/// Splits @fullString on @s_separatorForExtensionsPathSearchPaths
/// </summary>
/// FIXME: handle ; in path on Unix
private static List<string> SplitSearchPaths(string fullString) => fullString
.Split(new[] {s_separatorForExtensionsPathSearchPaths}, StringSplitOptions.RemoveEmptyEntries)
.Distinct().ToList();

/// <summary>
/// Returns true if @value might have a property reference
/// </summary>
private static bool HasPropertyReference(string value) => value.Contains("$(");

/// <summary>
/// Factory for serialization.
/// </summary>
Expand All @@ -110,4 +63,4 @@ internal static ProjectImportPathMatch FactoryForDeserialization(INodePacketTran
return new ProjectImportPathMatch(translator);
}
}
}
}
16 changes: 14 additions & 2 deletions src/Build/Definition/ToolsetConfigurationReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ internal class ToolsetConfigurationReader : ToolsetReader
/// </summary>
private bool _configurationReadAttempted = false;

/// <summary>
/// Character used to separate search paths specified for MSBuildExtensionsPath* in
/// the config file
/// </summary>
private char _separatorForExtensionsPathSearchPaths = ';';

/// <summary>
/// Cached values of tools version -> project import search paths table
/// </summary>
Expand Down Expand Up @@ -229,7 +235,13 @@ private Dictionary<string, ProjectImportPathMatch> 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;
Expand All @@ -255,4 +267,4 @@ private static Configuration ReadApplicationConfiguration()
return ConfigurationManager.OpenExeConfiguration(ConfigurationUserLevel.None);
}
}
}
}
Loading

0 comments on commit 54f30ab

Please sign in to comment.