Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Building a subset of a VS solution at the command line causes MSB4057 #6465

Merged
merged 3 commits into from May 20, 2021
Merged

Building a subset of a VS solution at the command line causes MSB4057 #6465

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b00d82c
Do not use Exclude
Forgind May 19, 2021
94ce625
Add test
Forgind May 19, 2021
d6a27dd
Bump version to 16.10.1
rainersigwald May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs
View file
Open in desktop
Expand Up @@ -175,6 +175,46 @@ public void SolutionFilterFiltersProjects()
}
}

[Fact]
public void BuildProjectAsTarget()
{
using (TestEnvironment testEnvironment = TestEnvironment.Create())
{
TransientTestFolder folder = testEnvironment.CreateFolder(createFolder: true);
TransientTestFolder classLibFolder = testEnvironment.CreateFolder(Path.Combine(folder.Path, "classlib"), createFolder: true);
TransientTestFile classLibrary = testEnvironment.CreateFile(classLibFolder, "classlib.csproj",
@"<Project>
<Target Name=""ClassLibraryTarget"">
<Message Text=""ClassLibraryBuilt""/>
</Target>
</Project>
");

TransientTestFolder simpleProjectFolder = testEnvironment.CreateFolder(Path.Combine(folder.Path, "simpleProject"), createFolder: true);
TransientTestFile simpleProject = testEnvironment.CreateFile(simpleProjectFolder, "simpleProject.csproj",
@"<Project>
<Target Name=""SimpleProjectTarget"">
<Message Text=""SimpleProjectBuilt""/>
</Target>
</Project>
");

TransientTestFile solutionFile = testEnvironment.CreateFile(folder, "testFolder.sln",
@"
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
VisualStudioVersion = 16.6.30114.105
MinimumVisualStudioVersion = 10.0.40219.1
Project(""{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}"") = ""simpleProject"", ""simpleProject\simpleProject.csproj"", ""{AA52A05F-A9C0-4C89-9933-BF976A304C91}""
EndProject
Project(""{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}"") = ""classlib"", ""classlib\classlib.csproj"", ""{80B8E6B8-E46D-4456-91B1-848FD35C4AB9}""
EndProject
");
RunnerUtilities.ExecMSBuild(solutionFile.Path + " /t:classlib", out bool success);
success.ShouldBeTrue();
}
}

/// <summary>
/// Verify the AddNewErrorWarningMessageElement method
/// </summary>
Expand Down
13 changes: 7 additions & 6 deletions src/Build/Construction/Solution/SolutionProjectGenerator.cs
View file
Open in desktop
Expand Up @@ -781,8 +781,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 that were not already added
foreach (string targetName in _targetNames.Except(traversalInstance.Targets.Keys, StringComparer.OrdinalIgnoreCase))
// Add any other targets specified by the user that were not already added. A target's presence or absence must be determined at the last
// minute because whether traversalInstance.Targets.ContainsKey(i) is true or not can change during the enumeration.
foreach (string targetName in _targetNames.Where(i => !traversalInstance.Targets.ContainsKey(i)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we extract method that returns the enumerable? And use it in four places? And add a detailed comment explaining that it has to be lazy because evidently the traversalInstance.Targets contents changes during enumeration? It's a tricky spot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also need to pay attention to how Keys behaves - is it returning the live collection or a snapshot of the keys?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract method might be dicey - maybe better keep as is for simplicity... just add comments? You decide

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want this to be as small of a change and as safe of a change as possible, since this is going into 16.10 rather than 17.0. This is a straight revert of the relevant part of the commit that caused the issue, which is safest in my opinion. Adding a comment is a good plan, though—I'll just add it for the first case, since that's the only one that I know should be just in time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on keeping this to a revert. Can extract as follow up in a future milestone if that's better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let’s not extract, it was just a knee jerk reaction of seeing the changes in four places. All good!

{
AddTraversalTargetForProject(traversalInstance, project, projectConfiguration, targetName, null, canBuildDirectly);
}
Expand All @@ -796,7 +797,7 @@ private void EvaluateAndAddProjects(List<ProjectInSolution> projectsInOrder, Lis
}

// Add any other targets specified by the user that were not already added
foreach (string targetName in _targetNames.Except(traversalInstance.Targets.Keys, StringComparer.OrdinalIgnoreCase))
foreach (string targetName in _targetNames.Where(i => !traversalInstance.Targets.ContainsKey(i)))
{
AddTraversalReferencesTarget(traversalInstance, targetName, null);
}
Expand Down Expand Up @@ -1201,7 +1202,7 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Rebuild");
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Publish");

foreach (string targetName in _targetNames.Except(metaprojectInstance.Targets.Keys, StringComparer.OrdinalIgnoreCase))
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, targetName);
}
Expand All @@ -1221,7 +1222,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.Except(metaprojectInstance.Targets.Keys, StringComparer.OrdinalIgnoreCase))
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForManagedProject(traversalProject, metaprojectInstance, project, projectConfiguration, targetName, null);
}
Expand All @@ -1233,7 +1234,7 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, "Rebuild", unknownProjectTypeErrorMessage);
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, "Publish", unknownProjectTypeErrorMessage);

foreach (string targetName in _targetNames.Except(metaprojectInstance.Targets.Keys, StringComparer.OrdinalIgnoreCase))
foreach (string targetName in _targetNames.Where(i => !metaprojectInstance.Targets.ContainsKey(i)))
{
AddMetaprojectTargetForUnknownProjectType(traversalProject, metaprojectInstance, project, targetName, unknownProjectTypeErrorMessage);
}
Expand Down