-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Set EnableDefaultEmbeddedResourceItems and EnableDefaultNoneItems to false for file-based apps using default SDK #51542
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
Set EnableDefaultEmbeddedResourceItems and EnableDefaultNoneItems to false for file-based apps using default SDK #51542
Conversation
…eDefaultNoneItems to false for file-based apps Co-authored-by: jjonescz <[email protected]>
…false for file-based apps with default SDK Co-authored-by: jjonescz <[email protected]>
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 changes the default behavior for file-based programs to exclude embedded resources and "None" items when using the default SDK (Microsoft.NET.Sdk). This prevents unexpected file inclusions (like .resx files) in simple file-based apps while preserving the default SDK behavior for non-default SDKs like Microsoft.NET.Sdk.Web.
- Modifies virtual project generation to conditionally set
EnableDefaultEmbeddedResourceItemsandEnableDefaultNoneItemstofalseonly when using the default SDK - Updates tests to reflect the new behavior and verify both default SDK and non-default SDK scenarios
- Adds documentation explaining the new customization
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs | Adds logic to conditionally disable default embedded resource and None items for the default SDK |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Updates tests to verify new behavior, adds explicit property settings where needed, removes obsolete test |
| test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs | Updates tests to explicitly enable default items or use non-default SDK where needed |
| documentation/general/dotnet-run-file.md | Documents the new behavior for default embedded resource and None items |
Comments suppressed due to low confidence (1)
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs:1
- Corrected double negative 'doesn't support neither' to 'doesn't support either'.
// Licensed to the .NET Foundation under one or more agreements.
| // Only set these to false when using the default SDK with no additional SDKs | ||
| // to avoid including .resx and other files that are typically not expected in simple file-based apps. | ||
| // When other SDKs are used (e.g., Microsoft.NET.Sdk.Web), keep the default behavior. | ||
| bool usingOnlyDefaultSdk = firstSdkName == defaultSdkName && sdkDirectives.Count() <= 1; |
Copilot
AI
Nov 4, 2025
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 condition sdkDirectives.Count() <= 1 is counterintuitive. When there is exactly 1 SDK directive and it's the default SDK, this should still be treated as "only default SDK". Consider changing to sdkDirectives.Count() == 0 || (sdkDirectives.Count() == 1 && firstSdkName == defaultSdkName) or simplify to !sdkDirectives.Skip(1).Any() since when firstSdkName == defaultSdkName, either there are no directives (count=0) or the first one is the default SDK (and we want to check there are no additional ones).
| bool usingOnlyDefaultSdk = firstSdkName == defaultSdkName && sdkDirectives.Count() <= 1; | |
| bool usingOnlyDefaultSdk = sdkDirectives.Count() == 0 || (sdkDirectives.Count() == 1 && firstSdkName == defaultSdkName); |
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 current condition seems simpler and more robust to me.
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.
Why can't it just be sdkDirectives is [] or [defaultSdkName]?
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.
sdkDirectives is an IEnumerable so it cannot be used with list patterns
|
@333fred @RikkiGibson @MiYanni for reviews please, we might want to take this through servicing |
|
/backport to release/10.0.1xx |
|
Started backporting to |
Simple file-based apps unexpectedly include
**/*.resxfiles and other default items, causing issues when scripts are placed in repository roots or directories with unrelated project files (refs #51494, #49826, #12546).Changes
EnableDefaultEmbeddedResourceItemsandEnableDefaultNoneItemstofalsewhen using onlyMicrosoft.NET.Sdk(no additional SDK directives)Microsoft.NET.Sdk.Web) retain default item inclusion behavior#:property EnableDefaultEmbeddedResourceItems=trueExample
Test Updates
Updated
RunFileTeststo verify:Original prompt
EnableDefaultEmbeddedResourceItemstofalseby default for file-based apps #51506💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.