Skip to content

Commit

Permalink
Ensure built-in and imported targets are respected by the solution pr… (
Browse files Browse the repository at this point in the history 
#1590)

* Ensure built-in and imported targets are respected by the solution project generator.

The goal is to take whatever target the user specified from the command-line and add it to the metaproj.  However, we add targets and targets can be imported.

This change ensures that command-line targets are added last after everything else and they're only added if they don't already exist.  This removes the need to know what built-in targets are there.  If the user specifies /t:Clean, then it's still added as a built-in target and at the end when adding user specified targets it is ignored.

Another case is for projects.  We add a targets like "Project_Name", "Project_Name:Clean" where the project name becomes a target.  However we also add sub-targets which need to be respected.  These are also now added before the user specified targets are added.  If you specify /t:Project_Name:Clean, it won't be added to the project as a user specified target.

Unit test has been updated to hopefully test all built-in targets.

Closes #1587
  • Loading branch information
@jeffkl @AndyGerlicher
jeffkl authored and AndyGerlicher committed Jan 20, 2017
1 parent 96b94a2 commit d480ade
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 45 deletions.
62 changes: 22 additions & 40 deletions src/XMakeBuildEngine/Construction/Solution/SolutionProjectGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,6 @@ internal class SolutionProjectGenerator
"Publish"
);

/// <summary>
/// Internal implementation targets that can't be used by users
/// </summary>
internal static readonly ISet<string> _solutionGeneratedTargetNames = ImmutableHashSet.Create<string>(StringComparer.OrdinalIgnoreCase,
"ValidateSolutionConfiguration",
"ValidateToolsVersions",
"ValidateProjects",
"GetSolutionConfigurationContents",
"GetFrameworkPathAndRedistList"
).Union(_defaultTargetNames);

/// <summary>
/// Version 2.0
/// </summary>
Expand Down Expand Up @@ -162,27 +151,10 @@ IReadOnlyCollection<string> targetNames

if (targetNames != null)
{
_targetNames = GetUserTargets(targetNames, solution.ProjectsInOrder.Select(_ => _.ProjectName));
_targetNames = targetNames.Select(i => i.Split(new[] {':'}, 2, StringSplitOptions.RemoveEmptyEntries).Last()).ToList();
}
}

private List<string> GetUserTargets(IReadOnlyCollection<string> targetNames, IEnumerable<string> projectNames)
{
// Special target names are generated for each project like My_Project:Clean. If the user specified a value like
// that then we need to split by the first colon and assume the rest is a real target name. This works unless the
// target has a colon in it and the user is not trying to run it in a specific project. At the time of writing this
// we figured it unlikely that a target name would have a colon in it...

var userTargets =
targetNames.Select(i => i.Split(new[] {':'}, 2, StringSplitOptions.RemoveEmptyEntries).Last())
// The known target names are also removed from the list in case something like /t:Build was specified it is just ignored
.Except(_solutionGeneratedTargetNames, StringComparer.OrdinalIgnoreCase)
// Ignore targets that have the same name as projects, since their meaning is special. Whatever else remains, it must be a user target (e.g. the MyBuild target)
.Except(projectNames, StringComparer.OrdinalIgnoreCase).ToList();

return userTargets;
}

#endregion // Constructors

#region Methods
Expand Down Expand Up @@ -800,8 +772,9 @@ private void EvaluateAndAddProjects(List<ProjectInSolution> projectsInOrder, Lis
AddTraversalTargetForProject(traversalInstance, project, projectConfiguration, "Rebuild", "BuildOutput", canBuildDirectly);
AddTraversalTargetForProject(traversalInstance, project, projectConfiguration, "Publish", null, canBuildDirectly);

// Add any other targets specified by the user
foreach (string targetName in _targetNames)

// Add any other targets specified by the user that were not already added
foreach (string targetName in _targetNames.Where(i => !traversalInstance.Targets.ContainsKey(i)))
{
AddTraversalTargetForProject(traversalInstance, project, projectConfiguration, targetName, null, canBuildDirectly);
}
Expand All @@ -813,6 +786,12 @@ private void EvaluateAndAddProjects(List<ProjectInSolution> projectsInOrder, Lis
projectInstances.Add(metaProject);
}
}

