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

Plumb error messages from sdk resolution through hostfxr_resolve_sdk2 #3662

Closed
nguerrera opened this issue Jun 27, 2019 · 4 comments
Closed

Comments

@nguerrera
Copy link
Contributor

sdk resolution emits errors to stderr, which is sufficient for command line scenarios, but stderr is not captured by VS when resolving SDKs.

(Aside: For a long time, VS would not show the errors returned by an SDK resolver at all, but that was fixed as of VS 16.0)

As of now, the that is shown in VS when sdk resolution fails is overly generic:

https://github.com/dotnet/cli/blob/6b0885139c1f37093ba4d3797f46fe90bf093883/src/Microsoft.DotNet.MSBuildSdkResolver/Strings.resx#L133

Unable to locate the .NET Core SDK. Check that it is installed and that the version specified in global.json (if any) matches the installed version.

Proposed solution:

  1. Add keys for error and warning to https://github.com/dotnet/core-setup/blob/1a10af88e3904b1d9790be38450c23eea683d6ae/src/corehost/cli/fxr/hostfxr.cpp#L141-L145

  2. Change sdk resolver to retain the errors and warnings that are logged only to stderr, and hand them back to callers of hostfxr_resolve_sdk2

  3. Change msbuild sdk resolver to return these back to msbuild / VS.

https://github.com/dotnet/cli/blob/6b0885139c1f37093ba4d3797f46fe90bf093883/src/Microsoft.DotNet.MSBuildSdkResolver/NETCoreSdkResolver.cs#L25

cc @peterhuene @vitek-karas @KathleenDollard

@nguerrera nguerrera changed the title Plumb error messages from sdk resolutions through hostfxr_resolve_sdk2 Plumb error messages from sdk resolution through hostfxr_resolve_sdk2 Jun 27, 2019
@vitek-karas
Copy link
Member

All the hosting layers already have a solution for this. In hostfxr it's here:
https://github.com/dotnet/core-setup/blob/1a10af88e3904b1d9790be38450c23eea683d6ae/src/corehost/cli/fxr/hostfxr.cpp#L391

The hostfxr_set_error_writer sets a thread-local variable which if set means all errors are written to the supplied callback and are not output to stderr. It's the responsibility of the custom error writer to report those errors in some way.

The caller should always remember the previous writer (returned by the set function) and set it back once the operation is done.
hostfxr would propagate the writer to hostpolicy if it makes calls into it as part of the operation, so all hosting errors are reported through the writer.

@vitek-karas
Copy link
Member

We don't have any handling like this for warnings, but we could add something if necessary.

@nguerrera
Copy link
Contributor Author

Oh, cool! I didn't know that was added. (I don't think it was there when hostfxr_resolve_sdk2 was added.)

I will close this and replace it with two different ones:

  1. Allow receiving warnings (at which point I wonder if it should be generalized to all levels).

  2. Use hostfxr_set_error_writer in msbuild sdk resolver. This will go in dotnet/cli.

I would say warnings are much lower priority. So we can do (2) first and see if we really need them.

@nguerrera
Copy link
Contributor Author

@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants