Skip to content

File-based apps: consider args in simple csc optimization#50779

Merged
jjonescz merged 2 commits intodotnet:release/10.0.1xxfrom
jjonescz:sprint-args-simplecsc
Sep 22, 2025
Merged

File-based apps: consider args in simple csc optimization#50779
jjonescz merged 2 commits intodotnet:release/10.0.1xxfrom
jjonescz:sprint-args-simplecsc

Conversation

@jjonescz
Copy link
Member

Fixes #50778.

@jjonescz jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Sep 12, 2025
@jjonescz jjonescz marked this pull request as ready for review September 12, 2025 12:13
@jjonescz jjonescz requested review from a team and Copilot September 12, 2025 12:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fixes an issue where file-based C# apps were not properly passing command-line arguments when using the CSC optimization path. The PR ensures that when dotnet run uses the direct CSC compilation approach for file-based apps, the application arguments are correctly forwarded to the executed program.

  • Modifies the CreateCommandForCscBuiltProgram method to accept and properly escape application arguments
  • Updates the method call site to pass the ApplicationArgs parameter
  • Adds comprehensive test coverage for the CSC-only path with arguments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Cli/dotnet/Commands/Run/RunCommand.cs Updates CSC optimization path to properly handle application arguments
test/dotnet.Tests/CommandTests/Run/RunFileTests.cs Adds test cases to verify argument passing works correctly in CSC-only scenarios

@RikkiGibson RikkiGibson self-assigned this Sep 12, 2025
@jjonescz jjonescz requested review from 333fred and MiYanni September 13, 2025 09:30
@jjonescz
Copy link
Member Author

@333fred @MiYanni for reviews, thanks

@jjonescz
Copy link
Member Author

@333fred for a review of a simple change, thanks

var programPath = Path.Join(testInstance.Path, "Program.cs");

var code = $"""
#:property Configuration=Release
Copy link
Member

@333fred 333fred Sep 17, 2025

Choose a reason for hiding this comment

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

What's the purpose of this flag in this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

To ensure we are testing the "CSC-only after MSBuild" optimization. Without this directive, the program could be built with CSC directly from the start (via hard-coded CSC args). That is already tested in the other test added in this PR. With any #: directive, the program needs to be built using MSBuild first (caching the CSC args) and only subsequent rebuilds can use CSC directly.

Copy link
Member

Choose a reason for hiding this comment

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

Consider leaving a comment to that effect.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's part of a series of tests CscOnly_AfterMSBuild_* which all look similar to this one. I can add some comment to the first test, or maybe all of them, to explain it though.

@jjonescz jjonescz requested a review from 333fred September 18, 2025 07:56
@jjonescz jjonescz enabled auto-merge (squash) September 19, 2025 09:46
@jjonescz jjonescz merged commit dfe8030 into dotnet:release/10.0.1xx Sep 22, 2025
26 of 27 checks passed
@jjonescz jjonescz deleted the sprint-args-simplecsc branch September 22, 2025 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-run-file Items related to the "dotnet run <file>" effort

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants