-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update multi-file 'dotnet run file' documentation #48437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
documentation/general/dotnet-run-file.md:48
- [nitpick] Consider verifying that the command-line flag naming (e.g., '--cs-from-stdin' and '--cs-code') is consistent and clearly communicates their purpose.
We can consider adding an option like `dotnet run --cs-from-stdin` which would read the C# file from the standard input
Aside: It would be a good idea to come up with and use a search-engine-friendly term for the I think "ignored directives" should basically be tucked away in the C# spec. That's just how we happen to allow them to pass nicely thru the compiler layer while having their "real" meaning be determined by an external layer. I don't think that's a term we should tell users when discussing this feature area. |
This comment was marked as resolved.
This comment was marked as resolved.
App/Program1/obj/ | ||
App/Program2/bin/ | ||
App/Program2/obj/ | ||
have the shared `.cs` files source-included via `<Compile Include="../Shared/**/*.cs" />`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this wouldn't include non-C# files like .resx
. We could add items like <EmbeddedResource Include="../Shared/**/*.cs" />
etc for all the default items, but I think I'm going to investigate if it would be possible to add a property like <AdditionalDefaultIncludeItemsDirectory>..\Shared</>
that would be recognized by the .NET SDK props/targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I haven't done this in the corresponding implementation PR to keep it more self-contained, I think we can implement this in a follow up.)
Similarly, it could be possible to specify the whole C# source text in a command-like argument | ||
like `dotnet run --code 'Console.WriteLine("Hi")'`. | ||
like `dotnet run --cs-code 'Console.WriteLine("Hi")'`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation is a bit hard to use a spec since it's hypothesizing features that we may not have. Can we either [a] make a separate spec that removes this or [b] remove these from this file if this is intended to be the spec? Besides the IDE needing a precise spec for purposes of implementing our stuff, I worry this might 'train' a copilot these commands exist, but don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yes, it started as a design doc with lots of unknowns, I guess I should clean it up now, thanks.
to check whether the file contains an entry point (top-level statements or a valid Main method) and report an error otherwise. | ||
(We cannot simply use Roslyn APIs to detect entry points ourselves because parsing depends on conditional symbols like those from `<DefineConstants>` | ||
and we can reliably know the set of those only after invoking MSBuild, and doing that up front would be an unnecessary performance hit just to detect entry points.) | ||
Internally, the SDK CLI detects entry points by parsing all C# files with default parsing options (in particular, no `<DefineConstants>`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is "all C# files" defined here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All C# files in the directory tree of the entry point file. I will clarify.
Note that it is possible for multiple users to run the same file-based program, however each user's run uses different build artifacts since the base directory is unique per user. | ||
Apart from keeping the source directory clean, such artifact isolation also avoids clashes of build outputs that are not project-scoped, like `project.assets.json`, in the case of multiple entry-point files. | ||
|
||
Artifacts are cleaned periodically by a background task that is started by `dotnet run` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the motivation for not deleting right away to allow for caching?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will clarify, thanks.
``` | ||
|
||
## Build outputs | ||
|
||
Build outputs are placed under a subdirectory whose name is hashed file path of the entry point |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I have two places running the same file twice in parallel? The usual rule would be you shouldn't run dotnet build twice on the same project, but from the user's perspective, there isn't a build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from the user's perspective, there isn't a build.
I'm not sure about that, there is even --no-build
option. Anyway this seems like a pre-existing issue - project-based dotnet run
should behave equivalently to the new file-based dotnet run
.
which also makes it possible to share it independently or symlink it to multiple script folders. | ||
This is also similar to `global using`s which users usually put into a single file but don't have to. | ||
For a given `dotnet run file.cs`, we include directives from the current entry point file (` F438 file.cs`) and all other non-entry-point files. | ||
The order in which other files are processed is currently unspecified (can change across SDK versions) but deterministic (stable in a given SDK version). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the lack of specification? Because doesn't the order of SDK inclusion impact the MSBuild property evaluation and such?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't the order of SDK inclusion impact the MSBuild property evaluation and such?
Yes it does, but I hope users won't rely on that in their simple file-based programs. We can specify this, but once we do, any changes will be breaking, so that's why I chose to not specify at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any changes will be breaking, so that's why I chose to not specify at first.
Any changes are still breaking even if we don't specify it. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but only if someone chooses to rely on undefined behavior in the first place.
Anyway, I can specify some ordering here, but maybe it would be better to wait a bit and see what makes most sense when we see real-world usage. For example, I'm currently unsure whether it makes sense for entry-point directives to go before or after non-entry-point directives? Should other files be just alphabetically ordered, or should their directory nesting also play a role in the order?
We do not limit these directives to appear only in entry point files because it allows: | ||
- a non-entry-point file like `Util.cs` to be self-contained and have all the `#:package`s it needs specified in it, | ||
- which also makes it possible to share it independently or symlink it to multiple script folders, | ||
- and it's similar to `global using`s which users usually put into a single file but don't have to. | ||
|
||
We could consider deduplicating `#:` directives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd at least expect us to block duplicate stuff, otherwise we've boxed ourselves into a corner we can't later design properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently don't do any deduplication so all directives end up in the final project and MSBuild should fail for invalid duplicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What duplicates do you expect MSBuild to fail on? ProjectReference maybe, but properties being assigned more than once is totally allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I guess it should be simple to block duplicate directives for now, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this seems a bit orthogonal to multi-file support (we can block duplicate directives in single files as well), so instead of bloating up the implementation PR, I filed an issue to track this: #48842
This means the CLI might consider a file to be an entry point which later the compiler doesn't | ||
(for example because its top-level statements are under `#if SYMBOL` and the build has `DefineConstants=SYMBOL`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be #if !SYMBOL
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #48943.
No description provided.