-
Notifications
You must be signed in to change notification settings - Fork 161
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
Spec for resolving '-p' in 'dotnet run' #229
Open
KathleenDollard
wants to merge
5
commits into
dotnet:main
Choose a base branch
from
KathleenDollard:dotnet-run-resolve-p
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b1e8eb2
Added proposal for dotnet run -p
KathleenDollard 6a183ec
Removed file
KathleenDollard 2f8c3cf
Add proposal for resolving -p in dotnet run
KathleenDollard 6b0fdb3
Responded to PR comments
KathleenDollard 8748947
More PR response
KathleenDollard File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Syntax for passing `--property` to MSBuild in `dotnet run` | ||
|
||
Kathleen Dollard | Status: Draft | ||
|
||
The .NET CLI passes all command line arguments that are not explicitly handled through to MSBuild for `dotnet build` and `dotnet publish`. This does not work for `dotnet run` because there are three sets of arguments: | ||
|
||
* CLI arguments for `dotnet run` itself. | ||
* Arguments for the resulting application, identified by `--` or because they are unrecognized | ||
* MSBuild arguments, which are not currently supported. | ||
|
||
To support the Xamarin/Maui workloads, we need to pass a property to MSBuild that specifies the device to target. This is being solved by adding a `--property` option to `dotnet run`. This will route to MSBuild as `-p:<delimited list>`. | ||
|
||
If an application has `--property` option today, and the option is passed without the `--` indicator, there will be a breaking change because `--property` would not be passed to the application. | ||
|
||
The first syntax would behave differently - the `--property` would pass the `--property` option and argument only to MSBuild, while the second syntax would pass it only to the app: | ||
|
||
``` | ||
dotnet run --property device=deviceName | ||
dotnet run -- --property device=deviceName | ||
``` | ||
|
||
We recommend using -- for clarity and to protect against issues arising if we add additional options to `dotnet run`. | ||
|
||
**Note:** A long term approach to managing "device" selection is planned for a later release. In .NET 6 CLI usage of these workloads is expected to just be CI, so this is not considered critical, and the design work is expected to include adjacent problems of specifying containers, and x64 emulation). | ||
|
||
A problem arises in the abbreviation `-p`. It is currently used to specify the `--project` for `dotnet run`. This proposal is a course to change `-p` from meaning `--project` to `--property` in order to be consistent with the similar commands - `dotnet build` and ` dotnet publish`. This proposal balances backward compatibility and consistency with other commands, which we can do because the usages can be distinguished syntactically. | ||
|
||
During a deprecation phase, which will be at least the length of .NET 6, older usage of `-p` as `--project` will be recognized as when there is no trailing colon (`-p:`) and the argument is not legal for MSBuild. Users will receive a warning that the abbreviation is deprecated. | ||
|
||
We will interpret `-p` as `--property` when the argument value is legal MSBuild syntax. Legal MSBuild syntax will be defined as: | ||
|
||
* A comma or semi-colon delimited strings of values in the format <name>=<value> or <name>:<value>. We may simplify this to only checking for the equals - the simplest sufficient check will be used. | ||
|
||
If you have any scripts that are using “dotnet run” and process the output you could encounter a break. dotnet run typically doesn’t output anything of its own if there are no errors, so you only get the output of the program that is being run. If you have a script or other program wrapping “dotnet run” and parsing the output, the warning would be unexpected text and may cause a failure. | ||
|
||
If you have a project argument that includes an `=` and use the `-p` abbreviation, it will be interpreted as `--property` and passed to MSBuild. | ||
|
||
## Goals | ||
|
||
We believe that in the long term `-p` should be used consistently across `dotnet build`, `dotnet publish` and `dotnet run` because of similarity between these commands. | ||
|
||
### Design | ||
|
||
The basic design is described in the opening: | ||
|
||
* `-p` with syntax that is valid for MSBuild properties will be treated as `--property`. | ||
* Any other usage will be treated as `--project` and will receive a warning that this use is deprecated. | ||
* At some future point, we may remove `-p` for `--project`. | ||
|
||
### Warning text | ||
|
||
When -p is used (with no colon) in .NET 6, the following warning will be displayed: | ||
|
||
> Warning NETSDKNNNN: The abbreviation of -p for --project is deprecated. Please use --project. | ||
|
||
Where `NNNN` will be replaced with an SDK error code. In contrast to many other errors and warnings generated by the .NET SDK, this warning will be generated by the .NET CLI parsing code instead of flowing through MSBuild. So it will not be affected by "warn as error" or "ignore warning" MSBuild settings. | ||
|
||
### Implementation notes | ||
|
||
We plan to display a deprecation warning for `-p` in .NET 6 and hope to do the additional work to support `-p` as `--property`. However, once the deprecation is in place, we can add the abbreviation in a feature band (quarterly) release. | ||
|
||
|
||
## Q & A | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.