Skip to content

Conversation

jtschuster
Copy link
Member

@jtschuster jtschuster commented Sep 15, 2025

See dotnet/sdk#50632 for context.

hostfxr_resolve_sdk2 will print error messages to stderr by default. We can add a flag to the enum to allow for silencing these errors when we want to try to resolve an sdk, but a failure to resolve shouldn't be surfaced to the console.

Alternatives:

  • Add a callback parameter for error messages the way we do for returning result paths. This could have a default to print to stderr and still be reasonable to backport.
  • Add a new boolean parameter to silence warnings. I think flags make more sense, especially since there's already a flags parameter.
  • Add a new API that only gets global.json information.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a do_not_print_errors flag to the hostfxr_resolve_sdk2 API to allow callers to suppress error messages when SDK resolution fails. This enables scenarios where SDK resolution is attempted but failures should not be surfaced to the console.

Key Changes:

  • Added a new do_not_print_errors flag (0x2) to the hostfxr_resolve_sdk2_flags_t enum
  • Modified the SDK resolver to respect this flag when printing error messages
  • Added comprehensive tests to verify the flag behavior in both success and failure scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/native/corehost/fxr/hostfxr.cpp Added do_not_print_errors flag to enum and passed it to the SDK resolver
src/installer/tests/HostActivation.Tests/NativeHostApis.cs Added test cases to verify the new flag works correctly
src/installer/tests/Assets/Projects/HostApiInvokerApp/HostFXR.cs Added do_not_print_errors flag to the C# enum definition
docs/design/features/hosting-layer-apis.md Updated API documentation to include the new flag

Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

@jtschuster jtschuster requested a review from Copilot September 15, 2025 18:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@marcpopMSFT
Copy link
Member

Do we know why hostfxr is reporting errors in 50632? In those cases, the SDK is actually found by the host so it feels like the primary bug is that the ResolveSdk api is failing (and secondarily, we don't want duplicate errors from the resolvesdk call and the host when the sdk truly can't be found). We actually don't call that api anywhere else in the SDK from .net core (we do call it in VS from teh resolver running in .net framework_). Everywhere else in core, we call GetAvailableSdks.

@jtschuster
Copy link
Member Author

jtschuster commented Sep 15, 2025

I think it's expected that sdk resolution always fails in that call in the sdk. It's passing an empty string as the dotnet_exe argument and only is trying to get the global.json state.

@agocke agocke requested a review from sbomer September 15, 2025 21:05
Copy link
Member

@sbomer sbomer left a comment

Choose a reason for hiding this comment

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

Implementation LGTM as someone not familiar with the host. I wonder if there's a way to avoid introducing a new public flag, but this makes sense to me given the way the telemetry is currently implemented. I don't have context to tell whether that could easily be changed (for example to only emit the global.json telemetry when actually doing the SDK resolution).

@jtschuster jtschuster merged commit fc5252e into dotnet:main Sep 15, 2025
155 checks passed
@github-project-automation github-project-automation bot moved this to Done in AppModel Sep 15, 2025
@jtschuster
Copy link
Member Author

/backport to release/10.0

Copy link
Contributor

jtschuster added a commit that referenced this pull request Sep 30, 2025
…essages (#119729)" (#120231)

This reverts commit fc5252e (#119729). That change isn't required after sdk reverted the telemetry change, and it adds unnecessarily to the API. The best workaround for similar functionality would be to use hostfxr_set_error_writer to swallow the messages before the call to hostfxr_resolve_sdk2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants