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

""ResolveAndroidTooling" task failed unexpectedly." with Visual Studio 2017 version 15.9 Preview if $(AndroidUseLatestPlatformSdk) is true and $(AndroidSdkDirectory) is set to a nonexistent directory #2200

Closed
brendanzagaeski opened this issue Sep 19, 2018 · 0 comments · Fixed by #2204
Labels
vs-sync For internal use only; creates a VSTS "mirror" issue.
Milestone

Comments

@brendanzagaeski
Copy link
Contributor

brendanzagaeski commented Sep 19, 2018

This is a different way of looking at #2195. This might be the more important scenario because it can potentially affect end-user Xamarin.Android app builds.

This is a regression in behavior between Visual Studio 2017 version 15.8.3 and Visual Studio 2017 version 15.9 Preview 2. Admittedly, it only affects a narrow scenario: when the old $(AndroidUseLatestPlatformSdk) property is set to true and $(AndroidSdkDirectory) is explicitly set to a nonexistent directory. This could affect some users who have been using a custom value for $(AndroidSdkDirectory) and who haven't updated their projects to remove $(AndroidUseLatestPlatformSdk), but it does seem somewhat unlikely. On the other hand, maybe the simple fact that $(AndroidUseLatestPlatformSdk) is not currently ignored is enough reason to preserve the original behavior for a nonexistent $(AndroidSdkDirectory) for now.

Steps to Reproduce

  1. Create a new Visual C# > Android > Android App (Xamarin) > Blank App.

  2. Set Project Properties > Application > Compile using Android version to the latest Android SDK version you have installed.

  3. Also set Project Properties > Android Manifest > Target Android version to that same version.

  4. Using a Developer Command Prompt (for Visual Studio 2017 version 15.9 Preview 2), attempt to build the solution from the command line. Set /p:AndroidUseLatestPlatformSdk=true and specify a nonexistent $(AndroidSdkDirectory):

     msbuild /t:Build /p:AndroidUseLatestPlatformSdk=true /p:AndroidSdkDirectory="C:\DoesNotExist" CsharpAndroidApp1.sln
    

Results with Visual Studio 2017 version 15.9 Preview 2

The build fails because the new ResolveAndroidTooling task attempts to use the $(AndroidSdkDirectory) unmodified.

Excerpt from the MSBuild output:

Build FAILED.

"C:\source\CsharpAndroidApp1\CsharpAndroidApp1.sln" (Build target) (1) ->
"C:\source\CsharpAndroidApp1\CsharpAndroidApp1\CsharpAndroidApp1.csproj" (default target) (2) ->
(_SetLatestTargetFrameworkVersion target) ->
  C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets(712,2): error MSB4018: The "ResolveAndroidTooling" task failed unexpectedly.
C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets(712,2): error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\DoesNotExist\platforms'.

Results with Visual Studio 2017 version 15.8.3

The build succeeds.

VS bug #690371

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Sep 20, 2018
…kDirectory)

Fixes: dotnet#2200

In 93ddf96, I split up the `ResolveSdks` MSBuild task.

In doing that I broke a situation:
- `msbuild /p:AndroidUseLatestPlatformSdk=true /p:AndroidSdkDirectory="C:\DoesNotExist" MyApp.csproj`
- This works in 15.8 stable, but not 15.9 or master where 93ddf96 is
  applied

The error message is also rather unpleasant, due to an unhandled
exception:

    (_SetLatestTargetFrameworkVersion target) ->
        error MSB4018: The "ResolveAndroidTooling" task failed unexpectedly.
        error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\DoesNotExist\platforms'.

The cause, is how `$(AndroidSdkDirectory)` is handled as an output:

    <Output TaskParameter="AndroidSdkPath" PropertyName="AndroidSdkDirectory" Condition="'$(AndroidSdkDirectory)' == ''" />

We don't want to overwrite the property the user passed in, but then
we shouldn't *also* use it here:

    <ResolveAndroidTooling
        ...
        AndroidSdkPath="$(AndroidSdkDirectory)"
        ...
    />

So the fix here is:
- Don't pass in `AndroidSdkPath` or `AndroidNdkPath` to
  `ResolveAndroidTooling`
- Use `MonoAndroidHelper.AndroidSdk.AndroidSdkPath` which is still
  configured by the slimmed down `ResolveSdks` MSBuild task

Overall I like this change anyway, because we are removing properties
from `ResolveAndroidTooling`.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Sep 21, 2018
…kDirectory)

Fixes: dotnet#2200

In 93ddf96, I split up the `ResolveSdks` MSBuild task.

In doing that I broke a situation:
- `msbuild /p:AndroidUseLatestPlatformSdk=true /p:AndroidSdkDirectory="C:\DoesNotExist" MyApp.csproj`
- This works in 15.8 stable, but not 15.9 or master where 93ddf96 is
  applied

The error message is also rather unpleasant, due to an unhandled
exception:

    (_SetLatestTargetFrameworkVersion target) ->
        error MSB4018: The "ResolveAndroidTooling" task failed unexpectedly.
        error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\DoesNotExist\platforms'.

The cause, is how `$(AndroidSdkDirectory)` is handled as an output:

    <Output TaskParameter="AndroidSdkPath" PropertyName="AndroidSdkDirectory" Condition="'$(AndroidSdkDirectory)' == ''" />

We don't modify `$(AndroidSdkDirectory)` if it was supplied somehow to
the build.

