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] remove usage of Before/AfterTargets #2795

Merged
merged 2 commits into from
Mar 6, 2019

Conversation

jonathanpeppers
Copy link
Member

Context: https://github.com/jonathanpeppers/MSBuildIncrementalClean

Through research into MSBuild, there are two problem we generally want
to fix:

  • We have targets we want to run before IncrementalClean. Our
    current implementation uses a private MSBuild target to achieve
    this.
  • We have use of BeforeTargets and AfterTargets, which causes some
    problems.

One problem we have, is that usage of AfterTargets causes C#
compiler errors to produce extra (and confusing errors):

MainActivity.cs(9,7): error CS0246: The type or namespace name 'ClassLibrary1' could not be found (are you missing a using directive or an assembly reference?)

Would also trigger this:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets(2160,5): error MSB4018: The "LinkAssemblies" task failed unexpectedly.
System.IO.FileNotFoundException: Could not load assembly 'App1, Version=0.0.0.0, Culture=neutral, PublicKeyToken='. Perhaps it doesn't exist in the Mono for Android profile?
    File name: 'App1.dll'
    at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters)
    at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.GetAssembly(String fileName)
    at Xamarin.Android.Tasks.LinkAssemblies.Execute(DirectoryAssemblyResolver res)
    at Xamarin.Android.Tasks.LinkAssemblies.Execute()
    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
    at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [C:\Users\joaqu\source\repos\ClassLibrary21\App1\App1.csproj]

And to make matters worse, the noisy/ignorable error is displayed
first in the list in the IDE!

Going through our targets here, there was a decent bit of rework
needed. I also updated our MSBuild "best practices" docs.

IncrementalClean

Using a new technique I figured out:

<PropertyGroup>
  <!--Add to this property as needed here-->
  <_BeforeIncrementalClean>
    _AddFilesToFileWrites;
  </_BeforeIncrementalClean>
  <CoreBuildDependsOn>
    $([MSBuild]::Unescape($(CoreBuildDependsOn.Replace('IncrementalClean;', '$(_BeforeIncrementalClean);IncrementalClean;'))))
  </CoreBuildDependsOn>
</PropertyGroup>

We can specify targets that need to run before IncrementalClean
without using BeforeTargets.

I then went through and fixed the following targets:

  • _AddFilesToFileWrites
  • _CompileDex
  • _PrepareAssemblies

_SetupApplicationJavaClass

This target had:

<Target Name="_SetupApplicationJavaClass" AfterTargets="_ResolveMonoAndroidSdks" DependsOnTargets="$(_BeforeSetupApplicationJavaClass)">

I moved the contents of this target to _ResolveMonoAndroidSdks.

Usage of $(_BeforeSetupApplicationJavaClass) will need to change to
use $(_ResolveMonoAndroidSdksDependsOn) instead.

_ResolveMonoAndroidFramework

This target was empty, and I could not find anything using it.

<Target Name="_ResolveMonoAndroidFramework" DependsOnTargets="GetReferenceAssemblyPaths" />

I removed it, and used GetReferenceAssemblyPaths where applicable.

_SetLatestTargetFrameworkVersion

_CreateAapt2VersionCache was using
AfterTargets="_SetLatestTargetFrameworkVersion", so this target
needed to be reworked.

I renamed _SetLatestTargetFrameworkVersion to _ResolveSdks.

I created a new, empty _SetLatestTargetFrameworkVersion target that
depends on:

  • _ResolveSdks then
  • _CreateAapt2VersionCache

Changes needed in monodroid

  • Usage of $(_BeforeSetupApplicationJavaClass) will need to use
    $(_ResolveMonoAndroidSdksDependsOn)
  • Need to do a general audit for BeforeTargets & AfterTargets

Context: https://github.com/jonathanpeppers/MSBuildIncrementalClean

Through research into MSBuild, there are two problem we generally want
to fix:

* We have targets we want to run *before* `IncrementalClean`. Our
  current implementation uses a private MSBuild target to achieve
  this.
* We have use of `BeforeTargets` and `AfterTargets`, which causes some
  problems.

One problem we have, is that usage of `AfterTargets` causes C#
compiler errors to produce extra (and confusing errors):

    MainActivity.cs(9,7): error CS0246: The type or namespace name 'ClassLibrary1' could not be found (are you missing a using directive or an assembly reference?)

