[AndroidDotnetToolTask] Use full path to dotnet #7038

pjcollins · 2022-05-26T20:04:33Z

A recent attempt to update Java.Interop to target net7.0 produced some
test failures on the Xamarin.Android side:

Using "ClassParse" task from assembly "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/Xamarin.Android.Build.Tasks.dll".
Task "ClassParse" (TaskId:139)
 Task Parameter:ToolPath=/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools (TaskId:139)
 Task Parameter:OutputFile=obj/Debug/api.xml.class-parse (TaskId:139)
 Task Parameter:SourceJars=/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/android-support-multidex.jar (TaskId:139)
 Using: dotnet /Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/class-parse.dll (TaskId:139)
 [class-parse] response file: obj/Debug/class-parse.rsp (TaskId:139)
 --o="obj/Debug/api.xml.class-parse" (TaskId:139)
 "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/android-support-multidex.jar" (TaskId:139)
 /Users/runner/.dotnet/dotnet /Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/class-parse.dll "@obj/Debug/class-parse.rsp"  (TaskId:139)
 It was not possible to find any compatible framework version (TaskId:139)
 The framework 'Microsoft.NETCore.App', version '7.0.0-preview.5.22271.4' (x64) was not found. (TaskId:139)
   - The following frameworks were found: (TaskId:139)
       3.1.1 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       3.1.3 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       3.1.6 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       3.1.25 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       5.0.2 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       5.0.5 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       5.0.8 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       5.0.17 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
       6.0.5 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139)
  (TaskId:139)
 You can resolve the problem by installing the specified framework and/or SDK. (TaskId:139)
  (TaskId:139)
 The specified framework can be found at: (TaskId:139)
   - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=7.0.0-preview.5.22271.4&arch=x64&rid=osx.12-x64 (TaskId:139)
1:7>/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/Xamarin.Android.Bindings.ClassParse.targets(30,5): error MSB6006: "dotnet" exited with code 150. [/Users/runner/work/1/a/TestRelease/05-25_01.03.40/temp/BuildWithExternalJavaLibrary/BuildWithExternalJavaLibraryBinding/BuildWithExternalJavaLibraryBinding.csproj]

Commit ff7f467 removed the need to have a global .NET 7 preview install
to build and test Xamarin.Android. This appears to expose an issue in
AndroidDotnetToolTask, which uses the latest dotnet in $PATH to run
some of our tools. After upgrading some of these tools (class-parse,
generator) to net7.0, we are no longer able to run them as there is no
net7.0 runtime installed globally.

Improve the AndroidDotnetToolTask to better handle "sandboxed" .NET
installations by using a full path to dotnet if one is found.

pjcollins · 2022-05-26T20:05:51Z

src/Xamarin.Android.Build.Tasks/Tasks/AndroidDotnetToolTask.cs

+						dotnetPath = processPath;
+					}
+				}
+				return File.Exists (dotnetPath) ? dotnetPath : "dotnet";


