Skip to content
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

Execute net tools plugins #6113

1 change: 0 additions & 1 deletion src/NuGet.Core/NuGet.Protocol/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@
[assembly: SuppressMessage("Build", "CA1031:Modify 'FireBeforeClose' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.Plugin.FireBeforeClose")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'FireClosed' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.Plugin.FireClosed")]
[assembly: SuppressMessage("Build", "CA2000:Call System.IDisposable.Dispose on object created by 'new MonitorNuGetProcessExitRequestHandler(plugin)' before all references to it are out of scope.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.PluginFactory.CreateFromCurrentProcessAsync(NuGet.Protocol.Plugins.IRequestHandlers,NuGet.Protocol.Plugins.ConnectionOptions,System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.Protocol.Plugins.IPlugin}")]
[assembly: SuppressMessage("Build", "CA2000:Use recommended dispose pattern to ensure that object created by 'new PluginProcess(startInfo)' is disposed on all paths. If possible, wrap the creation within a 'using' statement or a 'using' declaration. Otherwise, use a try-finally pattern, with a dedicated local variable declared before the try region and an unconditional Dispose invocation on non-null value in the 'finally' region, say 'x?.Dispose()'. If the object is explicitly disposed within the try region or the dispose ownership is transfered to another object or method, assign 'null' to the local variable just after such an operation to prevent double dispose in 'finally'.", Justification = "The responsibility to dispose the object is transferred to another object or wrapper that's created in the method and returned to the caller", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.PluginFactory.CreatePluginAsync(System.String,System.Collections.Generic.IEnumerable{System.String},NuGet.Protocol.Plugins.IRequestHandlers,NuGet.Protocol.Plugins.ConnectionOptions,System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.Protocol.Plugins.IPlugin}")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'SendCloseRequest' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.PluginFactory.SendCloseRequest(NuGet.Protocol.Plugins.IPlugin)")]
[assembly: SuppressMessage("Build", "CA1822:Member GetPluginOperationClaimsAsync does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.PluginManager.GetPluginOperationClaimsAsync(NuGet.Protocol.Plugins.IPlugin,System.String,Newtonsoft.Json.Linq.JObject,System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.Collections.Generic.IReadOnlyList{NuGet.Protocol.Plugins.OperationClaim}}")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'TryCreatePluginAsync' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.Protocol.Plugins.PluginManager.TryCreatePluginAsync(NuGet.Protocol.Plugins.PluginDiscoveryResult,NuGet.Protocol.Plugins.OperationClaim,NuGet.Protocol.Plugins.PluginManager.PluginRequestKey,System.String,Newtonsoft.Json.Linq.JObject,System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.Tuple{System.Boolean,NuGet.Protocol.Plugins.PluginCreationResult}}")]
Expand Down
1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.Protocol/Plugins/IPluginFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Task<IPlugin> GetOrCreateAsync(
IEnumerable<string> arguments,
IRequestHandlers requestHandlers,
ConnectionOptions options,
bool isRunnablePluginFile,
CancellationToken sessionCancellationToken);
}
}
17 changes: 11 additions & 6 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private static List<PluginFile> GetPluginFiles(IEnumerable<string> filePaths, Ca
{
return PluginFileState.InvalidFilePath;
}
}));
}), isRunnablePluginFile: false);
files.Add(pluginFile);
}

