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 
…oject 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 dotnet#1587
  • Loading branch information
@jeffkl
jeffkl committed Jan 19, 2017
1 parent dddc68c commit 5a30b3b
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 43 deletions.
58 changes: 20 additions & 38 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 @@ -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
31 changes: 26 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,14 +2106,35 @@ 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);
}

#region Helper Functions
Expand Down

0 comments on commit 5a30b3b

Please sign in to comment.