-
Notifications
You must be signed in to change notification settings - Fork 5k
Add hot reload apply changes API: AssemblyExtensions.ApplyUpdate #48366
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
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
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. |
Tagging subscribers to this area: @tommcdon Issue DetailsIssue: #45689 Currently Windows only: there will be another PR to enable this API on Linux/MacOS. The Hot reload API will fail if debugging or if the module isn't editable.
|
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
Any tests to add? |
We (Aleksey and I) are going to work on some tests. It will take a while to put them together and I didn't want to hold up getting in API so the dotnet-watch tooling (@pranavkm) can start using it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I anticipate we'll continue looking into the debugger interaction and error handling, but if the code as-is helps unblock anyone it seems like a fine starting point.
fyi @davidwrighton |
It should be straightforward to add basic tests that verify at least some of the error handling as part of this PR.
|
Issue: dotnet#45689 Currently Windows only. Fail hot reload API if debugging.
Allows for better error messaging.
132c1fd
to
71d6be3
Compare
@@ -0,0 +1,35 @@ | |||
// Licensed to the .NET Foundation under one or more agreements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: AssemblyExtensionsTest.cs
may be a better name for this file. The convention is to use the same name for the file and the main type in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Issue: #45689
Currently Windows only: there will be another PR to enable this API on Linux/MacOS.
The Hot reload API will fail if debugging or if the module isn't editable.