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

Debugging MSBuild Tasks #8730

Merged
merged 6 commits into from
Feb 19, 2024
Merged

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Feb 16, 2024

One thing that is very useful is the ability to debug your Tasks while they are being run on a build process. This is possible thanks to the MSBUILDDEBUGONSTART environment variable. When set to 2 this will force MSBuild to wait for a debugger connection before continuing. You will see the following prompt.

Waiting for debugger to attach (dotnet PID 13001).  Press enter to continue...

You can then use VS or VSCode to attach to this process and debug you tasks.

In the case of .NET Android we need to do a couple of thing first though. Firstly we need to disable the use of ILRepacker on the Xamarin.Android.Build.Tasks assembly. This is because ILRepacker does NOT handle debug symbols very well. Assemblies it generates seem to be JIT optimized so the debugger will not load the symbols. A new MSBuild property has been introduced to disable this feature while debugging. _ILRepackEnabled can be set as an environment variable which MSBuild will pickup.

make prepare && _ILRepackEnabled=false make jenkins

This will disable the ILRepacker for the build.

You can then start your test app with the dotnet-local script (so it uses your build)

MSBUILDDEBUGONSTART=2 ~/<some xamarin.android checkout>/dotnet-local.sh build -m:1

Once MSBuild starts it will print the following

Waiting for debugger to attach (dotnet PID xxxx).  Press enter to continue...

You need to copy the PID value so we can use this in the IDE. For Visual Studio you can use the Attach to Process menu option, while you have the Xamarin.Android.sln solution open. For VSCode open the workspace then use the Debug MSBuild Task Run and Debug option. You will be prompted for the PID and it will then connect.

Once connected go back to your command prompt and press ENTER so that the MSBuild process can continue.

You will be able to set breakpoints in Tasks (but not Targets) and step through code from this point on.

One thing that is very useful is the ability to debug your Tasks while
they are being run on a build process. This is possible thanks to the
`MSBUILDDEBUGONSTART` environment variable. When set to `2` this will
force MSBuild to wait for a debugger connection before continuing.
You will see the following prompt.

```dotnetcli
Waiting for debugger to attach (dotnet PID 13001).  Press enter to continue...
```

You can then use VS or VSCode to attach to this process and debug you tasks.

In the case of .NET Android we need to do a couple of thing first though. Firstly
we need to disable the use of `ILRepacker` on the `Xamarin.Android.Build.Tasks`
assembly. This is because `ILRepacker` does NOT handle debug symbols very well.
Assemblies it generates seem to be JIT optimized so the debugger will not load
the symbols. A new MSBuild property has been introduced to disable this feature
while debugging. `_ILRepackEnabled` can be set as an environment variable which
MSBuild will pickup.

```dotnetcli
make prepare && _ILRepackEnabled=false make jenkins
```

This will disable the `ILRepacker` for the build.

You can then start your test app with the `dotnet-local` script (so it uses your build)

```dotnetcli
MSBUILDDEBUGONSTART=2 ~/<some xamarin.android checkout>/dotnet-local.sh build -m:1
```

Once MSBuild starts it will print the following

```dotnetcli
Waiting for debugger to attach (dotnet PID xxxx).  Press enter to continue...
```

You need to copy the PID value so we can use this in the IDE. For Visual Studio you can use the `Attach to Process` menu option, while you have the Xamarin.Android.sln solution open. For VSCode open the workspace then use the `Debug MSBuild Task` Run and Debug option. You will be prompted for the PID and it will then connect.

Once connection go back to your command prompt and press ENTER so that the MSBuild process can continue.

You will be able to set breakpoints in Tasks (but not Targets) and step through code from this point on.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

I think this is good to merge, it is a good improvement over what we have. 👍

I find myself commenting out ILRepacker to debug on Windows occasionally.

@@ -271,6 +272,7 @@
</ItemGroup>

<Target Name="ILRepacker"
Condition=" '$(_ILRepackEnabled)' == 'true' "
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we only have 3 assemblies left?
https://github.com/xamarin/xamarin-android/blob/e987ac458536e59a8329a06d5c5d5f4d4ea2c6b6/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets#L268-L270

Can we remove these and not pack them?

We could also consider System.Text.Json over Newtonsoft.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving from Newtonsoft might help. As the others I'm not sure.
We should do that in a new PR

