-
-
Notifications
You must be signed in to change notification settings - Fork 738
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
GH3209: Add support for dotnet local tools #3212
base: develop
Are you sure you want to change the base?
Conversation
@@ -73,6 +73,10 @@ public IReadOnlyCollection<IFile> GetFiles(PackageReference package, PackageType | |||
return GetToolFiles(new DirectoryPath(_environment.GetEnvironmentVariable("USERPROFILE")).Combine(".dotnet/tools"), package); | |||
} | |||
} | |||
else if (package.Parameters.ContainsKey("local")) | |||
{ | |||
return new List<IFile>(); // TODO: implementation |
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.
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 is a good question, getting and executing tools from global NuGet cache is possible quick PoC gist here
https://gist.github.com/devlead/559d9b082c9cd81c5c5d52f94d68380c
Don't know if that helps, but potentially one could return entry points 🤔
@@ -207,7 +215,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 comment
The 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.
@@ -196,7 +204,7 @@ private List<DotNetToolPackage> GetInstalledTools(string toolLocation) | |||
var installedTools = isInstalledProcess.GetStandardOutput().ToList(); | |||
var installedToolNames = new List<DotNetToolPackage>(); | |||
|
|||
const string pattern = @"(?<packageName>[^\s]+)\s+(?<packageVersion>[^\s]+)\s+(?<packageShortCode>[^`s])"; | |||
const 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Optionally match the package manifest, when available, for local tools.
This regex had a bug; `s
should be \s
for non-whitespace match and it also didn't match more than one character.
@@ -237,7 +246,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 comment
The reason will be displayed to describe this comment to others. Learn more.
There was another minor bug here. Changed to use Environment.NewLine
when composing output message.
@augustoproiete it would be good to get some eyes on this and any ideas on how to actually implement it. 😛 |
Fixes #3209; migrated from cake-contrib/Cake.DotNetTool.Module#19.