Exclude default items from file-based apps in project dirs #49999

jjonescz · 2025-07-30T15:58:05Z

Resolves #49826.

baronfel · 2025-07-30T16:02:42Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets

+      ('$([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.sln&quot;))' != '' Or
+      $([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.slnx&quot;)) != '' Or
+      $([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.*proj&quot;)) != '')">false</EnableDefaultItems>


These calls return arrays, so checking Length != 0 might be more clear here, and wouldn't require MSBuild to coerce the return to a string for checking.

@rainersigwald one thing I think about with IO-based property functions - do these go through MSBuild's IO abstractions, or are they direct BCL calls? Are there things we need to worry about in general with the IO calls in MSBuild logic with multithreaded MSBuild (I know we want to prevent the use of some of these calls in Tasks)?

I know we have code to not force an item expansion to stringify when compared to empty string, but I don't know if it works in this property-function case.

I think today we do straight passthrough to BCL but I wouldn't want to stop us from doing something better in the future.

But evaluation is already parallel in many cases and calling this should be fine (as long as you don't like depend on mutating filesystem state between projects).

Copilot

Pull Request Overview

This PR resolves an issue where file-based apps would incorrectly include default items (like .resx files and .cs files) when running in directories that contain project or solution files. The fix introduces a new property DisableDefaultItemsInProjectFolder that automatically disables default items when project/solution files are detected in the same directory.

Key changes:

Adds logic to automatically disable default items in project folders for file-based apps
Updates test coverage to verify the new behavior with embedded resources and default items
Refactors existing test code to use shared constants for better maintainability

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets`	Implements the core logic to disable default items when project/solution files are detected
`src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs`	Sets the `DisableDefaultItemsInProjectFolder` property for file-based programs
`test/dotnet.Tests/CommandTests/Run/RunFileTests.cs`	Adds test coverage and refactors existing tests to use shared constants
`test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs`	Adds test coverage for the conversion scenario
`documentation/general/dotnet-run-file.md`	Updates documentation to describe the new behavior

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets

test/dotnet.Tests/CommandTests/Run/RunFileTests.cs

RikkiGibson · 2025-07-30T16:54:12Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets

+      '$(DisableDefaultItemsInProjectFolder)' == 'true' And
+      ('$([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.sln&quot;).Length)' != '0' Or
+      '$([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.slnx&quot;).Length)' != '0' Or
+      '$([System.IO.Directory]::GetFiles($(MSBuildProjectDirectory), &quot;*.*proj&quot;).Length)' != '0')">false</EnableDefaultItems>


I would feel more comfortable with identifying the exact set of extensions we want to condition the behavior on. For example, xcode projects use extension .pbxproj. I wouldn't expect us to change our behavior based on the presence of it.

Note that this is based on the existing logic here:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets

Line 41 in f629fe6

<DefaultItemExcludes>$(DefaultItemExcludes);**/*.*proj</DefaultItemExcludes>

But I guess that's different.

For example, xcode projects use extension .pbxproj. I wouldn't expect us to change our behavior based on the presence of it.

I'm not sure why not, file-based apps shouldn't be in project folders anyway.

I guess I am saying that if there is some proj file present, whose exact extension we don't actually know, we just don't know what it means. If there were some multimedia software, where the project format ends with proj, which the user had some reason for including in a folder with a FBP, it doesn't feel reasonable for us to condition our behavior based on that, for the same reason as the xcode case.

Like, basically imagine the bug report where someone says I tried putting my "completely unrelated to dotnet/msbuild" file into this folder and now my FBP won't build. How were they supposed to reason that this is something that would/should happen?

Makes sense, I can change to specific extensions, thanks. But still, handling the whole *proj pattern is common in .net sdk anyway, see https://github.com/search?q=repo%3Adotnet%2Fsdk%20%22*.*proj%22&type=code

It's good to know that we don't protect against this possibility in general. For example it might be the case today that having a random unrelated *proj file, breaks dotnet build . in that folder. In which case I'm ok with you deciding to defend against this case or not at your discretion.

documentation/general/dotnet-run-file.md

rainersigwald · 2025-07-30T17:24:11Z

documentation/general/dotnet-run-file.md


+  - `DisableDefaultItemsInProjectFolder` property is set to `true` which results in `EnableDefaultItems=false` by default
+    in case there is a project or solution in the same directory as the file-based app.
+    This ensures that items from nested projects and artifacts are not included by the app.


I think I'm confused by the need to probe for project/solutions. In this new mode how does the c# file get included in the project if not via default glob? If it's explicitly included, why is the probing needed?

Yes, the .cs file is included explicitly (that might change in .net 11 with multi-file mode though). The globbing is still used for other default items like .resx (EmbeddedResource) or .json (Content in web apps).

jaredpar · 2025-08-04T05:42:51Z

@JoeRobich, @RikkiGibson PTAL

RikkiGibson · 2025-08-04T23:19:53Z

test/dotnet.Tests/CommandTests/Run/RunFileTests.cs

+        var testInstance = _testAssetsManager.CreateTestDirectory();
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), s_programReadingEmbeddedResource);
+        File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx);
+        File.WriteAllText(Path.Join(testInstance.Path, "repo.sln"), "");


Consider also including cases for the other file types .csproj .slnx we have this behavior for, as well as a file type where we don't exclude default items but may consider doing so in future, such as .shproj.

RikkiGibson

LGTM, just one minor suggestion, non-blocking

jjonescz · 2025-08-11T09:30:49Z

@MiYanni for a review, thanks

Exclude default items from file-based apps in project dirs

8517d5b

jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Jul 30, 2025

baronfel reviewed Jul 30, 2025

View reviewed changes

Avoid coercing arrays to strings

f629fe6

jjonescz marked this pull request as ready for review July 30, 2025 16:48

jjonescz requested review from a team and Copilot July 30, 2025 16:48

Copilot AI reviewed Jul 30, 2025

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets Outdated Show resolved Hide resolved

test/dotnet.Tests/CommandTests/Run/RunFileTests.cs Show resolved Hide resolved

test/dotnet.Tests/CommandTests/Run/RunFileTests.cs Show resolved Hide resolved

RikkiGibson reviewed Jul 30, 2025

View reviewed changes

documentation/general/dotnet-run-file.md Show resolved Hide resolved

rainersigwald reviewed Jul 30, 2025

View reviewed changes

Check only csproj for starters

c0e4212

JoeRobich approved these changes Aug 4, 2025

View reviewed changes

RikkiGibson reviewed Aug 4, 2025

View reviewed changes

RikkiGibson approved these changes Aug 4, 2025

View reviewed changes

Extend tests

781baab

MiYanni approved these changes Aug 12, 2025

View reviewed changes

RikkiGibson approved these changes Aug 12, 2025

View reviewed changes

jjonescz merged commit c2d964b into dotnet:main Aug 13, 2025
27 checks passed

jjonescz deleted the sprint-sln branch August 13, 2025 07:47

Exclude default items from file-based apps in project dirs #49999

Exclude default items from file-based apps in project dirs #49999

Conversation

jjonescz commented Jul 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredpar commented Aug 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson left a comment

Choose a reason for hiding this comment

Uh oh!

jjonescz commented Aug 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants