Skip to content
This repository has been archived by the owner on Mar 1, 2021. It is now read-only.

WIP: GH18: Add support for dotnet local tools #19

Closed
wants to merge 1 commit into from
Closed

WIP: GH18: Add support for dotnet local tools #19

wants to merge 1 commit into from

Conversation

gitfool
Copy link
Contributor

@gitfool gitfool commented Feb 24, 2020

WIP to fix #18. No unit tests. Just playing around.

@@ -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])";
Copy link
Contributor Author

@gitfool gitfool Feb 24, 2020

Choose a reason for hiding this comment

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

This regex had a bug; `s should be \s for non-whitespace match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

@@ -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
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've done no testing so far, but expecting this to be empty and not throw when not matched.

@@ -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])";
string pattern = @"(?<packageName>[^\s]+)\s+(?<packageVersion>[^\s]+)\s+(?<packageShortCode>[^\s]+)(?:\s+(?<packageManifest>[^\s]+))?";
Copy link
Contributor Author

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.

@@ -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);
Copy link
Contributor Author

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.

@@ -69,6 +69,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
Copy link
Contributor Author

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.

@gitfool gitfool requested a review from gep13 February 24, 2020 05:42
@augustoproiete
Copy link
Member

@gitfool We've just integrated Cake.DotNetTool.Module (I believe you noticed as I saw you reacted to @devlead's PR)
Feel free to move this WIP PR to the Cake repo if you'd like, at your convenience.

cake-build/cake#3209

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DotNetTool Module: Add support for dotnet local tools
2 participants