Add basic support for 'dotnet run file.cs' #46915

jjonescz · 2025-02-18T14:21:30Z

Copilot

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/Cli/dotnet/commands/dotnet-run/RunCommand.cs:609

[nitpick] The method 'DiscoverProjectFilePath' now returns both a project file path and an entry point file via an out parameter; consider renaming it (for example, to 'DiscoverProjectAndEntryPointPaths') to better reflect its dual purpose.

private static string? DiscoverProjectFilePath(string? projectFileOrDirectoryPath, ref string[] args, out string? entryPointFilePath)

src/Cli/dotnet/commands/dotnet-run/RunCommand.cs:250

[nitpick] Consider renaming the 'projectFactory' out parameter to 'projectInstanceFactory' for improved clarity since it is intended to supply a ProjectInstance.

private void EnsureProjectIsBuilt(out Func<ProjectCollection, ProjectInstance>? projectFactory)

src/Cli/dotnet/commands/dotnet-run/RunCommand.cs

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

jjonescz · 2025-02-25T16:19:47Z

@MiYanni @RikkiGibson @chsienki for reviews, thanks

src/Cli/dotnet/commands/dotnet-run/Program.cs

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

baronfel · 2025-02-26T16:52:14Z

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

+    public int Execute(string[] binaryLoggerArgs, LoggerVerbosity verbosity)
+    {
+        var binaryLogger = GetBinaryLogger(binaryLoggerArgs);
+        var consoleLogger = new ConsoleLogger(verbosity);


nit: should use new MSBuild API from @MichalPavlik to auto-detect the use of the Console Logger vs the Terminal Logger.

Can you elaborate? I couldn't find such API.

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

baronfel · 2025-02-26T16:55:30Z

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

+
+                <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
+
+                <!-- Override targets which don't work with project files that are not present on disk. -->


Ok now this is some deep magic. It looks like all of this is Restore-time stuff, do we need to add some kinds of flags/detections to enable in-memory use cases for real instead of requiring these workarounds?

cc @rainersigwald / @nkolev92

It feels strange that an xml string is needed to make an in-memory project, when we are using the msbuild APIs directly for so many other things.

It feels strange that an xml string is needed to make an in-memory project, when we are using the msbuild APIs directly for so many other things.

Isn't that same with strings containing C# that you need when using Roslyn APIs? You could create SyntaxTrees manually but no one does that. Similarly, I could create the MSBuild project without the string but seems that would be less readable.

I think I am relatively comfortable with it, especially if it remains as "just a string" and we don't start using a complex interpolated string with XML escaping handling, etc. to make the project.

Ok now this is some deep magic. It looks like all of this is Restore-time stuff, do we need to add some kinds of flags/detections to enable in-memory use cases for real instead of requiring these workarounds?

Yes, ideally the targets would allow in-memory builds. Should I file an issue for it at https://github.com/NuGet/Home since the targets live there I think?

These are in obj folder right? That still exists (I imagine it would be much more difficult to get rid of that), so I don't think this should be a problem.

not quite - the big problem is that a key file in the obj folder is not 'scoped' to a single project - it's called project.assets.json. This name is customizable though, so your in-memory project here should set that property so that we're future-proof for the "multiple apps in one directory" scenario. That property name is ProjectAssetsFile.

You probably want to make sure that they synthetic project name is unique-per-app too to account for MSBuild logic that does things like $(MSBuildProjectName).foo.bar.

Can you help me understand why is the scoping of the file name a problem?

@jjonescz I think it's a good to consider some sort of a design review for #47020.

It involves NuGet gestures, but the NuGet team hasn't really had a chance to look into that yet.
I've tagged the appropriate people, but it's a bit of short notice for everyone to fully wrap their heads around what's going on.

@nkolev92 when I was looking at the ProjectAssetsFile logic in the targets, right next to it I saw other logic that was using the MSBuildProjectName to compute an intermediate file path - to me that implies that we need to be careful to ensure each 'app' has a distinct project name so that intermediates could reasonably be created in the same intermediate directory without clobbering

Scoping could be a problem if we allow multiple virtual projects in one folder, but the proposal currently says they would get separate artifacts folders, so that should avoid any problems.

short notice

I think it is expected that the implementation details of this feature might change over time, so there is no rush.

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

src/Cli/dotnet/commands/dotnet-run/RunCommand.cs

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

RikkiGibson · 2025-02-27T01:12:19Z

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

+
+                <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
+
+                <!-- Override targets which don't work with project files that are not present on disk. -->


It feels strange that an xml string is needed to make an in-memory project, when we are using the msbuild APIs directly for so many other things.

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

nkolev92 · 2025-02-27T18:22:12Z

src/Cli/dotnet/commands/VirtualProjectBuildingCommand.cs

+
+                <!--
+                    Override targets which don't work with project files that are not present on disk.
+                    See https://github.com/NuGet/Home/issues/14148.


What if we call static graph restore?
Is this still a problem?
https://learn.microsoft.com/en-us/nuget/reference/msbuild-targets#restoring-with-msbuild-static-graph-evaluation

cc @jeffkl

static graph is not ever going to be a thing with this loose-file usage mode.

Static graph restore never triggers any of the target being reported as a problem, so I thought it'd just skip the problem.
Why do we have a concern with static graph?

Add basic support for 'dotnet run file.cs'

aea7240

dotnet-issue-labeler bot added Area-CLI untriaged Request triage from a team member labels Feb 18, 2025

Fix tests on case sensitive OSes

8094d49

jjonescz force-pushed the sprint branch from e62e7f5 to 8094d49 Compare February 18, 2025 17:24

jjonescz requested review from DamianEdwards, baronfel and jaredpar February 19, 2025 12:15

jjonescz marked this pull request as ready for review February 19, 2025 12:15

Copilot bot review requested due to automatic review settings February 19, 2025 12:15

Copilot AI reviewed Feb 19, 2025

View reviewed changes

src/Cli/dotnet/commands/dotnet-run/RunCommand.cs Show resolved Hide resolved

jaredpar requested review from chsienki and RikkiGibson February 19, 2025 22:27

jaredpar reviewed Feb 19, 2025

View reviewed changes

RikkiGibson self-assigned this Feb 19, 2025

jjonescz added 2 commits February 20, 2025 09:53

Share binlog arg detection code

9f51756

Nullable-enable RunCommand.cs

5a5206f

jjonescz force-pushed the sprint branch from d36a366 to 5a5206f Compare February 20, 2025 11:53

This was referenced Feb 20, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

dotnet-watch tests failing #40006

Open

[dotnet watch] Cannot open project Lib.fsproj because extension fsproj is not associated with a language #46378

Open

jaredpar requested a review from MiYanni February 20, 2025 19:13

Test embedded resource

254eccb

jjonescz mentioned this pull request Feb 21, 2025

Add spec for dotnet run file.cs #47020

Open

Merge branch 'main' into sprint

aa913b0

baronfel reviewed Feb 26, 2025

View reviewed changes

src/Cli/dotnet/commands/dotnet-run/Program.cs Outdated Show resolved Hide resolved

baronfel reviewed Feb 26, 2025

View reviewed changes

RikkiGibson reviewed Feb 27, 2025

View reviewed changes

Allow --no-build

fe91ade

jjonescz added 3 commits February 27, 2025 14:40

Allow --launch-profile

5c873d1

Improve binlog option parsing

da30f3e

Improve code

fcd9a92

jjonescz mentioned this pull request Feb 27, 2025

Restoring in-memory projects NuGet/Home#14148

Open

jjonescz added 2 commits February 27, 2025 19:09

Fix test to work on Linux

eb2a1e8

Link related issues

d08a1be

nkolev92 reviewed Feb 27, 2025

View reviewed changes

baronfel added Area-run-file Items related to the "dotnet run <file>" effort and removed untriaged Request triage from a team member Area-CLI labels Feb 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic support for 'dotnet run file.cs' #46915

Add basic support for 'dotnet run file.cs' #46915

jjonescz commented Feb 18, 2025 •

edited

Loading

Copilot AI left a comment

jjonescz commented Feb 25, 2025

baronfel Feb 26, 2025

jjonescz Feb 27, 2025

baronfel Feb 26, 2025

RikkiGibson Feb 27, 2025

jjonescz Feb 27, 2025 •

edited

Loading

RikkiGibson Feb 27, 2025

jjonescz Feb 27, 2025

jjonescz Feb 28, 2025

baronfel Feb 28, 2025

nkolev92 Feb 28, 2025

baronfel Feb 28, 2025

jjonescz Feb 28, 2025

RikkiGibson Feb 27, 2025

nkolev92 Feb 27, 2025 •

edited

Loading

baronfel Feb 27, 2025

nkolev92 Feb 28, 2025


		<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />

		<!-- Override targets which don't work with project files that are not present on disk. -->

Add basic support for 'dotnet run file.cs' #46915

Are you sure you want to change the base?

Add basic support for 'dotnet run file.cs' #46915

Conversation

jjonescz commented Feb 18, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

jjonescz commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjonescz Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkolev92 Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjonescz commented Feb 18, 2025 •

edited

Loading

jjonescz Feb 27, 2025 •

edited

Loading

nkolev92 Feb 27, 2025 •

edited

Loading