-
Notifications
You must be signed in to change notification settings - Fork 3
WIP: GH18: Add support for dotnet local tools #19
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,10 +102,14 @@ public IReadOnlyCollection<IFile> Install(PackageReference package, PackageType | |
_log.Debug("Configured Tools Folder: {0}", toolsFolderDirectoryPath); | ||
|
||
var toolLocation = toolsFolderDirectoryPath.FullPath; | ||
if(package.Parameters.ContainsKey("global")) | ||
if (package.Parameters.ContainsKey("global")) | ||
{ | ||
toolLocation = "global"; | ||
} | ||
else if (package.Parameters.ContainsKey("local")) | ||
{ | ||
toolLocation = "local"; | ||
} | ||
|
||
// First we need to check if the Tool is already installed | ||
var installedTools = GetInstalledTools(toolLocation); | ||
|
@@ -159,24 +163,28 @@ public IReadOnlyCollection<IFile> Install(PackageReference package, PackageType | |
private List<DotNetToolPackage> GetInstalledTools(string toolLocation) | ||
{ | ||
var toolLocationArgument = string.Empty; | ||
if(toolLocation != "global") | ||
if (toolLocation == "global") | ||
{ | ||
toolLocationArgument = "--global"; | ||
} | ||
else if (toolLocation == "local") | ||
{ | ||
toolLocationArgument = "--local"; | ||
} | ||
else | ||
{ | ||
toolLocationArgument = string.Format("--tool-path \"{0}\"", toolLocation); | ||
var toolLocationDirectoryPath = new DirectoryPath(toolLocation).MakeAbsolute(_environment); | ||
var toolLocationDirectory = _fileSystem.GetDirectory(toolLocationDirectoryPath); | ||
|
||
// If the requested tools path doesn't exist, then there can't be any tools | ||
// installed there, so simply return an empty list. | ||
if(!toolLocationDirectory.Exists) | ||
if (!toolLocationDirectory.Exists) | ||
{ | ||
_log.Debug("Specified installation location doesn't currently exist."); | ||
return new List<DotNetToolPackage>(); | ||
} | ||
} | ||
else | ||
{ | ||
toolLocationArgument = "--global"; | ||
} | ||
|
||
var isInstalledProcess = _processRunner.Start( | ||
"dotnet", | ||
|
@@ -190,9 +198,9 @@ private List<DotNetToolPackage> GetInstalledTools(string toolLocation) | |
var installedTools = isInstalledProcess.GetStandardOutput().ToList(); | ||
var installedToolNames = new List<DotNetToolPackage>(); | ||
|
||
string pattern = @"(?<packageName>[^\s]+)\s+(?<packageVersion>[^\s]+)\s+(?<packageShortCode>[^`s])"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This regex had a bug; `s should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticed, it also didn't match more than one character. I guess it's not used. |
||
string pattern = @"(?<packageName>[^\s]+)\s+(?<packageVersion>[^\s]+)\s+(?<packageShortCode>[^\s]+)(?:\s+(?<packageManifest>[^\s]+))?"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optionally match the package manifest, when available, for local tools. |
||
|
||
foreach(var installedTool in installedTools.Skip(2)) | ||
foreach (var installedTool in installedTools.Skip(2)) | ||
{ | ||
foreach (Match match in Regex.Matches(installedTool, pattern, RegexOptions.IgnoreCase)) | ||
{ | ||
|
@@ -201,7 +209,8 @@ private List<DotNetToolPackage> GetInstalledTools(string toolLocation) | |
{ | ||
Id = match.Groups["packageName"].Value, | ||
Version = match.Groups["packageVersion"].Value, | ||
ShortCode = match.Groups["packageShortCode"].Value | ||
ShortCode = match.Groups["packageShortCode"].Value, | ||
Manifest = match.Groups["packageManifest"].Value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've done no testing so far, but expecting this to be empty and not throw when not matched. |
||
}); | ||
} | ||
} | ||
|
@@ -229,7 +238,7 @@ private void RunDotNetTool(PackageReference package, DirectoryPath toolsFolderDi | |
{ | ||
_log.Warning("dotnet exited with {0}", exitCode); | ||
var output = string.Join(Environment.NewLine, process.GetStandardError()); | ||
_log.Verbose(Verbosity.Diagnostic, "Output:\r\n{0}", output); | ||
_log.Verbose(Verbosity.Diagnostic, "Output:{0}{1}", Environment.NewLine, output); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was another minor bug here. Changed to use |
||
} | ||
} | ||
|
||
|
@@ -245,16 +254,27 @@ private static ProcessArgumentBuilder GetArguments( | |
arguments.Append(Enum.GetName(typeof(DotNetToolOperation), operation).ToLowerInvariant()); | ||
arguments.AppendQuoted(definition.Package); | ||
|
||
if(definition.Parameters.ContainsKey("global")) | ||
if (definition.Parameters.ContainsKey("global")) | ||
{ | ||
arguments.Append("--global"); | ||
} | ||
else if (definition.Parameters.ContainsKey("local")) | ||
{ | ||
arguments.Append("--local"); | ||
} | ||
else | ||
{ | ||
arguments.Append("--tool-path"); | ||
arguments.AppendQuoted(toolDirectoryPath.FullPath); | ||
} | ||
|
||
// Tool manifest | ||
if (definition.Parameters.ContainsKey("tool-manifest")) | ||
{ | ||
arguments.Append("--tool-manifest"); | ||
arguments.AppendQuoted(definition.Parameters["tool-manifest"].First()); | ||
} | ||
|
||
if (operation != DotNetToolOperation.Uninstall) | ||
{ | ||
if (definition.Address != null) | ||
|
@@ -271,14 +291,14 @@ private static ProcessArgumentBuilder GetArguments( | |
} | ||
|
||
// Config File | ||
if(definition.Parameters.ContainsKey("configfile")) | ||
if (definition.Parameters.ContainsKey("configfile")) | ||
{ | ||
arguments.Append("--configfile"); | ||
arguments.AppendQuoted(definition.Parameters["configfile"].First()); | ||
} | ||
|
||
// Whether to ignore failed sources | ||
if(definition.Parameters.ContainsKey("ignore-failed-sources")) | ||
if (definition.Parameters.ContainsKey("ignore-failed-sources")) | ||
{ | ||
arguments.Append("--ignore-failed-sources"); | ||
} | ||
|
@@ -290,7 +310,7 @@ private static ProcessArgumentBuilder GetArguments( | |
arguments.Append(definition.Parameters["framework"].First()); | ||
} | ||
|
||
switch(log.Verbosity) | ||
switch (log.Verbosity) | ||
{ | ||
case Verbosity.Quiet: | ||
arguments.Append("--verbosity"); | ||
|
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.
Not sure what, if anything, can be done here, since dotnet local tools are not put in the user subdirectory like dotnet global tools.
Only idea is they must be in the NuGet cache, but how to get the correct path?
Failing that, would need to revisit how tools work and allow them to be managed behind the dotnet CLI.