-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Enable building in-memory projects #12156
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for building in-memory or non-existent projects by opt-in via a new global property, avoiding a breaking change.
- Introduces a check for the
BuildNonexistentProjectsByDefault
global property to default toBuild
behavior - Adds equivalent logic in the backend intrinsic
MSBuild
task - Adds unit tests covering in-memory project scenarios with explicit and default behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Tasks/MSBuild.cs | Checks BuildNonexistentProjectsByDefault to set skip behavior to Build |
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs | Mirrors the same property check in the request builder |
src/Tasks.UnitTests/MSBuild_Tests.cs | Adds tests for explicit Build, explicit Error, and default Build scenarios |
Comments suppressed due to low confidence (3)
src/Tasks/MSBuild.cs:300
- [nitpick] The property name uses 'Nonexistent' in one word, while the enum
SkipNonExistentProjectsBehavior
uses 'NonExistent'. Consider aligning the casing and spelling (e.g., 'BuildNonExistentProjectsByDefault') for consistency.
.TryGetValue("BuildNonexistentProjectsByDefault", out var buildNonexistentProjectsByDefault) &&
src/Tasks/MSBuild.cs:299
- Add XML documentation or inline comments for the new
BuildNonexistentProjectsByDefault
global property in the MSBuild task, explaining its purpose and usage.
else if (BuildEngine is IBuildEngine6 buildEngine6 && buildEngine6.GetGlobalProperties()
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/MSBuild.cs:333
- Add unit tests for the backend
MSBuild
intrinsic task to verify thatBuildNonexistentProjectsByDefault
correctly triggersSkipNonExistProjectsBehavior.Build
in the request builder scenario.
else if (BuildEngine is IBuildEngine6 buildEngine6 && buildEngine6.GetGlobalProperties()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the property should be documented in src\MSBuild\MSBuild\Microsoft.Build.CommonTypes.xsd
please address comments, otherwise looks good
missed this comment, looking now |
This reverts commit 3eed861.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach, implementation, and tests look pretty good.
Could you please write up a speclet describing the change in our documentation/specs
folder, from the MSBuild perspective? Or link to a larger file-based apps spec that mentions it in the PR description.
Fixes #12058 via a global property opt-in (to avoid a breaking change).
To be used by file-based apps: dotnet/sdk#49745