Maybe this should be a task parameter that is passed in instead? It looks like the $(NetCoreRoot) msbuild property could also be used for this (see dotnet/sdk#20 (comment) and https://github.com/dotnet/sdk/blob/b15a88fb330548bf447d2df4936c4367db346a1a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L148-L154)

The Microsoft.NET.ILLink.targets snippet confuses me (aside: what doesn't confuse me today?), because it's setting various properties when $(DOTNET_HOST_PATH) isn't set. But what happens when $(DOTNET_HOST_PATH) is set?

 <PropertyGroup Condition=" '$(DOTNET_HOST_PATH)' == '' "> <_DotNetHostDirectory>$(NetCoreRoot)</_DotNetHostDirectory> <_DotNetHostFileName>dotnet</_DotNetHostFileName> <_DotNetHostFileName Condition="$([MSBuild]::IsOSPlatform(`Windows`))">dotnet.exe</_DotNetHostFileName> </PropertyGroup>

I ask because the above properties are used elsewhere, e.g. https://github.com/dotnet/sdk/blob/b15a88fb330548bf447d2df4936c4367db346a1a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L185-L186

<ILLink AssemblyPaths="@(ManagedAssemblyToLink)" … ToolExe="$(_DotNetHostFileName)" ToolPath="$(_DotNetHostDirectory)"

So when $(DOTNET_HOST_PATH) is set, those properties won't be set, so… shouldn't it just fail horrifically, as ToolExe and ToolPath are the empty string?

The answer to my confusion appears to be that the <ILLink/> task doesn't use ToolExe and ToolPath:

https://github.com/dotnet/linker/blob/87539d443b197b63b99fabf603e20811bad632c5/src/ILLink.Tasks/LinkTask.cs#L224-L239

partial class ILLink { private const string DotNetHostPathEnvironmentName = "DOTNET_HOST_PATH"; private string _dotnetPath; private string DotNetPath { get { if (!String.IsNullOrEmpty (_dotnetPath)) return _dotnetPath; _dotnetPath = Environment.GetEnvironmentVariable (DotNetHostPathEnvironmentName); if (String.IsNullOrEmpty (_dotnetPath)) throw new InvalidOperationException ($"{DotNetHostPathEnvironmentName} is not set"); return _dotnetPath; } } // … protected override string GenerateFullPathToTool () => DotNetPath; }

which is differently "horrifying"; this always throws if $(DOTNET_HOST_PATH) isn't set, and never uses ToolExe & ToolPath, so… How does this work when $(DOTNET_HOST_PATH) isn't set?

The confusion compounds.

By my understanding GenerateFullPathToTool () is only called when ToolPath and ToolExe do not point to a valid tool: https://github.com/dotnet/msbuild/blob/f1dae6ab690483458d37b8900f1d1e4a5fc72851/src/Utilities/ToolTask.cs#L463-L481

I think what effectively happens here is that $(DOTNET_HOST_PATH) is used if set, and $(_DotNetHostFileName) and $(_DotNetHostDirectory) are set and used as a fallback if not.

jonpryor · 2022-05-27T00:18:54Z

A "vaguely related" "concern" is one of consistency: "Normal" ToolTask usage involves setting the ToolExe and ToolPath properties, a'la <BindingsGenerator/>:

https://github.com/xamarin/xamarin-android/blob/97974f957129364b8d5a374cb6ec633f34bd4ede/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.ClassParse.targets#L44-L45

(There are "only" 48 such uses within src/Xamarin.Android.Build.Tasks…)

But then we have <ClassParse/>, which IS-A ToolTask (ClassParse inherits AndroidDotnetToolTask which inherits AndroidToolTask which inherits ToolTask), and thus I would normally expect that ToolExe and ToolPath are provided, yet it isn't:

https://github.com/xamarin/xamarin-android/blob/97974f957129364b8d5a374cb6ec633f34bd4ede/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.ClassParse.targets#L30-L35

I'm not sure if this means anything at all. We previously established that the <ILLink/> task certainly doesn't follow this convention, appearing to ignore the ToolExe & ToolPath properties. So maybe <ClassParse/>/etc. behavior is "fine". Inconsistent, but fine.

Alternatively, we could "embrace convention":

<PropertyGroup>
  <ClassParseToolExe Condition=" … on .NET … ">class-parse.dll</ClassParseToolExe>
  <ClassParseToolExe Condition=" '$(ClassParseToolExe)' == '' ">class-parse.exe</ClassParseToolExe>
  <ClassParseToolPath>$(MonoAndroidToolsDirectory)</ClassParseToolPath>
</PropertyGroup>
…
<ClassParse
    NetCoreRoot="$(NetCoreRoot)"
    ToolExe="$(ClassParseToolExe)"
    TooPath="$(ClassParseToolPath)"
    …
/>

<ClassParse/> can then use the file extension on ToolTask.ToolExe to determine if mono is needed (.exe extension)ordotnet is needed (.dll extension), and use $(NetCoreRoot) if/when appropriate.

The benefit to "embrace convention" is that if you want to test a custom class-parse w/o replacing the one in your build-tree/install, you can do so by overriding $(ClassParseToolPath)/etc.

pjcollins · 2022-05-31T21:19:36Z

I've updated tasks which inherit from AndroidDotnetToolTask to set both ToolExe and ToolPath parameters. I did not add new *ToolPath properties for each tool, as I believe MonoAndroidToolsDirectory can be overridden to point to a new tool location if desired.

The AndroidDotnetToolTask will attempt to use $(NetCoreRoot) if it is set, and fall back to $(DOTNET_HOST_PATH) or finally a non-rooted dotnet otherwise.

Context: dotnet/java-interop#988 A recent attempt to update Java.Interop to target net7.0 produced some test failures on the Xamarin.Android side: Using "ClassParse" task from assembly "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/Xamarin.Android.Build.Tasks.dll". Task "ClassParse" (TaskId:139) Task Parameter:ToolPath=/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools (TaskId:139) Task Parameter:OutputFile=obj/Debug/api.xml.class-parse (TaskId:139) Task Parameter:SourceJars=/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/android-support-multidex.jar (TaskId:139) Using: dotnet /Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/class-parse.dll (TaskId:139) [class-parse] response file: obj/Debug/class-parse.rsp (TaskId:139) --o="obj/Debug/api.xml.class-parse" (TaskId:139) "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/android-support-multidex.jar" (TaskId:139) /Users/runner/.dotnet/dotnet /Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/class-parse.dll "@obj/Debug/class-parse.rsp" (TaskId:139) It was not possible to find any compatible framework version (TaskId:139) The framework 'Microsoft.NETCore.App', version '7.0.0-preview.5.22271.4' (x64) was not found. (TaskId:139) - The following frameworks were found: (TaskId:139) 3.1.1 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 3.1.3 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 3.1.6 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 3.1.25 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 5.0.2 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 5.0.5 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 5.0.8 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 5.0.17 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) 6.0.5 at [/Users/runner/.dotnet/shared/Microsoft.NETCore.App] (TaskId:139) (TaskId:139) You can resolve the problem by installing the specified framework and/or SDK. (TaskId:139) (TaskId:139) The specified framework can be found at: (TaskId:139) - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=7.0.0-preview.5.22271.4&arch=x64&rid=osx.12-x64 (TaskId:139) 1:7>/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/packs/Microsoft.Android.Sdk.Darwin/33.0.0-ci.pr.gh7028.23/tools/Xamarin.Android.Bindings.ClassParse.targets(30,5): error MSB6006: "dotnet" exited with code 150. [/Users/runner/work/1/a/TestRelease/05-25_01.03.40/temp/BuildWithExternalJavaLibrary/BuildWithExternalJavaLibraryBinding/BuildWithExternalJavaLibraryBinding.csproj] Commit ff7f467 removed the need to have a global .NET 7 preview install to build and test Xamarin.Android. This appears to expose an issue in `AndroidDotnetToolTask`, which uses the latest `dotnet` in $PATH to run some of our tools. After upgrading some of these tools (class-parse, generator) to net7.0, we are no longer able to run them as there is no net7.0 runtime installed globally. Improve the `AndroidDotnetToolTask` to better handle "sandboxed" .NET installations by using a full path to `dotnet` if one is found.

jonathanpeppers

I was looking at a random .binlog and I see both:

DOTNET_HOST_PATH = C:\Program Files\dotnet\dotnet.exe
NetCoreRoot = C:\Program Files\dotnet\

So it seems like this should work.

pjcollins commented May 26, 2022

View reviewed changes

pjcollins mentioned this pull request May 26, 2022

[build] Target net7.0. dotnet/java-interop#988

Merged

pjcollins force-pushed the sandbox-tooltask branch from 77825f1 to 32e0791 Compare May 26, 2022 23:38

pjcollins force-pushed the sandbox-tooltask branch 2 times, most recently from b560032 to 84d51b2 Compare May 31, 2022 21:15

pjcollins mentioned this pull request May 31, 2022

[Java.Interop] Bump to xamarin/java.interop@1bab47d #7028

Merged

pjcollins force-pushed the sandbox-tooltask branch from 84d51b2 to 8cd79a6 Compare May 31, 2022 21:34

pjcollins marked this pull request as ready for review May 31, 2022 21:36

pjcollins requested review from dellis1972 and jonathanpeppers as code owners May 31, 2022 21:36

jonathanpeppers approved these changes May 31, 2022

View reviewed changes

dellis1972 approved these changes Jun 1, 2022

View reviewed changes

jonpryor merged commit 23e245b into dotnet:main Jun 1, 2022

github-actions bot locked and limited conversation to collaborators Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AndroidDotnetToolTask] Use full path to dotnet #7038

[AndroidDotnetToolTask] Use full path to dotnet #7038

Uh oh!

pjcollins commented May 26, 2022

Uh oh!

pjcollins May 26, 2022 •

edited

Loading

Uh oh!

jonpryor May 26, 2022

Uh oh!

jonpryor May 27, 2022

Uh oh!

pjcollins May 31, 2022 •

edited

Loading

Uh oh!

jonpryor commented May 27, 2022

Uh oh!

pjcollins commented May 31, 2022 •

edited

Loading

Uh oh!

jonathanpeppers left a comment

Uh oh!

Uh oh!

[AndroidDotnetToolTask] Use full path to dotnet #7038

[AndroidDotnetToolTask] Use full path to dotnet #7038

Uh oh!

Conversation

pjcollins commented May 26, 2022

Uh oh!

pjcollins May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonpryor May 26, 2022

Choose a reason for hiding this comment

Uh oh!

jonpryor May 27, 2022

Choose a reason for hiding this comment

Uh oh!

pjcollins May 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonpryor commented May 27, 2022

Uh oh!

pjcollins commented May 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pjcollins May 26, 2022 •

edited

Loading

pjcollins May 31, 2022 •

edited

Loading

pjcollins commented May 31, 2022 •

edited

Loading