-
Notifications
You must be signed in to change notification settings - Fork 488
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
Document dotnet tools plugin usage #3379
base: main
Are you sure you want to change the base?
Conversation
Learn Build status updates of commit 18ce1c9: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Learn Build status updates of commit eba36da: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Learn Build status updates of commit 7de8612: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
|
||
1. Plugins installed as .NET tools must follow a naming convention: **`nuget-plugin-*`**. | ||
2. Upon installation, these plugins are added to the `PATH` by the .NET SDK. NuGet scans the `PATH` environment variable for executables with names starting with `nuget-plugin-`. | ||
3. On Windows, NuGet looks for `.exe` or `.bat` files, while on Linux and macOS, it identifies plugins by checking for the executable bit. |
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 this in addition to #1?
So how does one ship a global tool that works on both linux & windows?
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.
Sorry I was confused about this because I clicked on #1 and it ssays 47 files changed, and I couldn't figure out how it was related to this PR. Now, I understand you meant to ask about bullet point number 1. Yes, all dotnet tool plugins are expected to follow the naming conventions in number 1. Shipping a global package is enabled by dotnet pack as discussed https://github.com/dotnet/docs/blob/main/docs/core/tools/global-tools-how-to-create.md. I have update the doc to link the instructions
Learn Build status updates of commit 2ceb6cf: 💡 Validation status: suggestions
docs/reference/extensibility/NuGet-Cross-Platform-Plugins.md
For more details, please refer to the build report. Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them. For any questions, please:
|
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 doc is authored more as a conceptual doc, but the tone of the changes feels like more of a pitch.
I think the language should be tweaked to be more "matter of fact" way.
- `NUGET_PLUGIN_PATHS` | ||
- defines the plugins that will be used for that NuGet process, priority preserved. If this environment variable is set, it overrides the convention based discovery. Ignored if either of the framework specific variables is specified. | ||
|
||
- Starting 6.13, this can also be used to specify a path to a .Net tools plugin path. |
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.
- Starting 6.13, this can also be used to specify a path to a .Net tools plugin path. | |
- Starting 6.13, this can also be used to specify a path to a .NET tools plugin path. | |
- ``` |
1. Plugins installed as .NET tools must follow a naming convention: **`nuget-plugin-*`**. | ||
2. Upon installation, these plugins are added to the `PATH` by the .NET SDK. NuGet scans the `PATH` environment variable for executables with names starting with `nuget-plugin-`. | ||
3. In addition to number 1, on Windows, NuGet looks for `.exe` or `.bat` files, while on Linux and macOS, it identifies plugins by checking for the executable bit. | ||
4. These plugins are launched in a separate process, consistent with the existing design. |
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 think this type of information made sense in the design, but this document is talking about all plugins period, so I think we can remove it.
|
||
1. Plugins installed as .NET tools must follow a naming convention: **`nuget-plugin-*`**. | ||
2. Upon installation, these plugins are added to the `PATH` by the .NET SDK. NuGet scans the `PATH` environment variable for executables with names starting with `nuget-plugin-`. | ||
3. In addition to number 1, on Windows, NuGet looks for `.exe` or `.bat` files, while on Linux and macOS, it identifies plugins by checking for the executable bit. |
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 think these are in a way separate features.
The support is that nuget-plugin-* is looked from the path, the fact that .NET tools are how it's going to be done is a separate.
This document serves as more of a reference, telling you how things work.
|
||
#### Example Workflow | ||
|
||
**For Plugin Authors:** |
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.
nit: I think is too much detail.
Say it can be installed as a .NET tool by linking and it's enough.
It doesn't need to be a step by step guide.
```xml | ||
<PropertyGroup> | ||
<PackAsTool>true</PackAsTool> | ||
<ToolCommandName>nuget-plugin-myplugin</ToolCommandName> |
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.
My suggestion would be remove this, but package name and tool command name aren't necessarily the same.
|
||
### Security Considerations | ||
|
||
.NET tools run in full trust. It is essential to install only trusted plugins. While this is not a new concern, users should be aware of the risks when installing NuGet plugins via .NET tools. |
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 this apply to plugins in general?
Should this be a generic statement?
dotnet tool install -g nuget-plugin-myplugin | ||
``` | ||
|
||
2. Use the plugin seamlessly in scenarios requiring NuGet authentication or operations like `dotnet restore --interactive`. |
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 think we should remove this is. People would normally expect that the tools regardless of how they're installed would work the same.
Learn Build status updates of commit 5457a9f: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Learn Build status updates of commit 35dae67: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
That makes sense as my documentation was motivated by the design spec. I have simplified it to mention things that would concern users only. |
@@ -47,11 +47,14 @@ Under this version, the requirements are as follows: | |||
- Adhere to the negotiated plugin protocol version. | |||
- Respond to all requests within a reasonable time period. | |||
- Honor cancellation requests for any in-progress operation. | |||
- Plugins installed as global .NET tools starting with NuGet 6.13. These plugins: |
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 we mention that the feature is more about executables on the path?
@@ -70,7 +73,10 @@ CI/CD scenarios and power users can use environment variables to override the be | |||
|
|||
- `NUGET_NETFX_PLUGIN_PATHS` - defines the plugins that will be used by the .NET Framework based tooling (NuGet.exe/MSBuild.exe/Visual Studio). Takes precedence over `NUGET_PLUGIN_PATHS`. (NuGet version 5.3+ only) | |||
- `NUGET_NETCORE_PLUGIN_PATHS` - defines the plugins that will be used by the .NET Core based tooling (dotnet.exe). Takes precedence over `NUGET_PLUGIN_PATHS`. (NuGet version 5.3+ only) | |||
- `NUGET_PLUGIN_PATHS` - defines the plugins that will be used for that NuGet process, priority preserved. If this environment variable is set, it overrides the convention based discovery. Ignored if either of the framework specific variables is specified. | |||
- `NUGET_PLUGIN_PATHS` |
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 comment is about the Plugin installation and discovery
section.
We should call ouit that search for nuget-plugin-* executables here as well. We only have it in the general plugin requirements right now.
Fixes: NuGet/Home#13858