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

Add path to global.json in SDK resolution error message when it's available #25823

Closed
baronfel opened this issue Jun 3, 2022 · 4 comments
Closed
Assignees
Labels
Area-CLI cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on) good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined.
Milestone

Comments

@baronfel
Copy link
Member

baronfel commented Jun 3, 2022

SDK resolution error messages currently look like this:

Unable to locate the .NET SDK version '{0}' as specified by global.json, please check that the specified version is installed.

This is helpful, but doesn't tell the user which global.json is impacting the resolution. This is often fairly straightforward to find out (since global.json is heirarchical, it must be at or above the working directory of the process invocation), but can be outside of what the user expects.

We should incorporate the path to the global.json into this message when it is available for every message that mentions global.json, for the following named message strings (as possible) (list valid as of this commit):

  • MSBuildSDKDirectoryNotFound
  • MSBuildSmallerThanMinimumVersion
  • NETCoreSDKSmallerThanMinimumRequestedVersion
  • NETCoreSDKSmallerThanMinimumVersionRequiredByVisualStudio
  • UnableToLocateNETCoreSdk
  • GlobalJsonResolutionFailed
  • GlobalJsonResolutionFailedSpecificVersion

Here are the docs for the native API:

//   result
//      Callback invoked to return values. It can be invoked more
//      than once. String values passed are valid only for the
//      duration of a call.
//
//      If resolution succeeds, then result will be invoked with
//      resolved_sdk_dir key and the value will hold the path to
//      the resolved SDK directory.
//
//      If global.json is used, then result will be invoked with
//      global_json_path key and the value will hold the path
//      to global.json. If there was no global.json found,
//      or the contents of global.json did not impact resolution
//      (e.g. no version specified), then result will not be
//      invoked with global_json_path key. This will occur for
//      both resolution success and failure.
//
//      If a specific version is requested (via global.json), then
//      result will be invoked with requested_version key and the
//      value will hold the requested version. This will occur for
//      both resolution success and failure.

Per these docs, if we have a 'requested version' in our result, then a global.json was present, so we should always have the path. Therefore it should be safe to have two variants of most of these messages:

  • one without version/global.json path information
  • one with version/global.json path information

Originally posted by @baronfel in #25733 (comment)

This is related to #24480

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-CLI untriaged Request triage from a team member labels Jun 3, 2022
@baronfel baronfel added good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined. cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on) and removed untriaged Request triage from a team member labels Jun 3, 2022
@baronfel baronfel added this to the 7.0.1xx milestone Jun 3, 2022
@nagilson
Copy link
Member

nagilson commented Jun 7, 2022

If the user has several global.json in the hierarchy, say a/b/c and b/ contains global.json and a/ also contains global.json, do we stop the search at b? It could be something we'd want to warn but it also might be a weird repo in repo setup.

@baronfel
Copy link
Member Author

baronfel commented Jun 7, 2022

It stops at the 'closest' global.json from the Current Working Directory of the dotnet process being invoked, so for VS I believe that's the directory containing the solution that's opened. For VSCode/CLI use that's usually the directory the user executed code . or dotnet from. If we really wanted to reach for the sky, we could add the CWD to the messages, but I think identifying the global.json that ended up being used is a good step on its own.

@dsplaisted
Copy link
Member

This may be fixed by #26904, or at least fixed in some cases

@marcpopMSFT marcpopMSFT modified the milestones: 7.0.2xx, 7.0.3xx Jan 11, 2023
@marcpopMSFT marcpopMSFT modified the milestones: 7.0.3xx, 7.0.4xx Apr 19, 2023
@marcpopMSFT
Copy link
Member

marking as fixed per Daniel's comment above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CLI cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on) good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined.
Projects
None yet
Development

No branches or pull requests

4 participants