// Add any other targets specified by the user that were not already added
foreach (string targetName in _targetNames.Where(i => !traversalInstance.Targets.ContainsKey(i)))
{
AddTraversalReferencesTarget(traversalInstance, targetName, null);
}
}

/// <summary>
Expand All @@ -828,10 +807,6 @@ private void AddStandardTraversalTargets(ProjectInstance traversalInstance, List
AddTraversalReferencesTarget(traversalInstance, "Clean", null);
AddTraversalReferencesTarget(traversalInstance, "Rebuild", "CollectedBuildOutput");
AddTraversalReferencesTarget(traversalInstance, "Publish", null);
foreach (string targetName in _targetNames)
{
AddTraversalReferencesTarget(traversalInstance, targetName, null);
}
}

/// <summary>
Expand Down Expand Up @@ -897,7 +872,7 @@ private ProjectInstance CreateTraversalInstance(string wrapperProjectToolsVersio
// These are just dummies necessary to make the evaluation into a project instance succeed when
// any custom imported targets have declarations like BeforeTargets="Build"
// They'll be replaced momentarily with the real ones.
foreach (string targetName in _defaultTargetNames.Union(_targetNames))
foreach (string targetName in _defaultTargetNames)
{
traversalProject.AddTarget(targetName);
}
Expand All @@ -922,7 +897,7 @@ private ProjectInstance CreateTraversalInstance(string wrapperProjectToolsVersio
);

// Make way for the real ones
foreach (string targetName in _defaultTargetNames.Union(_targetNames))
foreach (string targetName in _defaultTargetNames)
{
traversalInstance.RemoveTarget(targetName);
}
Expand Down Expand Up @@ -1116,7 +1091,7 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Rebuild");
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Publish");

foreach (string targetName in _targetNames)
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, targetName);
}
Expand All @@ -1132,7 +1107,7 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
AddMetaprojectTargetForManagedProject(traversalProject, metaprojectInstance, project, projectConfiguration, "Rebuild", targetOutputItemName);
AddMetaprojectTargetForManagedProject(traversalProject, metaprojectInstance, project, projectConfiguration, "Publish", null);

foreach (string targetName in _targetNames)
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForManagedProject(traversalProject, metaprojectInstance, project, projectConfiguration, targetName, null);
}
Expand All @@ -1144,7 +1119,7 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, "Rebuild", unknownProjectTypeErrorMessage);
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, "Publish", unknownProjectTypeErrorMessage);

foreach (string targetName in _targetNames)
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, targetName, unknownProjectTypeErrorMessage);
}
Expand Down Expand Up @@ -1942,6 +1917,13 @@ private void AddTraversalTargetForProject(ProjectInstance traversalProject, Proj
actualTargetName += ":" + targetToBuild;
}

// Don't add the target again. The user might have specified /t:Project:target which was already added but only this method knows about Project:Target so
// after coming up with that target name, it can check if it has already been added.
if (traversalProject.Targets.ContainsKey(actualTargetName))
{
return;
}

// The output item name is the concatenation of the project name with the specified outputItem. In the typical case, if the
// project name is MyProject, the outputItemName will be MyProjectBuildOutput, and the outputItemAsItem will be @(MyProjectBuildOutput).
// In the case where the project contains characters not allowed as Xml element attribute values, those characters will
Expand Down
87 changes: 82 additions & 5 deletions src/XMakeBuildEngine/UnitTests/Construction/SolutionProjectGenerator_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2106,13 +2106,90 @@ public void IllegalUserTargetNamesDoNotThrow()
EndGlobal
");

