Ensure built-in and imported targets are respected by the solution pr… #1590
Conversation
…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
| @@ -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)) | |||
There was a problem hiding this comment.
Under what circumstances is this not redundant with the filtering you do here?
There was a problem hiding this comment.
If the project name contains a . or if you have two projects with the same name (like foo.csproj and foo.vbproj) then only this method knows what the final target names will be.
I tried to explain that in the comment. Should I reword it so it makes more sense?
|
|
||
| Assert.Equal(1, instances.Length); | ||
|
|
||
| Assert.Equal(14, instances[0].TargetsCount); |
There was a problem hiding this comment.
Since it was a late regression, I'd love to see an explicit test of the after.sln.targets scenario. Would that be prohibitively hard to produce?
There was a problem hiding this comment.
Okay I added a test for this.
There was a problem hiding this comment.
LGTM. T'would be nice to have a new test like @rainersigwald suggested.
|
Retargeted to RTW. |
…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 #1587