-
Notifications
You must be signed in to change notification settings - Fork 565
[hot reload] implement DeployHotReloadAgentConfiguration target
#10699
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
base: main
Are you sure you want to change the base?
Changes from all commits
72b9fc1
5a5fc67
38de42f
8aa3cd7
ec61662
081426d
e2964b4
e7b1edf
81b1f29
1bf0c48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| <!-- | ||
| *********************************************************************************************** | ||
| Microsoft.Android.Sdk.HotReload.targets | ||
|
|
||
| This file contains targets for Hot Reload support in .NET for Android. | ||
| These targets are invoked by dotnet-watch/IDE when Hot Reload starts. | ||
|
|
||
| See: https://github.com/dotnet/sdk/issues/52492 | ||
|
|
||
| *********************************************************************************************** | ||
| --> | ||
|
|
||
| <Project> | ||
|
|
||
| <!-- Always enable startup hooks support when Hot Reload agent is configured --> | ||
| <PropertyGroup Condition=" '$(DotNetHotReloadAgentStartupHook)' != '' "> | ||
| <StartupHookSupport>true</StartupHookSupport> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- | ||
| _AndroidHotReloadAgentConfiguration: | ||
| Configures the Hot Reload agent by: | ||
| a) Adding the Hot Reload agent DLL as a reference | ||
| b) Setting up STARTUP_HOOKS via RuntimeHostConfigurationOption (for MonoVM) | ||
| c) Setting up DOTNET_STARTUP_HOOKS environment variable (for CoreCLR) | ||
| d) Adding additional environment variables from DotNetHotReloadAgentEnvironment | ||
| --> | ||
| <Target Name="_AndroidHotReloadAgentConfiguration" | ||
| Condition=" '$(DotNetHotReloadAgentStartupHook)' != '' "> | ||
|
|
||
| <PropertyGroup> | ||
| <_HotReloadAgentAssemblyName>$([System.IO.Path]::GetFileNameWithoutExtension('$(DotNetHotReloadAgentStartupHook)'))</_HotReloadAgentAssemblyName> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Add the Hot Reload agent DLL as a reference so it gets deployed --> | ||
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <ItemGroup> | ||
| <Reference Include="$(DotNetHotReloadAgentStartupHook)" Condition=" Exists('$(DotNetHotReloadAgentStartupHook)') " /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this still work if the app is trimmed? What if the hot reload agent assembly contains references to BCL APIs that were trimmed away?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They have the BCL part setup here: I also did this in a test project that seems to work in all runtimes/configurations: <TrimmerRootAssembly Include="StartupHook" RootMode="All" />We might have to do this, if Debug builds are trimmed on iOS.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot Reload isn't supported for trimmed apps.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My usage of But yeah, hot reload, would break randomly if they decided to use iOS Debug builds run the trimmer, but I bet it is setup to preserve almost everything. We may need to review that behavior. @rolfbjarne it's also worth noting we are mostly targeting CoreCLR for this -- if that helps anything. |
||
| </ItemGroup> | ||
|
|
||
| <!-- Set STARTUP_HOOKS via RuntimeHostConfigurationOption for MonoVM (read by Mono runtime) --> | ||
| <ItemGroup Condition=" '$(UseMonoRuntime)' == 'true' "> | ||
| <RuntimeHostConfigurationOption Include="STARTUP_HOOKS" Value="$(_HotReloadAgentAssemblyName)" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Generate Hot Reload environment file for other runtimes --> | ||
| <ItemGroup> | ||
| <_HotReloadEnvironment Include="DOTNET_STARTUP_HOOKS=$(_HotReloadAgentAssemblyName)" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- <DotNetHotReloadAgentEnvironment Include="<name>" Value="<value>"/> --> | ||
| <ItemGroup> | ||
| <_HotReloadEnvironment Include="@(DotNetHotReloadAgentEnvironment->'%(Identity)=%(Value)')" /> | ||
| </ItemGroup> | ||
|
|
||
| <WriteLinesToFile | ||
| File="$(IntermediateOutputPath)__hotreload_environment__.txt" | ||
| Lines="@(_HotReloadEnvironment)" | ||
| Overwrite="True" | ||
| WriteOnlyWhenDifferent="True" | ||
| /> | ||
|
|
||
| <ItemGroup> | ||
| <AndroidEnvironment Include="$(IntermediateOutputPath)__hotreload_environment__.txt" /> | ||
| <FileWrites Include="$(IntermediateOutputPath)__hotreload_environment__.txt" /> | ||
| </ItemGroup> | ||
|
|
||
| </Target> | ||
|
|
||
| <!-- | ||
| DeployHotReloadAgentConfiguration: | ||
| Entry point target invoked by dotnet-watch/IDE. | ||
| Sets up the Hot Reload agent configuration and deploys to the device. | ||
|
|
||
| This target is discovered by checking if the project has a target named | ||
| "DeployHotReloadAgentConfiguration". If found, dotnet-watch/IDE will | ||
| set the following: | ||
| - DotNetHotReloadAgentStartupHook (Property): Path to Microsoft.Extensions.DotNetDeltaApplier.dll | ||
| - DotNetHotReloadAgentEnvironment (ItemGroup): Environment variables as <DotNetHotReloadAgentEnvironment Include="name" Value="value" /> | ||
| --> | ||
| <Target Name="DeployHotReloadAgentConfiguration" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The caller of this target will have to set the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, after we have the initial part working, I hope to share the code with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot Reload is typically used from an IDE though, and the IDE in question must pass the selected device in this case. |
||
| DependsOnTargets="_AndroidHotReloadAgentConfiguration;DeployToDevice" /> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does Because adding anything to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I just looked at Android's code, and it looks like you effectively end up modifying the apk when running This seems a bit wasteful for us, because this is what would end up happening:
Note how the app is signed twice here, which is not ideal when we're supposedly in a fast path for the debug loop. I would propose something else:
This way we only sign the app bundle once.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here is the new plan:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We have an option to "fast deploy" env vars, and so if something isn't working there or not skipping, it is a bug. |
||
|
|
||
| </Project> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,11 +52,12 @@ | |
| <StartupHookSupport>true</StartupHookSupport> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup Condition=" '$(Configuration)' == 'Debug' "> | ||
| <ItemGroup> | ||
| <!-- trying to track: | ||
| JNI ERROR (app bug): accessed deleted Global 0x3056 | ||
| --> | ||
| <AndroidEnvironment Include="env.txt" /> | ||
| <AndroidEnvironment Include="env.txt" Condition=" '$(Configuration)' == 'Debug' " /> | ||
| <AndroidEnvironment Include="hotreload.env" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
|
|
@@ -231,7 +232,8 @@ | |
| <RuntimeHostConfigurationOption Include="test_bool" Value="true" /> | ||
| <RuntimeHostConfigurationOption Include="test_integer" Value="42" /> | ||
| <RuntimeHostConfigurationOption Include="test_string" Value="foo" /> | ||
| <RuntimeHostConfigurationOption Include="STARTUP_HOOKS" Value="StartupHook" /> | ||
| <!-- Set STARTUP_HOOKS via RuntimeHostConfigurationOption for MonoVM and NativeAOT (read via AppContext.GetData) --> | ||
| <RuntimeHostConfigurationOption Include="STARTUP_HOOKS" Value="StartupHook" Condition=" '$(UseMonoRuntime)' == 'true' or '$(PublishAot)' == 'true' " /> | ||
|
Comment on lines
+235
to
+236
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Filed this to fix NativeAOT later: |
||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition=" '$(AndroidPackageFormat)' != 'aab' "> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| # Environment Variables and system properties | ||
| # debug.mono.log=gref,default | ||
| debug.mono.debug=1 | ||
| DOTNET_STARTUP_HOOKS=StartupHook |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| DOTNET_STARTUP_HOOKS=StartupHook | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note this wasn't being used in some of our on-device tests, because we had: <ItemGroup Condition=" '$(Configuration)' == 'Debug' ">
<!-- trying to track:
JNI ERROR (app bug): accessed deleted Global 0x3056
-->
<AndroidEnvironment Include="env.txt" />I just made a new file for this. |
||
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.
@rolfbjarne do you see anything problematic in this file for iOS?
$(DotNetHotReloadAgentStartupHook)will have the full path to the.dll.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.
The solution for iOS will be rather different, but it doesn't look impossible.