Would also trigger this:

    C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets(2160,5): error MSB4018: The "LinkAssemblies" task failed unexpectedly.
    System.IO.FileNotFoundException: Could not load assembly 'App1, Version=0.0.0.0, Culture=neutral, PublicKeyToken='. Perhaps it doesn't exist in the Mono for Android profile?
        File name: 'App1.dll'
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters)
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.GetAssembly(String fileName)
        at Xamarin.Android.Tasks.LinkAssemblies.Execute(DirectoryAssemblyResolver res)
        at Xamarin.Android.Tasks.LinkAssemblies.Execute()
        at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
        at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [C:\Users\joaqu\source\repos\ClassLibrary21\App1\App1.csproj]

And to make matters worse, the noisy/ignorable error is displayed
first in the list in the IDE!

Going through our targets here, there was a decent bit of rework
needed. I also updated our MSBuild "best practices" docs.

~~ IncrementalClean ~~

Using a new technique I figured out:

    <PropertyGroup>
      <!--Add to this property as needed here-->
      <_BeforeIncrementalClean>
        _AddFilesToFileWrites;
      </_BeforeIncrementalClean>
      <CoreBuildDependsOn>
        $([MSBuild]::Unescape($(CoreBuildDependsOn.Replace('IncrementalClean;', '$(_BeforeIncrementalClean);IncrementalClean;'))))
      </CoreBuildDependsOn>
    </PropertyGroup>

We can specify targets that need to run *before* `IncrementalClean`
without using `BeforeTargets`.

I then went through and fixed the following targets:

* `_AddFilesToFileWrites`
* `_CompileDex`
* `_PrepareAssemblies`

~~ _SetupApplicationJavaClass ~~

This target had:

    <Target Name="_SetupApplicationJavaClass" AfterTargets="_ResolveMonoAndroidSdks" DependsOnTargets="$(_BeforeSetupApplicationJavaClass)">

I moved the contents of this target to `_ResolveMonoAndroidSdks`.

Usage of `$(_BeforeSetupApplicationJavaClass)` will need to change to
use `$(_ResolveMonoAndroidSdksDependsOn)` instead.

~~ _ResolveMonoAndroidFramework ~~

This target was empty, and I could not find anything using it.

    <Target Name="_ResolveMonoAndroidFramework" DependsOnTargets="GetReferenceAssemblyPaths" />

I removed it, and used `GetReferenceAssemblyPaths` where applicable.

~~ _SetLatestTargetFrameworkVersion ~~

`_CreateAapt2VersionCache` was using
`AfterTargets="_SetLatestTargetFrameworkVersion"`, so this target
needed to be reworked.

I renamed `_SetLatestTargetFrameworkVersion` to `_ResolveSdks`.

I created a new, empty `_SetLatestTargetFrameworkVersion` target that
depends on:

* `_ResolveSdks` then
* `_CreateAapt2VersionCache`

~~ Changes needed in monodroid ~~

* Usage of `$(_BeforeSetupApplicationJavaClass)` will need to use
  `$(_ResolveMonoAndroidSdksDependsOn)`
* Need to do a general audit for `BeforeTargets` & `AfterTargets`
@jonpryor
Copy link
Member

jonpryor commented Mar 6, 2019

The macOS PR Release Build failed for two reasons:

  1. Commit e138034 "renamed" make package-test-errors to make package-test-results, but the Freestyle Jenkins builder hasn't been updated to use the new target. (Doh? The error is ignored, but it still means that test results aren't properly packaged.)
  2. The Java.Interop.Dynamic-Tests.dll unit tests crashed.

(1) can be fixed in the Freestyle build definition.

(2) is ignorable.

@dellis1972
Copy link
Contributor

This looks ok. One question I still have is how SignAndroidPackage interacts with IncrementalClean. Is that a problem?

@jonathanpeppers
Copy link
Member Author

@dellis1972 I think I will move onto looking at SignAndroidPackage in another PR.

Theoretically, I'll be able to remove a lot of the <Delete/> calls we have that run during Clean when I do that.

@jonathanpeppers
Copy link
Member Author

The Windows failures seem to be parallel related:

  • One is file sharing on a file we download in another test...
  • Second is NuGet trying to install a .nupkg, but another process is doing it at the same time...

Maybe it's time to figure out something to make these tests more reliable? Most of the issues are around downloading files / and NuGet.

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