// "GetFrameworkPathAndRedistList" is for web projects only
var illegalTargetNamesForCsproj = SolutionProjectGenerator._solutionGeneratedTargetNames.Union(new []{"ClassLibrary1"}).Except(new []{ "GetFrameworkPathAndRedistList" }).ToList();
ProjectInstance[] instances = SolutionProjectGenerator.Generate(solution, null, null, BuildEventContext.Invalid, null, illegalTargetNamesForCsproj);
ProjectInstance[] instances;

foreach (var illegalTargetName in illegalTargetNamesForCsproj)
foreach (string builtInTargetName in new[]
{
Assert.Equal(1, instances[0].Targets.Count(target => String.Compare(target.Value.Name, illegalTargetName, StringComparison.OrdinalIgnoreCase) == 0));
null,
"Build",
"Rebuild",
"Clean",
"Publish",
"ClassLibrary1",
"ClassLibrary1:Clean",
"ClassLibrary1:Rebuild",
"GetSolutionConfigurationContents",
"ValidateProjects",
})
{
instances = SolutionProjectGenerator.Generate(solution, null, null, BuildEventContext.Invalid, null, builtInTargetName == null ? null : new [] { builtInTargetName });

Assert.Equal(1, instances.Length);

Assert.Equal(12, instances[0].TargetsCount);
}


instances = SolutionProjectGenerator.Generate(solution, null, null, BuildEventContext.Invalid, null, new[] { "Foo" });

Assert.Equal(1, instances.Length);

Assert.Equal(14, instances[0].TargetsCount);
}

/// <summary>
/// Verifies that when a user has an after.solution.sln.targets that the targets are not overridden by the solution project generator.
/// </summary>
[Fact]
public void AfterTargetsComeFromImport()
{
string baseDirectory = Guid.NewGuid().ToString("N");

string solutionFilePath = ObjectModelHelpers.CreateFileInTempProjectDirectory(Path.Combine(baseDirectory, $"{Guid.NewGuid():N}.sln"), @"
Microsoft Visual Studio Solution File, Format Version 14.00
# Visual Studio 2015
Project(""{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}"") = ""ClassLibrary1"", ""ClassLibrary1.csproj"", ""{6185CC21-BE89-448A-B3C0-D1C27112E595}""
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{6185CC21-BE89-448A-B3C0-D1C27112E595}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{6185CC21-BE89-448A-B3C0-D1C27112E595}.Debug|Any CPU.Build.0 = Debug|Any CPU
{6185CC21-BE89-448A-B3C0-D1C27112E595}.Release|Any CPU.ActiveCfg = Release|Any CPU
{6185CC21-BE89-448A-B3C0-D1C27112E595}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal
");

ObjectModelHelpers.CreateFileInTempProjectDirectory(Path.Combine(baseDirectory, $"after.{Path.GetFileName(solutionFilePath)}.targets"), @"
<Project ToolsVersion=""msbuilddefaulttoolsversion"" xmlns=""msbuildnamespace"">
<Target Name=""MyTarget"">
<MyTask />
</Target>
</Project>");

try
{
var solutionFile = SolutionFile.Parse(solutionFilePath);

ProjectInstance projectInstance = SolutionProjectGenerator.Generate(solutionFile, null, null, BuildEventContext.Invalid, null, new[] { "MyTarget" }).FirstOrDefault();

Assert.NotNull(projectInstance);

Assert.True(projectInstance.Targets.ContainsKey("MyTarget"));

Assert.Equal(1, projectInstance.Targets["MyTarget"].Children.Count);

ProjectTaskInstance task = Assert.IsType<ProjectTaskInstance>(projectInstance.Targets["MyTarget"].Children[0]);

Assert.Equal("MyTask", task.Name);
}
finally
{
ObjectModelHelpers.DeleteTempProjectDirectory();
}
}

Expand Down

0 comments on commit d480ade

Please sign in to comment.