Documentation/guides/MSBuildBestPractices.md Show resolved Hide resolved
@dellis1972 dellis1972 merged commit 0418e7b into dotnet:main Feb 19, 2024
47 checks passed
@dellis1972 dellis1972 deleted the debugmsbuildtasks branch February 19, 2024 13:09
grendello added a commit that referenced this pull request Feb 19, 2024
* main:
  Bump NDK to r26c (#8732)
  Debugging MSBuild Tasks (#8730)
grendello added a commit that referenced this pull request Feb 28, 2024
* main:
  Bump to xamarin/xamarin-android-tools/main@37d79c9 (#8752)
  Bump to dotnet/installer@d070660282 9.0.100-preview.3.24126.2 (#8763)
  Bump to xamarin/java.interop/main@14a9470 (#8766)
  $(AndroidPackVersionSuffix)=preview.3; net9 is 34.99.0.preview.3 (#8765)
  [Mono.Android] Do not dispose request content stream in AndroidMessageHandler (#8764)
  Bump com.android.tools:r8 from 8.2.42 to 8.2.47 (#8761)
  [Mono.Android] fix a set of the "easiest" trimmer warnings (#8731)
  Bump to dotnet/installer@0a73f814e1 9.0.100-preview.2.24122.3 (#8716)
  [ci] Always run the MAUI test job (#8750)
  Add a property required by #8478 (#8749)
  [xamarin-android-tools] import $(LibZipSharpVersion) value (#8738)
  Bump to xamarin/Java.Interop/main@c825dcad (#8701)
  Bump to xamarin/monodroid@cb01503327 (#8742)
  Bump to xamarin/Java.Interop/main@ae65609 (#8744)
  Bring in changes from PR #8478 (#8727)
  [xaprepare] Make 7zip work with "dangerous" symlinks in ZIPs (#8737)
  Bump NDK to r26c (#8732)
  Debugging MSBuild Tasks (#8730)
jonpryor pushed a commit that referenced this pull request Mar 13, 2024
…8707)

Fixes: #8703

#8730 reported that we did not bind
[`HardwareBuffer.create(int width, int height, int format, int layers, long usage)`][0].

Indeed, in the build logs for `src/Mono.Android`, there is a BG8800
warning about it!

	obj/Debug/net9.0/android-34/mcw/api-34.xml(32327,10): warning BG8800:
	  Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member
	  'Android.Hardware.HardwareBuffer.Create (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware.HardwareBufferUsage)'

The BG8800 was generated because we attempted to map `long usage` to
an `enum`, but no enum was created as the values are of type `long`,
not `int`.

Manually create a `long`-based `HardwareBufferUsage` enum:

	[Flags]
	/* partial */ enum HardwareBufferUsage : long {
	    None = 0,
	    UsageComposerOverlay = 0x800,
	    // …
	}

However, `generator` does not support `long` enums, and generates
marshalling code using an `int`.  Thus, we need to manually bind
`HardwareBuffer.create()` and [`HardwareBuffer.getUsage()`][1] so we
can replace the `int` machinery with `long`.

While we're at it, audit all of the `BG8800` warnings that are caused
by improper enumification and fix them:

	warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.Create (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware.Hardw...
	warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.IsSupported (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware....
	warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context'.
	warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context.BindIsolatedService (Android.Content.Intent, Android.App.Bind, java.lang.String, java.util.concurrent.Executor, ...
	warning BG8800: Unknown parameter type 'Android.Graphics.ImageDecoderAllocatorType' for member 'Android.Graphics.ImageDecoder.SetAllocator (Android.Graphics.ImageDecoderAllocatorType)'.
	warning BG8800: Unknown parameter type 'Android.Net.WpsFailureReason' for member 'Android.Net.Wifi.WifiManager.WpsCallback.OnFailed (Android.Net.WpsFailureReason)'.
	warning BG8800: Unknown parameter type 'Android.OS.DeviceTemperatureSource' for member 'Android.OS.HardwarePropertiesManager.GetDeviceTemperatures (Android.OS.DeviceTemperatureType, Android.OS.DeviceTempera...
	warning BG8800: Unknown parameter type 'Android.Telephony.Mbms.DownloadStatus' for member 'Android.Telephony.Mbms.DownloadStatusListener.OnStatusUpdated (Android.Telephony.Mbms.DownloadRequest, Android.Tel...
	warning BG8800: Unknown parameter type 'Android.Telephony.StreamingMethod' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamMethodUpdated (Android.Telephony.StreamingMethod)'.
	warning BG8800: Unknown parameter type 'Android.Telephony.StreamingState' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamStateUpdated (Android.Telephony.StreamingState, Android.Telepho...
	warning BG8800: Unknown parameter type 'Android.Icu.Text.CollatorReorderCodes' for member 'Android.Icu.Text.Collator.GetEquivalentReorderCodes (Android.Icu.Text.CollatorReorderCodes)'.
	warning BG8800: Unknown parameter type 'params Android.Icu.Text.CollatorReorderCodes[]' for member 'Android.Icu.Text.Collator.SetReorderCodes (params Android.Icu.Text.CollatorReorderCodes[])'.

This results in new API being surfaced that was previously not being
bound, requiring updates to
`src/Mono.Android/PublicAPI/API-34/PublicAPI.Unshipped.txt`.

One of these new APIs is [`WpsCallback.OnFailed(WpsFailureReason)`][2],
which is a *new* `abstract` method on an existing non-`abstract` type.
Although this is a breaking change, this type previously could not have
been inherited from as the Java-side `abstract` method would not have
been implemented; consider this C# code:

	// This C# code compiles, no warnings or errors:
	class MyCallback : Android.Net.Wifi.WifiManager.WpsCallback {
	    public override void OnStarted(string? pin) {}
	    public override void OnSucceeded() {}
	}

However, the `.csproj` containing `MyCallback` will fail to build:

	obj/Debug/net8.0-android/android/src/crc64475861335642e0f6/MyCallback.java(4,8):
	  javac error JAVAC0000:
	  error: MyCallback is not abstract and does not override abstract method onFailed(int) in WpsCallback

Thus, add it as an "acceptable breakage".

[0]: https://developer.android.com/reference/android/hardware/HardwareBuffer?hl=en#create(int,%20int,%20int,%20int,%20long)
[1]: https://developer.android.com/reference/android/hardware/HardwareBuffer?hl=en#getUsage()
[2]: https://developer.android.com/reference/android/net/wifi/WifiManager.WpsCallback?hl=en#onFailed(int)
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants