Skip to content
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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jonathanpeppers
Copy link
Member

Context: dotnet/sdk#42155
Context: dotnet/sdk#42240
Fixes: dotnet/sdk#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:

  • ComputeRunArguments depends on Install, so the app is deployed, the <FastDeploy/> MSBuild target runs, etc.

  • $(RunCommand) is a path to adb

  • $(RunArguments) is an shell am start command to launch the main activity.

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.

…target

Context: dotnet/sdk#42155
Context: dotnet/sdk#42240
Fixes: dotnet/sdk#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:

* `ComputeRunArguments` depends on `Install`, so the app is deployed,
  the `<FastDeploy/>` MSBuild target runs, etc.

* `$(RunCommand)` is a path to `adb`

* `$(RunArguments)` is an `shell am start` command to launch the main
  activity.

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`.
Comment on lines -18 to -19
<RunCommand>dotnet</RunCommand>
<RunArguments>build &quot;$(MSBuildProjectFullPath)&quot; -target:Run --configuration &quot;$(Configuration)&quot;</RunArguments>
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 that before, we had no way to parse all the -p arguments passed to dotnet run through to the underlying Run target. We had $(Configuration) hardcoded here, but that was the only one that worked.

@jonathanpeppers

This comment was marked as outdated.

@dellis1972
Copy link
Contributor

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
Copy link
Member Author

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:

<AndroidAdb
ToolExe="$(AdbToolExe)"
ToolPath="$(AdbToolPath)"
AdbTarget="$(AdbTarget)"
Command="shell"
Arguments="am start -S -n &quot;$(_AndroidPackage)/$(AndroidLaunchActivity)&quot;"
/>

@jonathanpeppers jonathanpeppers marked this pull request as ready for review October 31, 2024 16:39
@jonpryor
Copy link
Member

jonpryor commented Nov 8, 2024

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
Copy link
Member Author

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:

  • dotnet build
  • dotnet run --no-build

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?

<Output TaskParameter="ActivityName" PropertyName="AndroidLaunchActivity" />
</GetAndroidActivityName>
<PropertyGroup>
<RunCommand>$(AdbToolExe)</RunCommand>
Copy link
Member

Choose a reason for hiding this comment

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

Elsewhere, we support $(AdbToolPath) + $(AdbToolExe). As-is, it looks like this requires that $(AdbToolExe)/adb be in %PATH%

Copy link
Member

Choose a reason for hiding this comment

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

I think this should do at least two things:

  1. Depend upon the _ResolveSdks target, so that we can use $(_AndroidSdkDirectory)
  2. Have $(RunCommand) use Path.Combine($(_AndroidSdkDirectory), $(AdbToolExe)) when they're not empty.
<_AdbToolExe>$(AdbToolExe)</_AdbToolExe>
<_AdbToolExe Condition=" '$(_AdbToolExe)' == '' and $([MSBuild]::IsOSPlatform('windows')) ">adb.exe</_AdbToolExe>
<_AdbToolExe Condition=" '$(_AdbToolExe)' == '' and !$([MSBuild]::IsOSPlatform('windows')) ">adb</_AdbToolExe>

<RunCommand Condition=" '$(_AndroidSdkDirectory)' != '' ">$([System.IO.Path]::Combine($(_AndroidSdkDirectory), $(_AdbToolExe)))</RunCommand>
<RunCommand Condition=" '$(RunCommand)' == '' ">$(_AdbToolExe)</RunCommand>

This way, we can still invoke adb even if it isn't in $PATH, and we'll use the same Android SDK adb that was used to build the rest of the project.

<Output TaskParameter="ActivityName" PropertyName="AndroidLaunchActivity" />
</GetAndroidActivityName>
<PropertyGroup>
<RunCommand>$(AdbToolExe)</RunCommand>
Copy link
Member

Choose a reason for hiding this comment

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

I think this should do at least two things:

  1. Depend upon the _ResolveSdks target, so that we can use $(_AndroidSdkDirectory)
  2. Have $(RunCommand) use Path.Combine($(_AndroidSdkDirectory), $(AdbToolExe)) when they're not empty.
<_AdbToolExe>$(AdbToolExe)</_AdbToolExe>
<_AdbToolExe Condition=" '$(_AdbToolExe)' == '' and $([MSBuild]::IsOSPlatform('windows')) ">adb.exe</_AdbToolExe>
<_AdbToolExe Condition=" '$(_AdbToolExe)' == '' and !$([MSBuild]::IsOSPlatform('windows')) ">adb</_AdbToolExe>

<RunCommand Condition=" '$(_AndroidSdkDirectory)' != '' ">$([System.IO.Path]::Combine($(_AndroidSdkDirectory), $(_AdbToolExe)))</RunCommand>
<RunCommand Condition=" '$(RunCommand)' == '' ">$(_AdbToolExe)</RunCommand>

This way, we can still invoke adb even if it isn't in $PATH, and we'll use the same Android SDK adb that was used to build the rest of the project.

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.

How to make dotnet run invoke an MSBuild target?
3 participants