dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.94k stars 533 forks source link

[Xamarin.Android.Build.Tasks] implement `dotnet run` with an MSBuild target #9470

Open jonathanpeppers opened 3 weeks ago

jonathanpeppers commented 3 weeks ago

Context: https://github.com/dotnet/sdk/issues/42155 Context: https://github.com/dotnet/sdk/pull/42240 Fixes: https://github.com/dotnet/sdk/issues/31253

The .NET SDK has introduced a new ComputeRunArguments MSBuild target that allows you to set $(RunCommand) and $(RunArguments) in a more dynamic way.

So, on Android:

The new implementation also allows us to use the -p parameter with dotnet run, such as:

dotnet run -bl -p:AdbTarget=-d

This will pass -d to adb, which allows you to select an attached device if an emulator is running.

Previously, we had no way to pass -p arguments to dotnet run.

jonathanpeppers commented 3 weeks ago

I need to add a test, I don't see anything in here that calls DotNetCLI.Run():

https://github.com/dotnet/android/blob/ea6734fa342687deaf5616449cf5c074978d4205/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetCLI.cs#L114

We need a test that calls and verifies the app launched successfully. Maybe a breakpoint, too?

dellis1972 commented 3 weeks ago

How will this effect the monodroid RunActivity and debugging system ? https://github.com/xamarin/monodroid/blob/main/tools/msbuild/Xamarin.Android.Common.Debugging.targets#L201

jonathanpeppers commented 3 weeks ago

The new target here is only invoked by dotnet run, and then the options are to set $(RunCommand) and $(RunArguments) to an adb command.

This won't affect any existing targets, but we could consider refactoring later to try to share more code. I couldn't reuse the <AndroidAdb/> task the way $(RunCommand) is used by the dotnet CLI:

https://github.com/dotnet/android/blob/13b73787aeba349c4e0de7b2e463ffffceef457e/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Application.targets#L23-L29

jonpryor commented 2 weeks ago

What gives me pause is:

ComputeRunArguments depends on Install

which I interpret as meaning "every dotnet run command will implicitly Install the app."

I'm not sure that this is desirable? In a non-fastdev environment, Install takes ~8-9 seconds for an unchanged "hello world" template, because it just re-installs the .apk every time. (The Install and _DeployApk targets don't check that the app has already been installed in any way.)

I think dotnet run shouldn't install-by-default, though I'm overall torn about what semantics should actually be.

Additionally, dotnet run should ensure that the app has been killed before restarting.

jonathanpeppers commented 2 weeks ago

which I interpret as meaning "every dotnet run command will implicitly Install the app."

If we didn't do this, the app would never be installed.

One example is:

It seems like we have to deploy? Otherwise, we need a "deployed" concept in the dotnet-cli.

Additionally, dotnet run should ensure that the app has been killed before restarting.

If the Install target and <FastDeploy/> task do this already, is there another place this is needed?