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

Move AssemblyExtensions.GetApplyUpdateCapabilities to be guarded by Debugger.IsSupported #51994

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

eerhardt
Copy link
Member

We are currently using the Debugger.IsSupported feature switch to trim out hot reload support. So moving this method to only be preserved when Debugger.IsSupported is true.

#51159 tracks using a separate "hot reload" feature switch if we don't want to tie "debugging" and "hot reload" together.

@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Apr 28, 2021
@dotnet-issue-labeler
Copy link

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
Copy link

ghost commented Apr 28, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer
See info in area-owners.md if you want to be subscribed.

Issue Details

We are currently using the Debugger.IsSupported feature switch to trim out hot reload support. So moving this method to only be preserved when Debugger.IsSupported is true.

#51159 tracks using a separate "hot reload" feature switch if we don't want to tie "debugging" and "hot reload" together.

Author: eerhardt
Assignees: -
Labels:

linkable-framework

Milestone: -

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@lambdageek
Copy link
Member

lambdageek commented Apr 28, 2021

Do we run tests in configurations without a debugger? In that case GetApplyUpdateCapabilitiesIsCallable might need to be a [ConditionalFact]

[Fact]
public void GetApplyUpdateCapabilitiesIsCallable()

@eerhardt
Copy link
Member Author

Do we run tests in configurations without a debugger?

The one place that I know of that will trim the test with Debugger.IsSupported=false is the AOT wasm tests. The way we solve this for other tests is to set DebuggerSupport to true in the test project. For example:

<!-- these tests depend on the pdb files -->
<DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(TargetOS)' == 'Browser'">true</DebuggerSupport>

@eerhardt
Copy link
Member Author

Looking at the test code:

var ty = typeof(System.Reflection.Metadata.AssemblyExtensions);
var mi = ty.GetMethod("GetApplyUpdateCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty<Type>());

The trimmer should be able to recognize this reflection pattern, and not trim the GetApplyUpdateCapabilities method from the assembly in the AOT wasm case. So we should be fine here.

@jkotas jkotas merged commit 10c9f15 into dotnet:main Apr 28, 2021
@eerhardt eerhardt deleted the HotReloadDescriptor branch April 28, 2021 19:38
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection.Metadata linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants