Skip to content

Annotate Extensions.Options to make it linker friendly - #40294

Merged
eerhardt merged 2 commits into
dotnet:masterfrom
eerhardt:AnnotateOptions
Aug 5, 2020
Merged

Annotate Extensions.Options to make it linker friendly#40294
eerhardt merged 2 commits into
dotnet:masterfrom
eerhardt:AnnotateOptions

Conversation

@eerhardt

@eerhardt eerhardt commented Aug 3, 2020

Copy link
Copy Markdown
Member

I choose to annotate the following interfaces and their implementations:

  • IOptions
  • IOptionsFactory
  • IOptionsMonitorCache
  • IOptionsMonitor
  • IOptionsSnapshot

I didn't annotate AddOptions<TOptions>, OptionsBuilder<TOptions>, Configure, PostConfigure, or Validate because these operations don't actually create an Options class - they just consume one. It is only once someone tries getting an Options from the ServiceProvider when the class actually gets instantiated. And they way they get the Options is through one of the above annotated interfaces.

Fix #40236

@Dotnet-GitSync-Bot

Copy link
Copy Markdown
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost

ghost commented Aug 4, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Comment thread src/libraries/Microsoft.Extensions.Options/tests/TrimmingTests/ConfigureTests.cs Outdated

@davidfowl davidfowl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SWEET 😄

@eerhardt

eerhardt commented Aug 4, 2020

Copy link
Copy Markdown
Member Author

Failure is #40115.

@maryamariyan maryamariyan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think CI also shows apicompat error for linker tests

e.g.:

.packages/microsoft.dotnet.apicompat/5.0.0-beta.20381.6/build/Microsoft.DotNet.ApiCompat.targets(82,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) TypesMustExist : Type 'Microsoft.Extensions.DependencyInjection.DefaultServiceProviderFactory' does not exist in the implementation but it does exist in the contract. [/Users/runner/work/1/s/src/libraries/Microsoft.Extensions.DependencyInjection/src/Microsoft.Extensions.DependencyInjection.csproj]

@eerhardt

eerhardt commented Aug 4, 2020

Copy link
Copy Markdown
Member Author

I think CI also shows apicompat error for linker tests

re-running. I don't think my change caused these errors.

@eerhardt
eerhardt merged commit a1339cc into dotnet:master Aug 5, 2020
@eerhardt
eerhardt deleted the AnnotateOptions branch August 5, 2020 02:13
Jacksondr5 pushed a commit to Jacksondr5/runtime that referenced this pull request Aug 10, 2020
* Annotate Extensions.Options to make it linker friendly

Fix dotnet#40236
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The options subsystem needs linker attributes to preserve constructors on options types

6 participants