Support dotnet pack file.cs
#50168
Support dotnet pack file.cs
#50168
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for dotnet pack file.cs to enable packing file-based programs directly from C# source files. The change allows users to create NuGet packages from standalone C# files without requiring a traditional project file structure.
Key changes:
- Modified the
PackCommandto detect and handle file-based programs using the virtual project building infrastructure - Updated command completion descriptions to include C# files as valid inputs
- Added test coverage for the new functionality
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/Commands/Pack/PackCommand.cs | Refactored to use CommandFactory pattern and support file-based programs through VirtualProjectBuildingCommand |
| src/Cli/dotnet/Commands/Pack/PackCommandParser.cs | Updated argument name and description to include C# files as valid inputs |
| src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs | Added PackageOutputPath property and updated runtime config logic to exclude Pack target |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Added comprehensive test for pack functionality and updated virtual project templates |
| test/dotnet.Tests/CommandTests/MSBuild/GivenDotnetPackInvocation.cs | Fixed cast to handle new return type from PackCommand.FromArgs |
| documentation/general/dotnet-run-file.md | Updated documentation to reflect pack support and clarify behavior |
| test/dotnet.Tests/CompletionTests/snapshots/zsh/DotnetCliSnapshotTests.VerifyCompletions.verified.zsh | Updated completion description to include C# files |
Comments suppressed due to low confidence (1)
There was a problem hiding this comment.
What's the end-to-end scenario for packing file based project?
My understanding is that file based apps are always console apps, is that right? If so, should they be packed as tools, rather than packed as class libraries?
Tools are installed and used with dotnet tool install, whereas class libraries can only be used by PackageReference or equivilent.
There was a problem hiding this comment.
Yes, tools are the likely scenario. But you could actually pack a library as well if you wanted; like:
#:property OutputType=Library
public class C;(you just couldn't dotnet run such file-based app)
There was a problem hiding this comment.
I see the tests were updated to do #:property PackAsTool=true. I don't know why the default for csproj console apps is still PackAsTool=false, maybe we've just been so busy with other work we haven't considered changing the default. IMO it would be nice for the default to be true when the SDK knows it's a console app. But since file based apps is a new thing, should we consider changing the default to true for them, if we think that most customers will want that, so that it's not a breaking change in the future?
Normally .NET does breaking changes based on project TFM, so customers should feel safe to update the SDK and their projects keep working. What is the breaking change story for file based apps, since they don't have a TFM?
I'm still wondering what the scope is for file based apps. Reading the dotnet-run-file.md docs, it talks about projects needing to "grow up" under some scenarios, like when build customizations are needed. Is pack one of these scenarios that customers should need a project file for? This PR shows that technically we can support pack without a project file, but my question is should we?
The top of the document says file-based programs are intended to be an alternative to other scripting languages in .NET repos, and also to be easier for new customers. Pack isn't needed to be an alternative to other scripting languages. Being able to pack a file or directory without a project file certainly makes it easier for people new to .NET, I just want to make sure that we're doing this for the right reasons and minimise risk of needing breaking changes in the future.
Since NuGet treats packages as immutable, once a package has been extracted into the global packages folder, then NuGet will not check for "updates". The developer will either need to increment the package version number or manually delete the directory from the global packages folder. A bunch of customers have filed issues on NuGet saying they find this hard to test their packages. Is being able to pack without a project file going to make this easier, harder, or the same to experience this issue? I think if people start trying to use nupkgs as an alternative to projects and project references, then they're much more likely to encounter the problem.
But you could actually pack a library as well if you wanted; like:
#:property OutputType=Library
Is it inappropriate for me to be so opinionated as to think that class libraries should have unit tests, and therefore being able to pack a "file-based class library" is actually a bad thing for the ecosystem? Maybe I'm just being too stubborn, as there's nothing preventing customers from doing dotnet new classlib ; dotnet pack and pushing without any tests. But at least there's a project file for a test project to reference.
Other ecosystems have ceremony (create a manifest file) that the package author must do to create a package. Maybe packing without a project would be a competitive advantage for .NET. Or maybe it's precedent to say it's ok for customers to need to grow into a project file if they want to create a package.
Sorry for the long, rambly comment. I don't have context for what discussions and plans were made for file based apps already. I just saw this PR adding a NuGet scenario and nobody else from my team commented, so I did.
@baronfel is my go to for any ".NET SDK end to end scenario" questions, though I don't know if there's a different PM who owns the file based app scenario.
There was a problem hiding this comment.
Is pack one of these scenarios that customers should need a project file for? This PR shows that technically we can support pack without a project file, but my question is should we?
One of the scenarios for file based apps is tool publishing which is why pack support is being added. #49369 discusses a bit of the motivation here.
A bunch of customers have filed issues on NuGet saying they find this hard to test their packages. Is being able to pack without a project file going to make this easier, harder, or the same to experience this issue?
I agree that testing NuGet packages today, particularly when they in the global cache, can be challenging at times. I cannot see though how file based and project based apps are significantly different here.
Is it inappropriate for me to be so opinionated as to think that class libraries should have unit tests, and therefore being able to pack a "file-based class library" is actually a bad thing for the ecosystem?
How is this any different than running pack on a project based class lib that has no unit tests? As much as I believe in unit testing (personally and professionally) I don't think it's a requirement that we should be enforcing on customers.
There was a problem hiding this comment.
One of the scenarios for file based apps is tool publishing which is why pack support is being added. #49369 discusses a bit of the motivation here.
Thanks, I haven't seen this before. The original comment in the issue says:
We should consider supporting them with dotnet pack so that .NET SDK Tools can be made out of them.
So, that comes back to my original question: Would we be giving customers a better experience if we default PackAsTool to true, rather than making customers add #:property PackAsTool=true in their code file?
There was a problem hiding this comment.
PackAsTool=true by default for file-based apps would make sense to me. But any defaults that differ from project-based apps make the converted project file noisier. We currently only have one such difference (PublishAot). I'd be cautious before liberally adding more. If we could make PackAsTool=true the default for all console apps, that would be much nicer. But I guess it's too late in .NET 10 cycle now to do that. Perhaps we can do that in .NET 11?
There was a problem hiding this comment.
I strongly lean that we should make PackAsTool=true the default for file-based apps; if you are packing a single file then it seems extremely likely that the intent is to create a .NET tool, rather than a class library.
I also see the issue with drifting from the defaults when we convert to a project. Controversially my suggestion here is simple: don't add the property as part of grow up. Yes, it means that if you grow up a project and then pack it you get a different behavior, but it seems extremely noisy to add this to every project file when most of them won't need it. The number of users it will impact seems small. Instead, we just document the difference as an annoying paper cut and move on with our lives. IMHO the benefit of packing files as tools outweighs the pain of a small divergence.
|
Removing my assignment since I speculate Fred and Chris's sign off will be enough. Let me know if I misunderstood something. Thanks |
|
@333fred for another look, thanks |
Resolves #49369.