⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@jonathanpeppers
Copy link
Member

Context: dotnet/sdk#52492
Context: https://github.com/dotnet/runtime/blob/79609d7dbef01231b0694a356de1d4e5618785cd/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs#L32
Context: https://github.com/dotnet/runtime/blob/79609d7dbef01231b0694a356de1d4e5618785cd/src/mono/mono/metadata/object.c#L8137-L8140
Context: b2332ca

Added a new DeployHotReloadAgentConfiguration MSBuild target that dotnet-watch can use to configure Hot Reload for Android apps.

Notes on runtimes:

  • MonoVM currently passes string.Empty to ProcessStartupHooks and relies on reading STARTUP_HOOKS from AppContext.

  • To fix CoreCLR, we can read DOTNET_STARTUP_HOOKS from environment variables, and pass to ProcessStartupHooks.

  • NativeAOT likely works as-is.

I added/updated tests to verify these changes.

Context: dotnet/sdk#52492
Context: https://github.com/dotnet/runtime/blob/79609d7dbef01231b0694a356de1d4e5618785cd/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs#L32
Context: https://github.com/dotnet/runtime/blob/79609d7dbef01231b0694a356de1d4e5618785cd/src/mono/mono/metadata/object.c#L8137-L8140
Context: b2332ca

Added a new `DeployHotReloadAgentConfiguration` MSBuild target that
`dotnet-watch` can use to configure Hot Reload for Android apps.

Notes on runtimes:

* MonoVM currently passes `string.Empty` to `ProcessStartupHooks` and
  relies on reading `STARTUP_HOOKS` from `AppContext`.

* To fix CoreCLR, we can read `DOTNET_STARTUP_HOOKS` from environment
  variables, and pass to `ProcessStartupHooks`.

* NativeAOT likely works as-is.

I added/updated tests to verify these changes.

<!-- Parse DotNetHotReloadAgentEnvironment (semicolon-separated KEY=VALUE pairs) -->
<ItemGroup Condition=" '$(DotNetHotReloadAgentEnvironment)' != '' ">
<_HotReloadEnvironment Include="$(DotNetHotReloadAgentEnvironment)" />
Copy link
Member

Choose a reason for hiding this comment

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

I realized it'd be easier to make DotNetHotReloadAgentEnvironment an item group.

<DotNetHotReloadAgentEnvironment Include="<name>" Value="<value>"/>

That way we don't need to worry about escaping = and ;.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to use an item group now.

@@ -0,0 +1 @@
DOTNET_STARTUP_HOOKS=StartupHook
Copy link
Member Author

Choose a reason for hiding this comment

The 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.

@@ -0,0 +1,84 @@
<!--
***********************************************************************************************
Microsoft.Android.Sdk.HotReload.targets
Copy link
Member Author

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.

Copy link
Member

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.

Introduces a new test to verify that the DOTNET_STARTUP_HOOKS environment variable is set to 'StartupHook'. Also renames an existing test method for clarity.
Comment on lines +235 to +236
<!-- 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' " />
Copy link
Member Author

Choose a reason for hiding this comment

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

Filed this to fix NativeAOT later:

- DotNetHotReloadAgentStartupHook (Property): Path to Microsoft.Extensions.DotNetDeltaApplier.dll
- DotNetHotReloadAgentEnvironment (ItemGroup): Environment variables as <DotNetHotReloadAgentEnvironment Include="name" Value="value" />
-->
<Target Name="DeployHotReloadAgentConfiguration"
Copy link
Member

Choose a reason for hiding this comment

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

The caller of this target will have to set the Device property, so that we know which device/simulator to deploy to.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 dotnet-run that will prompt for a target framework and device. So, you should get the same prompts and property.

Copy link
Member

Choose a reason for hiding this comment

The 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.


<!-- Add the Hot Reload agent DLL as a reference so it gets deployed -->
<ItemGroup>
<Reference Include="$(DotNetHotReloadAgentStartupHook)" Condition=" Exists('$(DotNetHotReloadAgentStartupHook)') " />
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

Hot Reload isn't supported for trimmed apps.

Copy link
Member Author

Choose a reason for hiding this comment

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

My usage of @(TrimmerRootAssembly) was to test startup hooks in general.

But yeah, hot reload, would break randomly if they decided to use System.Guid, for example, during a reload and it was trimmed away.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants