Discover plugins installed using Net tools #5990
Merged
Conversation
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
dotnet-policy-service
bot
added
the
Status:No recent activity
Nigusu-Allehu
removed
the
Status:No recent activity
zivkan
reviewed
Sep 3, 2024
Nigusu-Allehu
changed the title
zivkan
reviewed
Sep 10, 2024
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs
Outdated
Show resolved
Hide resolved
zivkan
reviewed
Sep 19, 2024
nkolev92
added
the
Merge next release
nkolev92
reviewed
Sep 24, 2024
nkolev92
removed
the
Merge next release
microsoft-github-policy-service
bot
added
Status:No recent activity
Nigusu-Allehu
commented
Oct 8, 2024
| } | ||
|
|
||
| [PlatformFact(Platform.Windows)] | ||
| public void GetNetToolsPluginFiles_WithNuGetPluginPaths_ReturnsPluginsInNuGetPluginPathOnly() |
There was a problem hiding this comment.
Test priority of the discovery methods we have. This test makes sure we prioritize the environmental variables.
Nigusu-Allehu
force-pushed
the
dev-nyenework-net-tools-plugin
branch
from
85141d8
to
3eab1a7
Compare
nkolev92
approved these changes
Oct 16, 2024
jgonz120
approved these changes
Oct 17, 2024
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs
Outdated
Show resolved
Hide resolved
| File.Create(pluginFilePath); | ||
|
|
||
| // Remove execute permissions | ||
| var process = new Process(); |
There was a problem hiding this comment.
nit: There is an opportunity to refactor here because we set/remove execute permissions for files in all the new tests. I think we can create a private utility method and invoke it from the tests as needed. Please feel free to address it a follow-up issue.
kartheekp-ms
approved these changes
Oct 22, 2024
Nigusu-Allehu
merged commit 6c1f792
into
dev-feature-dot-net-tool-plugin-support
25 of 31 checks passed
| process.StartInfo.UseShellExecute = false; | ||
| process.StartInfo.RedirectStandardOutput = true; | ||
| process.Start(); | ||
| process.WaitForExit(); |
There was a problem hiding this comment.
Given that Process.WaitForExit waits indefinitely for the process to exit, while addressing this comment you might want to consider this code by changing it to the following:
Assert.True(process.WaitForExit(1000), userMessage: "some string that helps to troubleshoot the issue");With this approach, the test fails gracefully if the process doesn't complete on time for any unknown reason.
3 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Bug
Fixes: NuGet/Home#13740
Description
This PR makes sure NuGet discovers credential providers installed using .NET tools.
dotnet tool install <provider package name> --globalshould install the tool and add its path to thePATHenvironmental varibale. Iterate through all the paths inPATHand all the files in the path to find the plugins. By design, we expect these plugins to be namednuget-plugin*.Plugin discovery method
The discovery is done based on which environmental variable has been specified
NUGET_NETFX_PLUGIN_PATHSorNUGET_NETCORE_PLUGIN_PATHShave been specified, use the old discovery method to look up non dotnet tools plugins in the paths added in the env variable.NUGET_NETFX_PLUGIN_PATHSandNUGET_NETCORE_PLUGIN_PATHShave not been specifiedNUGET_PLUGIN_PATHSspecified,NUGET_PLUGIN_PATHSnot specified,Default discovery method
PATHenv variable, and search for executablenuget-plugin-*filesPR Checklist