Tool installation swallows NuGet warnings.

[peterhuene]

Steps to reproduce

Install a tool that causes NuGet to output warnings, but no errors:

$ dotnet install tool $PKG_WITH_WARNINGS

Expected behavior

The install tool command displays the warnings emitted by NuGet during tool installation.

Actual behavior

No warnings are displayed.

Environment data

$ dotnet --info
.NET Command Line Tools (2.2.0-preview1-007986)

Product Information:
 Version:            2.2.0-preview1-007986
 Commit SHA-1 hash:  e447bae210

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.13
 OS Platform: Darwin
 RID:         osx-x64
 Base Path:   /Users/peterhuene/src/cli/bin/2/osx-x64/dotnet/sdk/2.2.0-preview1-007986/

Microsoft .NET Core Shared Framework Host

  Version  : 2.1.0-preview1-26116-04
  Build    : eeb3a84e2142aba791fe003564b8afd048c45ff9

[peterhuene]

I think the proper fix for this is to allow the nuget output to pass through from the internal restore operation like we do for other commands.

I have a fix for this that adds a verbosity option to the command that is passed through.

[wli3]

I don't like the idea to let nuget output pass through. I prefer to hide NuGet is underneath as possible and "translate" it to global tools specific on consumer side. Say, "restore failed on due to no such package" should translate to "install tool failed, no such tool named blah on nuget.org and myget.dotnet-core". Because we force user to understand nuget, how CLI use nuget to implement global tool and the notion of temp project (which is not the most ideal implementation). It is bad we swallow warnings, but I don't think let it pass through is the best solution at least not the default experience.

[peterhuene]

I don't like the idea of translating specific NuGet warnings and errors into friendlier messages to hide the fact NuGet is being invoked behind the scenes. Not only would it be quite fragile, it makes it harder to diagnose problems that perhaps diagnostic-level output from NuGet could solve (i.e. --verbosity:diag). As a developer, I want "clear and concise by default" output with an option to drink from the diagnostic firehose in case something goes wrong. By swallowing or otherwise translating the output, it prevents me, as a user, from understanding what's going on.

[livarcocc]

I agree. I am not uncomfortable that folks are aware that NuGet is behind the implementation of how we acquire packages. They will have to push their packages to a NuGet feed for things to work anyways.

Plus, if the NuGet messages are not good, then this should be an issue for NuGet to solve and not for us to work around it.

[wli3]

If we assume the user need to understand the nuget is behind, there will be expectations come along with it. The versioning will be similar to nuget (different versions for different project), tools should share libraries, clean nuget cache should also clean tools, people can reference a tool package in csproj. It can help us on some topic, but when the experience diverge, it is more confusing.

[wli3]

@KathleenDollard

[wli3]

https://github.com/dotnet/cli/issues/8418#issuecomment-360302804

A relate issue, how to deal with error like the following?

Install failed. Failed to download package:
NuGet returned:

Failed to restore package.
WorkingDirectory:
Arguments: restore C:\Users\namc\AppData\Local\Temp\xxvuzjml.r2u\tsyuxbkj.kwm.csproj --source .\artifacts\build\ --runtime win10-x64 /p:BaseIntermediateOutputPath=\"C:\Users\namc\.dotnet\tools\dotnet-watch\2.1.0-preview1-t000\"
Output:   Restoring packages for C:\Users\namc\AppData\Local\Temp\xxvuzjml.r2u\tsyuxbkj.kwm.csproj...
  Installing dotnet-watch 2.1.0-preview1-t000.
C:\Users\namc\AppData\Local\Temp\xxvuzjml.r2u\tsyuxbkj.kwm.csproj : error NU1202: Package dotnet-watch 2.1.0-preview1-t000 is not compatible with netcoreapp2.1 (.NETCoreApp,Version=v2.1) / win10-x64. Package dotnet-watch 2.1.0-preview1-t000 supports: netcoreapp2.1 (.NETCoreApp,Version=v2.1) / any
  Restore failed in 263.85 ms for C:\Users\namc\AppData\Local\Temp\xxvuzjml.r2u\tsyuxbkj.kwm.csproj.

• if we go with parsing the result route(or a better communication protocol than shell), we give a better error "the tool package is malformed, please inform the producer likely 'Microsoft.NETCore.Platforms is not added as package reference'"

• Or should we keep it that way?