Then we use `$(AndroidSdkDirectory)` directly here:

    <ResolveAndroidTooling
        ...
        AndroidSdkPath="$(AndroidSdkDirectory)"
        ...
    />

So the fix here is a bit of refactoring:
- Let's use `$(_AndroidSdkDirectory)`, `$(_AndroidNdkDirectory)`, and
  `$(_JavaSdkDirectory)` throughout our targets, and leave the
  original values unchanged. We should remove the non-underscored
  values everywhere else.
- We can remove places we call `<CreateProperty />` to create
  `$(_AndroidSdkDirectory)`, since it is set earlier now.
- I saw several places we are using `<CreateProperty
  Value="$(_JavaSdkDirectory)\bin">`, which uses a double slash. We
  can omit a `\` here.
- We can refactor `GetJavaPlatformJar` in general, it doesn't use this
  property so we can remove it. We can also remove extra log messages.

Then there was one place that introduced a dilemma:

    <_PropertyCacheItems Include="AndroidSdkPath=$(AndroidSdkDirectory)" />
    <_PropertyCacheItems Include="AndroidNdkPath=$(AndroidNdkDirectory)" />
    <_PropertyCacheItems Include="JavaSdkPath=$(JavaSdkDirectory)" />

This happens at project evaluation time, so it's not actually using
the proper value for these properties!

So I reworked the targets using this:
- `_CreatePropertiesCache` now depends on
  `_SetLatestTargetFrameworkVersion`
- `_CreatePropertiesCache` now uses `$(_AndroidSdkDirectory)` and
  friends
- We now take advantage of `<WriteLinesToFile
  WriteOnlyWhenDifferent="True" />`
- We can completely remove the `_ReadPropertiesCache` target

Overall this moves `_SetLatestTargetFrameworkVersion` to happen
earlier, but I believe our build is *more correct* as a result.

Downstream in `monodroid`, we will need to update two targets:
- `ResolveXamarinAndroidTools`
- `InstallPackageAssemblies`

I will investigate if this can be done separately to bumping
`xamarin-android`.
@jonathanpeppers jonathanpeppers added this to the d16-0 milestone Sep 24, 2018
@jonathanpeppers jonathanpeppers added the vs-sync For internal use only; creates a VSTS "mirror" issue. label Sep 24, 2018
jonpryor pushed a commit that referenced this issue Sep 24, 2018
…kDirectory) (#2204)

Fixes: #2200

In 93ddf96, I split up the `ResolveSdks` MSBuild task.

In doing that I broke a scenario:

	msbuild /p:AndroidUseLatestPlatformSdk=true /p:AndroidSdkDirectory="C:\DoesNotExist" MyApp.csproj

This works in 15.8 stable, but not 15.9 or master with 93ddf96 applied.

The error message is also rather unpleasant, due to an unhandled
exception:

	(_SetLatestTargetFrameworkVersion target) ->
	    error MSB4018: The "ResolveAndroidTooling" task failed unexpectedly.
	    error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\DoesNotExist\platforms'.

The cause, is how `$(AndroidSdkDirectory)` is handled as an output:

	<Output TaskParameter="AndroidSdkPath" PropertyName="AndroidSdkDirectory" Condition="'$(AndroidSdkDirectory)' == ''" />

We don't modify `$(AndroidSdkDirectory)` if it was supplied somehow to
the build.

Then we use `$(AndroidSdkDirectory)` directly here:

	<ResolveAndroidTooling
	    ...
	    AndroidSdkPath="$(AndroidSdkDirectory)"
	    ...
	/>

So the fix here is a bit of refactoring:

  - Let's use `$(_AndroidSdkDirectory)`, `$(_AndroidNdkDirectory)`,
    and `$(_JavaSdkDirectory)` throughout our targets, and leave the
    original values unchanged.  We should remove the non-underscored
    values everywhere else.
  - We can remove places we call `<CreateProperty/>` to create
    `$(_AndroidSdkDirectory)`, since it is set earlier now.
  - I saw several places we are using
    `<CreateProperty Value="$(_JavaSdkDirectory)\bin">`, which uses a
    double backslash. We can omit a `\` here.
  - We can refactor `<GetJavaPlatformJar/>` in general, it doesn't
    use this property so we can remove it.
    We can also remove extra log messages.

Then there was one place that introduced a dilemma:

    <_PropertyCacheItems Include="AndroidSdkPath=$(AndroidSdkDirectory)" />
    <_PropertyCacheItems Include="AndroidNdkPath=$(AndroidNdkDirectory)" />
    <_PropertyCacheItems Include="JavaSdkPath=$(JavaSdkDirectory)" />

This happens at project evaluation time, so it's not actually using
the proper value for these properties!

So I reworked the targets using this:

  - The `_CreatePropertiesCache` target now depends on the
    `_SetLatestTargetFrameworkVersion` target.
  - The `_CreatePropertiesCache` target now uses
    `$(_AndroidSdkDirectory)` and friends
  - We now take advantage of
    `<WriteLinesToFile WriteOnlyWhenDifferent="True" />`.
  - We can completely remove the `_ReadPropertiesCache` target

Overall this moves `_SetLatestTargetFrameworkVersion` to happen
earlier, but I believe our build is *more correct* as a result.

Downstream in `monodroid`, we will need to update two targets:

  - `ResolveXamarinAndroidTools`
  - `InstallPackageAssemblies`

I will investigate if this can be done separately to bumping
`xamarin-android`.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
vs-sync For internal use only; creates a VSTS "mirror" issue.
Projects
None yet
2 participants