Expand All @@ -182,6 +182,11 @@ private static List<PluginFile> GetPluginFiles(IEnumerable<string> filePaths, Ca
/// <returns>A list of valid <see cref="PluginFile"/> objects representing the discovered plugins.</returns>
internal List<PluginFile> GetPluginsInNuGetPluginPaths()
{
bool isDesktop = false;

#if IS_DESKTOP
isDesktop = true;
#endif
var pluginFiles = new List<PluginFile>();
string[] paths = _nuGetPluginPaths?.Split(Path.PathSeparator) ?? Array.Empty<string>();

Expand All @@ -197,15 +202,15 @@ internal List<PluginFile> GetPluginsInNuGetPluginPaths()
// A DotNet tool plugin
if (IsValidPluginFile(fileInfo))
{
PluginFile pluginFile = new PluginFile(fileInfo.FullName, new Lazy<PluginFileState>(() => PluginFileState.Valid), isDotnetToolsPlugin: true);
PluginFile pluginFile = new PluginFile(fileInfo.FullName, new Lazy<PluginFileState>(() => PluginFileState.Valid), isRunnablePluginFile: true);
pluginFiles.Add(pluginFile);
}
}
else
{
// A non DotNet tool plugin file
var state = new Lazy<PluginFileState>(() => PluginFileState.Valid);
pluginFiles.Add(new PluginFile(fileInfo.FullName, state));
pluginFiles.Add(new PluginFile(fileInfo.FullName, state, isRunnablePluginFile: isDesktop));
}
}
else if (Directory.Exists(path))
Expand All @@ -215,7 +220,7 @@ internal List<PluginFile> GetPluginsInNuGetPluginPaths()
}
else
{
pluginFiles.Add(new PluginFile(path, new Lazy<PluginFileState>(() => PluginFileState.InvalidFilePath)));
pluginFiles.Add(new PluginFile(path, new Lazy<PluginFileState>(() => PluginFileState.InvalidFilePath), isRunnablePluginFile: isDesktop));
}
}

Expand All @@ -240,7 +245,7 @@ internal List<PluginFile> GetPluginsInPath()
}
else
{
pluginFiles.Add(new PluginFile(path, new Lazy<PluginFileState>(() => PluginFileState.InvalidFilePath)));
pluginFiles.Add(new PluginFile(path, new Lazy<PluginFileState>(() => PluginFileState.InvalidFilePath), isRunnablePluginFile: false));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test to validate this scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt we don’t need a test specifically for this scenario, as it would end up testing the behavior of PathValidator.IsValidLocalPath and PathValidator.IsValidUncPath within the if statement. Since the else case only triggers if both methods return false, testing this would indirectly validate PathValidator’s internal logic, which I felt isn’t necessary for this context.

}
}

Expand All @@ -260,7 +265,7 @@ private static List<PluginFile> GetNetToolsPluginsInDirectory(string directoryPa
{
if (IsValidPluginFile(file))
{
PluginFile pluginFile = new PluginFile(file.FullName, new Lazy<PluginFileState>(() => PluginFileState.Valid), isDotnetToolsPlugin: true);
PluginFile pluginFile = new PluginFile(file.FullName, new Lazy<PluginFileState>(() => PluginFileState.Valid), isRunnablePluginFile: true);
pluginFiles.Add(pluginFile);
}
}
Expand Down
63 changes: 37 additions & 26 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public void Dispose()
/// <param name="arguments">Command-line arguments to be supplied to the plugin.</param>
/// <param name="requestHandlers">Request handlers.</param>
/// <param name="options">Connection options.</param>
/// <param name="isRunnablePluginFile">Is the file a runnable file.</param>
/// <param name="sessionCancellationToken">A cancellation token for the plugin's lifetime.</param>
/// <returns>A task that represents the asynchronous operation.
/// The task result (<see cref="Task{TResult}.Result" />) returns a <see cref="Plugin" />
Expand All @@ -104,6 +105,7 @@ public async Task<IPlugin> GetOrCreateAsync(
IEnumerable<string> arguments,
IRequestHandlers requestHandlers,
ConnectionOptions options,
bool isRunnablePluginFile,
CancellationToken sessionCancellationToken)
{
if (_isDisposed)
Expand Down Expand Up @@ -136,7 +138,7 @@ public async Task<IPlugin> GetOrCreateAsync(
var lazyTask = _plugins.GetOrAdd(
filePath,
(path) => new Lazy<Task<IPlugin>>(
() => CreatePluginAsync(filePath, arguments, requestHandlers, options, sessionCancellationToken)));
() => CreatePluginAsync(filePath, arguments, requestHandlers, options, isRunnablePluginFile: isRunnablePluginFile, sessionCancellationToken)));

await lazyTask.Value;

Expand All @@ -149,36 +151,45 @@ private async Task<IPlugin> CreatePluginAsync(
IEnumerable<string> arguments,
IRequestHandlers requestHandlers,
ConnectionOptions options,
bool isRunnablePluginFile,
CancellationToken sessionCancellationToken)
{
var args = string.Join(" ", arguments);
#if IS_DESKTOP
var startInfo = new ProcessStartInfo(filePath)

ProcessStartInfo startInfo;

if (!isRunnablePluginFile)
{
Arguments = args,
UseShellExecute = false,
RedirectStandardError = false,
RedirectStandardInput = true,
RedirectStandardOutput = true,
StandardOutputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
WindowStyle = ProcessWindowStyle.Hidden,
};
#else
var startInfo = new ProcessStartInfo
startInfo = new ProcessStartInfo
{
FileName = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ??
(NuGet.Common.RuntimeEnvironmentHelper.IsWindows ?
"dotnet.exe" :
"dotnet"),
Arguments = $"\"{filePath}\" " + args,
UseShellExecute = false,
RedirectStandardError = false,
RedirectStandardInput = true,
RedirectStandardOutput = true,
StandardOutputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
WindowStyle = ProcessWindowStyle.Hidden,
};
}
else
{
FileName = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ??
(NuGet.Common.RuntimeEnvironmentHelper.IsWindows ?
"dotnet.exe" :
"dotnet"),
Arguments = $"\"{filePath}\" " + args,
UseShellExecute = false,
RedirectStandardError = false,
RedirectStandardInput = true,
RedirectStandardOutput = true,
StandardOutputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
WindowStyle = ProcessWindowStyle.Hidden,
};
#endif
// Execute file directly.
startInfo = new ProcessStartInfo(filePath)
{
Arguments = args,
UseShellExecute = false,
RedirectStandardError = false,
RedirectStandardInput = true,
RedirectStandardOutput = true,
StandardOutputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
WindowStyle = ProcessWindowStyle.Hidden,
};
}

