Allow publishing file-based apps #49310
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for publishing file‐based apps by updating argument parsing, commands, and tests.
- Updated command help and argument parsing to allow file-based inputs.
- Added tests for file-based publishing and adjusted build/publish commands to support the new scenario.
- Updated documentation to reflect the new support for file-based publish.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CompletionTests/snapshots/zsh/DotnetCliSnapshotTests.VerifyCompletions.verified.zsh | Updated completion help text to include FILE as a valid input. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Added new tests covering publishing file-based apps, including artifact verification. |
| test/dotnet.Tests/Project/Convert/DotnetProjectConvertTests.cs | Adjusted conversion tests to ignore the PublishAot attribute differences. |
| test/dotnet.Tests/CommandTests/MSBuild/MSBuildArgumentCommandLineParserTests.cs | Made minor type cast adjustments for consistency. |
| test/dotnet.Tests/CommandTests/MSBuild/GivenDotnetPublishInvocation.cs | Applied explicit casting to ensure consistent PublishCommand behavior. |
| src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs | Changed msbuildArgs type and introduced a BuildTarget property to improve target flexibility. |
| src/Cli/dotnet/Commands/Run/CommonRunHelpers.cs | Updated parameter type for user-passed properties. |
| src/Cli/dotnet/Commands/Publish/PublishCommandParser.cs | Renamed and updated argument to accept project, solution, or file inputs. |
| src/Cli/dotnet/Commands/Publish/PublishCommand.cs | Updated command logic to support file-based publish scenarios and adjusted argument insertion. |
| src/Cli/dotnet/Commands/Build/BuildCommand.cs | Modified build target selection by replacing NoIncremental with BuildTarget usage. |
| documentation/general/dotnet-run-file.md | Revised documentation to mention support for 'dotnet publish file.cs'. |
Comments suppressed due to low confidence (2)
src/Cli/dotnet/Commands/Publish/PublishCommand.cs:51
- The pattern matching here assumes exactly one non-binlog argument. Consider whether multiple file arguments require explicit handling or should be rejected to avoid ambiguity.
if (nonBinLogArgs is [{ } arg] && VirtualProjectBuildingCommand.IsValidEntryPointPath(arg))
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs:173
- Switching from a boolean 'NoIncremental' flag to a string-based 'BuildTarget' offers increased flexibility; please verify that scenarios requiring a 'Rebuild' behave as expected and are well covered by tests.
targetsToBuild: [BuildTarget ?? "Build"]
| @@ -305,6 +305,8 @@ but that would require changes to the native dotnet host. | |||
| ### Other commands | |||
|
|
|||
| Commands `dotnet restore file.cs` and `dotnet build file.cs` are needed for IDE support and hence work for file-based programs. | |||
| Command `dotnet publish file.cs` is also supported for file-based programs. | |||
There was a problem hiding this comment.
Should we mention here that this default to AOT and how to override that if it's not desirable?
| public VerbosityOptions Verbosity { get; } | ||
| public bool NoRestore { get; init; } | ||
| public bool NoCache { get; init; } | ||
| public bool NoBuild { get; init; } | ||
| public bool NoIncremental { get; init; } | ||
| public string? BuildTarget { get; init; } |
There was a problem hiding this comment.
| public string? BuildTarget { get; init; } | |
| public string BuildTarget { get; init; } = "Build"; |
That would remove an annotation but get the same result.
| @@ -170,7 +170,7 @@ public override int Execute() | |||
| { | |||
| var buildRequest = new BuildRequestData( | |||
| CreateProjectInstance(projectCollection), | |||
| targetsToBuild: [NoIncremental ? "Rebuild" : "Build"]); | |||
| targetsToBuild: [BuildTarget ?? "Build"]); | |||
There was a problem hiding this comment.
| targetsToBuild: [BuildTarget ?? "Build"]); | |
| targetsToBuild: [BuildTarget]); |
Could do this if you take the above suggestion.
| @@ -540,6 +540,7 @@ public static void WriteProjectFile( | |||
| <TargetFramework>net10.0</TargetFramework> | |||
| <ImplicitUsings>enable</ImplicitUsings> | |||
| <Nullable>enable</Nullable> | |||
| <PublishAot>true</PublishAot> | |||
There was a problem hiding this comment.
Should we unconditionally do this? What if the user has the following code?
#:property PublishAot falseThere was a problem hiding this comment.
The project will get <PublishAot>false</PublishAot> later in the file, overriding the default. There is also #49177 to remove the default in case the directive is present (just to make the project file cleaner but functionally nothing should change).
|
|
||
| bool noRestore = noBuild || parseResult.HasOption(PublishCommandParser.NoRestoreOption); | ||
|
|
||
| if (nonBinLogArgs is [{ } arg] && VirtualProjectBuildingCommand.IsValidEntryPointPath(arg)) |
There was a problem hiding this comment.
@chsienki has a PR out to remove the need for the .cs suffix in the files. Think this needs to adjust to account for that.
There was a problem hiding this comment.
It shouldn't need any adjusting - Chris's logic lives inside VirtualProjectBuildingCommand.IsValidEntryPointPath so this will pick it up automatically - and hence dotnet publish no-cs-extension will work (and similarly others like build/restore) - assuming we want that?
There was a problem hiding this comment.
Def want it to work. I just want to make sure we're confident these PRs will converge correctly.
|
@dotnet/run-file for reviews, thanks |
|
@MiYanni for a review, thanks |
Resolves #49189.