• CLI don't have access to the package before nuget, the only way to check the package by CLI is to restore the package first and check the content. But we would swallow the error for the first restore.

• Or a better NuGet message? Even NuGet mentioned the lacking of Microsoft.NETCore.Platforms, the consumer could get confused with temp project reference with the package reference.

[KathleenDollard]

It is a pretty ugly wall of text. Same thing actually on other verbs like pack though. I'm thinking of the warning on pack using a preview and you don't call your tool a preview.

I especially dislike that the wall of text makes it hard to see our "Install failed" message in the command session. (there is more text before) and we don't have colors.

We might need to talk about this tomorrow. My first instinct is that we need both, with the default being at least a bit abbreviated and a verbose version with info like the above.

Does NuGet have any verbosity options?

[wli3]

(pack error message will go away when we do final release/non preview. Make tool a preview will force the user type dotnet install tool -g mytool --version preview-123 , since nuget will not install preview version by default)

[peterhuene]

With my proposed changes (see dotnet/cli#8486), the wall of text becomes something like:

$ dotnet install tool -g nonexistent
/var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/0om0ol4f.odd/juhaz4r4.stm.csproj : error NU1101: Unable to find package nonexistent. No packages exist with this id in source(s): /Users/peterhuene/src/cli/bin/2/osx-x64/dotnet/sdk/NuGetFallbackFolder, nuget.org
The tool package could not be restored.
Tool 'nonexistent' failed to install.

With the verbosity set to normal (for example, if you were trying to diagnose a package source issue):

$ dotnet install tool -g nonexistent -v:n
Build started 1/26/18 3:19:00 PM.
     1>Project "/var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj" on node 1 (Restore target(s)).
     1>Restore:
         Restoring packages for /var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj...
           GET https://api.nuget.org/v3-flatcontainer/nonexistent/index.json
           NotFound https://api.nuget.org/v3-flatcontainer/nonexistent/index.json 1790ms
     1>/var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj : error NU1101: Unable to find package nonexistent. No packages exist with this id in source(s): /Users/peterhuene/src/cli/bin/2/osx-x64/dotnet/sdk/NuGetFallbackFolder, nuget.org
         Committing restore...
         Writing lock file to disk. Path: /Users/peterhuene/.dotnet/tools/nonexistent/ux335hii.mca/project.assets.json
         Restore failed in 2.06 sec for /var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj.

         NuGet Config files used:
             /Users/peterhuene/.nuget/NuGet/NuGet.Config

         Feeds used:
             https://api.nuget.org/v3/index.json
             /Users/peterhuene/src/cli/bin/2/osx-x64/dotnet/sdk/NuGetFallbackFolder
     1>Done Building Project "/var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj" (Restore target(s)) -- FAILED.

Build FAILED.

       "/var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj" (Restore target) (1) ->
       (Restore target) ->
         /var/folders/rl/m1rv1rxd643gpwspqrbxy76r0000gn/T/yniobm3s.itx/kqd54rnv.zcx.csproj : error NU1101: Unable to find package nonexistent. No packages exist with this id in source(s): /Users/peterhuene/src/cli/bin/2/osx-x64/dotnet/sdk/NuGetFallbackFolder, nuget.org

    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:03.01
The tool package could not be restored.
Tool 'nonexistent' failed to install.

Failures related to an invalid tool settings file would end with:

...
Tool 'nonexistent' failed to install. Please contact the tool author for assistance.

The PR also corrects the behavior of showing command line help when we fail to install the tool (despite nothing being wrong with the command line syntax).

[KathleenDollard]

Is it normal for verbosity on the CLI to be something other than "normal" by default?

[KathleenDollard]

Sorry, I missed that this was visibility.

Why aren't we using verbosity for this?

[peterhuene]

We currently do the same default level for run so that we suppress restore/build output by default.

[peterhuene]

Erm, that was a commit message error of mine; it is verbosity. I'll address.

[KathleenDollard]

Back to the question about the big wall of text being "normal" not "verbose"

[peterhuene]

This is just the standard verbosity option used by the other commands which corresponds to (and is simply forwarded as) MSBuild's verbosity option. normal is not as verbose as say detailed or diagnostic, for example.

The default verbosity used in this case is quiet to minimize tool installation output to only NuGet warnings and errors, similar to what we do for other commands such as run and test.