var pluginProcess = new PluginProcess(startInfo);
string pluginId = Plugin.CreateNewId();

Expand Down
19 changes: 5 additions & 14 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,33 +21,24 @@ public sealed class PluginFile
public Lazy<PluginFileState> State { get; }

/// <summary>
/// Is the plugin file, a dotnet tools plugin file?
/// Indicates if the plugin file is runnable, such as an executable or a script.
/// </summary>
internal bool IsDotnetToolsPlugin { get; }
internal bool IsRunnablePluginFile { get; }

/// <summary>
/// Instantiates a new <see cref="PluginFile" /> class.
/// </summary>
/// <param name="filePath">The plugin's file path.</param>
/// <param name="state">A lazy that evaluates the plugin file state.</param>
/// <param name="isDotnetToolsPlugin">Is the plugin file, a dotnet tools plugin file?</param>
internal PluginFile(string filePath, Lazy<PluginFileState> state, bool isDotnetToolsPlugin) : this(filePath, state)
{
IsDotnetToolsPlugin = isDotnetToolsPlugin;
}

/// <summary>
/// Instantiates a new <see cref="PluginFile" /> class.
/// </summary>
/// <param name="filePath">The plugin's file path.</param>
/// <param name="state">A lazy that evaluates the plugin file state.</param>
public PluginFile(string filePath, Lazy<PluginFileState> state)
/// <param name="isRunnablePluginFile">Is the plugin file, a runnable plugin file?</param>
public PluginFile(string filePath, Lazy<PluginFileState> state, bool isRunnablePluginFile)
{
if (string.IsNullOrEmpty(filePath))
{
throw new ArgumentException(Strings.ArgumentCannotBeNullOrEmpty, nameof(filePath));
}

IsRunnablePluginFile = isRunnablePluginFile;
Path = filePath;
State = state;
}
Expand Down
15 changes: 9 additions & 6 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,15 @@ private async Task<Tuple<bool, PluginCreationResult>> TryCreatePluginAsync(
{
if (result.PluginFile.State.Value == PluginFileState.Valid)
{
var plugin = await _pluginFactory.GetOrCreateAsync(
result.PluginFile.Path,
PluginConstants.PluginArguments,
new RequestHandlers(),
_connectionOptions,
cancellationToken);
IPlugin plugin;

plugin = await _pluginFactory.GetOrCreateAsync(
filePath: result.PluginFile.Path,
arguments: PluginConstants.PluginArguments,
requestHandlers: new RequestHandlers(),
options: _connectionOptions,
isRunnablePluginFile: result.PluginFile.IsRunnablePluginFile,
sessionCancellationToken: cancellationToken);

var utilities = await PerformOneTimePluginInitializationAsync(plugin, cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,6 @@ NuGet.Protocol.Plugins.IPlugin.Closed -> System.EventHandler
NuGet.Protocol.Plugins.IPluginDiscoverer
~NuGet.Protocol.Plugins.IPluginDiscoverer.DiscoverAsync(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Generic.IEnumerable<NuGet.Protocol.Plugins.PluginDiscoveryResult>>
NuGet.Protocol.Plugins.IPluginFactory
~NuGet.Protocol.Plugins.IPluginFactory.GetOrCreateAsync(string filePath, System.Collections.Generic.IEnumerable<string> arguments, NuGet.Protocol.Plugins.IRequestHandlers requestHandlers, NuGet.Protocol.Plugins.ConnectionOptions options, System.Threading.CancellationToken sessionCancellationToken) -> System.Threading.Tasks.Task<NuGet.Protocol.Plugins.IPlugin>
NuGet.Protocol.Plugins.IPluginManager
~NuGet.Protocol.Plugins.IPluginManager.CreatePluginsAsync(NuGet.Protocol.Core.Types.SourceRepository source, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Generic.IEnumerable<NuGet.Protocol.Plugins.PluginCreationResult>>
~NuGet.Protocol.Plugins.IPluginManager.FindAvailablePluginsAsync(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Generic.IEnumerable<NuGet.Protocol.Plugins.PluginDiscoveryResult>>
Expand Down Expand Up @@ -1212,11 +1211,9 @@ NuGet.Protocol.Plugins.PluginException
~NuGet.Protocol.Plugins.PluginException.PluginException(string message, System.Exception innerException) -> void
NuGet.Protocol.Plugins.PluginFactory
NuGet.Protocol.Plugins.PluginFactory.Dispose() -> void
~NuGet.Protocol.Plugins.PluginFactory.GetOrCreateAsync(string filePath, System.Collections.Generic.IEnumerable<string> arguments, NuGet.Protocol.Plugins.IRequestHandlers requestHandlers, NuGet.Protocol.Plugins.ConnectionOptions options, System.Threading.CancellationToken sessionCancellationToken) -> System.Threading.Tasks.Task<NuGet.Protocol.Plugins.IPlugin>
NuGet.Protocol.Plugins.PluginFactory.PluginFactory(System.TimeSpan pluginIdleTimeout) -> void
NuGet.Protocol.Plugins.PluginFile
~NuGet.Protocol.Plugins.PluginFile.Path.get -> string
~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy<NuGet.Protocol.Plugins.PluginFileState> state) -> void
~NuGet.Protocol.Plugins.PluginFile.State.get -> System.Lazy<NuGet.Protocol.Plugins.PluginFileState>
NuGet.Protocol.Plugins.PluginFileState
NuGet.Protocol.Plugins.PluginFileState.InvalidEmbeddedSignature = 3 -> NuGet.Protocol.Plugins.PluginFileState
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
#nullable enable
NuGet.Protocol.Plugins.PluginDiscoverer.PluginDiscoverer() -> void
~NuGet.Protocol.Plugins.IPluginFactory.GetOrCreateAsync(string filePath, System.Collections.Generic.IEnumerable<string> arguments, NuGet.Protocol.Plugins.IRequestHandlers requestHandlers, NuGet.Protocol.Plugins.ConnectionOptions options, bool isRunnablePluginFile, System.Threading.CancellationToken sessionCancellationToken) -> System.Threading.Tasks.Task<NuGet.Protocol.Plugins.IPlugin>
~NuGet.Protocol.Plugins.PluginFactory.GetOrCreateAsync(string filePath, System.Collections.Generic.IEnumerable<string> arguments, NuGet.Protocol.Plugins.IRequestHandlers requestHandlers, NuGet.Protocol.Plugins.ConnectionOptions options, bool isRunnablePluginFile, System.Threading.CancellationToken sessionCancellationToken) -> System.Threading.Tasks.Task<NuGet.Protocol.Plugins.IPlugin>
~NuGet.Protocol.Plugins.PluginFile.PluginFile(string filePath, System.Lazy<NuGet.Protocol.Plugins.PluginFileState> state, bool isRunnablePluginFile) -> void
Loading