-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[mono] Implement public hot reload API #48380
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. |
8b3ff62
to
5230ea2
Compare
Tagging subscribers to this area: @CoffeeFlux Issue DetailsAlso add ApplyUpdateSdb that takes byte[] arguments for use with Contributes to #45689 This is the mono counterpart of #48366
|
5230ea2
to
b969de0
Compare
b969de0
to
71af4d3
Compare
Probably going to conflict with the CoreCLR PR since we're both modifying the |
@joj once this is merged, (and there's a new nightly runtime installer) you will need to update your prototype to use System.Reflection.Metadata.AssemblyExtensions.ApplyUpdateSdb(Assembly assembly, byte[] metadataDelta, byte[] ilDelta, byte[] pdbDelta) |
src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs
Outdated
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
ca629d0
to
5b4d175
Compare
5b4d175
to
8739cd7
Compare
Also add ApplyUpdateSdb that takes byte[] arguments for use with mono/debugger-libs as its awkward to call a ROS<byte> method from the debugger. Contributes to dotnet#45689
…/AssemblyExtensions.cs Co-authored-by: Marek Safar <[email protected]>
34493ca
to
144da10
Compare
@stephentoub I rebased on master picking up @mikem8361 's changes and moved the argument checking to the shared area. Would you mind reviewing again? @marek-safar leaving ApplyUpdateSdb as we discussed for now. |
src/libraries/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
@lambdageek, are you going to enable the invalid arg testing in runtime\src\libraries\System.Runtime.Loader\tests\AssemblyExtensionsTest.cs? By removing the "SkipOnMono" and "ActiveIssue" attributes? FYI, to build/run the test on Windows:
On Linux/MacOS:
|
Enabled the |
It's unfortunate that |
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.Mono.cs
Outdated
Show resolved
Hide resolved
41d3913
to
495b623
Compare
ActiveIssue seems to be working on non-Windows coreclr
Is there anything else that should be done before this is merged? If not could someone approve. |
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs
Outdated
Show resolved
Hide resolved
Hello @lambdageek! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Also add ApplyUpdateSdb that takes byte[] arguments for use with
mono/debugger-libs as its awkward to call a ROS method from the debugger.
Contributes to #45689
This is the mono counterpart of #48366