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.

Sign up for GitHub

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

Jump to bottom

Clean improvements for BinObj issues #1741

Closed
wants to merge 1 commit into from
Closed

Clean improvements for BinObj issues #1741

wants to merge 1 commit into from

Conversation

adalon
Copy link
Contributor

@adalon adalon commented May 29, 2018

  1. Clean outputs on nuget install/restore

When building an Android project inside VS and the corresponding NuGet assets file
(packages.config or obj\project.assets.json) is updated, the Clean target is also
executed to make sure that remaining artifacts provided by previously installed packages
are removed.

  1. Ensure all files are included when the Clean target is executed

For now, we're only scheduling the targets when they are executed inside the VS
in order to avoid CI issues and because the reports we have so far are for dev
environments only.

@adalon adalon requested a review from dellis1972 as a code owner May 29, 2018 14:15
jonpryor
jonpryor reviewed May 29, 2018
View reviewed changes
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Outdated
@@ -3077,6 +3077,41 @@ because xbuild doesn't support framework reference assemblies.
</CalculateProjectDependencies>
</Target>

<PropertyGroup Condition="'$(BuildingInsideVisualStudio)' == 'true'">
<EnableCleanAllFiles Condition="'$(EnableCleanAllFiles)' == ''">true</EnableCleanAllFiles>
<EnableCleanOnNuGetPackagesChange Condition="'$(CleanOnNuGetPaEnableCleanOnNuGetPackagesChangeckagesChange)' == ''">true</EnableCleanOnNuGetPackagesChange>
Copy link
Member

Choose a reason for hiding this comment

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

Is CleanOnNuGetPaEnableCleanOnNuGetPackagesChangeckagesChange a typo? That looks like gibberish/like someone pasted a word into the middle of an existing string.

Is this intended to be a "public"/user-overridable property? If not, it should start with a _. If it is public, then it should be documented in Documentation/guides/BuildProcess.md.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a last minute renaming, I will correct it and push the typo fix.

The idea is to have a way of disabling it for the user if for some reason breaks an scenario that we are not aware of. It should not but it's good to have the alternative just in case in my opinion. So it should be public and documented.

jonpryor
jonpryor reviewed May 29, 2018
View reviewed changes
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Outdated
<Target Name="_CleanAllFiles">
<ItemGroup>
<Clean Include="$(OutputPath)**\*.*" />
<Clean Include="$(IntermediateOutputPath)**\*.*" />
Copy link
Member

Choose a reason for hiding this comment

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

Won't this screw up the Design-Time build? I'm reminded of 01be8ac / Issue #1286, in which a Rebuild ("Clean", "Build") would break IntelliSense, as the directories that IntelliSense was using were removed.

The directories that IntelliSense uses includes $(IntermediateOutputPath)\designtime, which this PR would remove. I don't think that is desirable.

Copy link
Member

Choose a reason for hiding this comment

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

Aside lol: the whole reason the unit tests added in 01be8ac pass is because this new behavior only applies when $(BuildingInsideVisualStudio)=True, which isn't the case for the unit tests.

Consequently, the scenario/environment that 01be8ac was attempting to fix/address -- use within Visual Studio -- is precisely the environment that this PR will change, in an untested manner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try it and let you know. We could exclude that folder if necessary.

jonathanpeppers
jonathanpeppers reviewed May 29, 2018
View reviewed changes
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 we should also write a test that verifies theClean target is deleting these files now.

We might also try a design-time build after the Clean in the test.

@adalon
Copy link
Contributor Author

adalon commented May 29, 2018

@jonathanpeppers sure, I pushed the commit to start discussing the proposal fix and I was waiting for Dean (on vacation) for some help with the tests or the environment setup.

@adalon adalon added the do-not-merge PR should not be merged. label May 29, 2018
@adalon adalon force-pushed the BinObj branch 2 times, most recently from 27569b1 to 0a7f6b4 Compare May 29, 2018 16:36
@adalon
Clean improvements for BinObj issues
e8fa158 
1. Clean outputs on nuget install/restore

When building an Android project inside VS and the corresponding NuGet assets file
(packages.config or obj\project.assets.json) is updated, the Clean target is also
executed to make sure that remaining artifacts provided by previously installed packages
are removed.

2. Ensure all files are included when the Clean target is executed

For now, we're only scheduling the targets when they are executed inside the VS
in order to avoid CI issues and because the reports we have so far are for dev
environments only.
@dellis1972
Copy link
Contributor

Looks ok to me. @jonathanpeppers do we know what the VSTS failure is?

@jonathanpeppers
Copy link
Member

Xamarin.Android.sln failed to build on Windows.

You might have to wait for a new bundle or rebase to 7bd8b7d.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jun 22, 2018
@jonathanpeppers
[Xamarin.Android.Build.Tasks] clean intermediates if NuGets change
5618634 
Context: dotnet#1741
Fixes: dotnet#1828

PR dotnet#1741 is a good attempt at fixing this, but...
- It is a little heavy handed and basically "nukes the world".
- It does not reproduce an issue in a test and "fix it", it fixes an
  "unknown" #deletebinobj problem.

I also have a few nitpicks about PR dotnet#1741:
- It only comes into effect if `BuildingInsideVisualStudio` is `True`.
  We should also fix command-line builds.
- The `$(ProjectLockFile).stamp` file isn't added to `FileWrites`.

So let's improve on dotnet#1741! As it had some good ideas, we just need to
narrow its focus.

First, I reproduced a real issue in a test with the following
scenario:
- Create a Xamarin.Forms 2.3.4 app that uses the 25.4.x support
  libraries
- Build it
- Update the NuGets to Xamarin.Forms 3.0.x and the 27.x support
  libraries
- Build again
- Stuff breaks... namely the following message:

    bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(2153,3): error MSB4018: The "GenerateJavaStubs" task failed unexpectedly.
        System.IO.FileNotFoundException: Could not load assembly 'System.IO, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. Perhaps it doesn't exist in the Mono for Android profile?
        File name: 'System.IO.dll'
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 241
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 191
        at Mono.Cecil.MetadataResolver.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\MetadataResolver.cs:line 101
        at Mono.Cecil.ModuleDefinition.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\ModuleDefinition.cs:line 774
        at Mono.Cecil.TypeReference.Resolve() in external\mono\external\cecil\Mono.Cecil\TypeReference.cs:line 280
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.GetBaseType(TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 14
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.<GetTypeAndBaseTypes>d__1.MoveNext() in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 21
        at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.IsSubclassOf(TypeDefinition type, String typeName) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 55
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.AddJavaTypes(List`1 javaTypes, TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 46
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.GetJavaTypes(IEnumerable`1 assemblies, IAssemblyResolver resolver) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 36
        at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res) in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 138
        at Xamarin.Android.Tasks.GenerateJavaStubs.Execute() in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 91
        at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
        at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [bin\TestDebug\temp\BuildAfterUpgradingNuget\UnnamedProject.csproj]

The real problem here though is that many of the intermediate files in
`obj\Debug\lp` (among others) are out of sync. Namely we can see
messages like this from the `ResolveLibraryProjectImports` MSBuild
task in the build log:

    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.animated.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Animated.Vector.Drawable.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.annotations\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Annotations.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.ui\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.UI.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.utils\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.Utils.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.design\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Design.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.fragment\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Fragment.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.media.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Media.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.transition\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Transition.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v4\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v4.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.appcompat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.AppCompat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.cardview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.CardView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.mediarouter\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.MediaRouter.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.palette\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.Palette.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.recyclerview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.RecyclerView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Vector.Drawable.dll: extracted files are up to date

These are *bad*, since the extracted files were from the 25.4.x
support libraries! It is happening because the timestamps of these
system-wide `%UserProfile%\.nuget` NuGet packages are quite old.

So a solution for now, based on dotnet#1741:
- When NuGet packages change
- Clean some subset of files/directories -- not a full `Clean`

I borrowed the logic for calculating `$(_NuGetAssetsFile)` from dotnet#1741,
since it was working pretty well. I added a
`_CleanIntermediateIfNuGetsChange` MSBuild target that runs the
`_CleanMonoAndroidIntermediateDir` target as this was the smallest
deletion I could figure out that fixes the problem. At first I tried
deleting other subsets of files/directories, but couldn't find a
combination that worked.

This change does have some impact on build times, when
`_CleanIntermediateIfNuGetsChange` runs and deletes files, it takes
1-2 seconds on my machine for the test case. There will also be
further build time taken from other targets that run because files
were deleted. However, we would likely prefer a slightly slower,
correct build, than a faster incorrect one... It is certaining going
to be faster than the `Build`, error message, `Rebuild` cycle -- or
nuking `bin` and `obj`.

Down the road, we could consider some other change to refactor how
`obj\Debug\lp` is generated and make it resilient to NuGet changes.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jun 22, 2018
@jonathanpeppers
[Xamarin.Android.Build.Tasks] clean intermediates if NuGets change
6a367a0 
Context: dotnet#1741
Fixes: dotnet#1828

PR dotnet#1741 is a good attempt at fixing this, but...
- It is a little heavy handed and basically "nukes the world".
- It does not reproduce an issue in a test and "fix it", it fixes an
  "unknown" #deletebinobj problem.

I also have a few nitpicks about PR dotnet#1741:
- It only comes into effect if `BuildingInsideVisualStudio` is `True`.
  We should also fix command-line builds.
- The `$(ProjectLockFile).stamp` file isn't added to `FileWrites`.

So let's improve on dotnet#1741! As it had some good ideas, we just need to
narrow its focus.

First, I reproduced a real issue in a test with the following
scenario:
- Create a Xamarin.Forms 2.3.4 app that uses the 25.4.x support
  libraries
- Build it
- Update the NuGets to Xamarin.Forms 3.0.x and the 27.x support
  libraries
- Build again
- Stuff breaks... namely the following message:

    bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(2153,3): error MSB4018: The "GenerateJavaStubs" task failed unexpectedly.
        System.IO.FileNotFoundException: Could not load assembly 'System.IO, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. Perhaps it doesn't exist in the Mono for Android profile?
        File name: 'System.IO.dll'
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 241
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 191
        at Mono.Cecil.MetadataResolver.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\MetadataResolver.cs:line 101
        at Mono.Cecil.ModuleDefinition.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\ModuleDefinition.cs:line 774
        at Mono.Cecil.TypeReference.Resolve() in external\mono\external\cecil\Mono.Cecil\TypeReference.cs:line 280
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.GetBaseType(TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 14
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.<GetTypeAndBaseTypes>d__1.MoveNext() in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 21
        at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.IsSubclassOf(TypeDefinition type, String typeName) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 55
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.AddJavaTypes(List`1 javaTypes, TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 46
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.GetJavaTypes(IEnumerable`1 assemblies, IAssemblyResolver resolver) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 36
        at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res) in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 138
        at Xamarin.Android.Tasks.GenerateJavaStubs.Execute() in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 91
        at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
        at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [bin\TestDebug\temp\BuildAfterUpgradingNuget\UnnamedProject.csproj]

The real problem here though is that many of the intermediate files in
`obj\Debug\lp` (among others) are out of sync. Namely we can see
messages like this from the `ResolveLibraryProjectImports` MSBuild
task in the build log:

    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.animated.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Animated.Vector.Drawable.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.annotations\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Annotations.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.ui\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.UI.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.utils\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.Utils.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.design\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Design.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.fragment\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Fragment.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.media.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Media.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.transition\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Transition.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v4\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v4.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.appcompat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.AppCompat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.cardview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.CardView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.mediarouter\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.MediaRouter.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.palette\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.Palette.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.recyclerview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.RecyclerView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Vector.Drawable.dll: extracted files are up to date

These are *bad*, since the extracted files were from the 25.4.x
support libraries! It is happening because the timestamps of these
system-wide `%UserProfile%\.nuget` NuGet packages are quite old.

So a solution for now, based on dotnet#1741:
- When NuGet packages change
- Clean some subset of files/directories -- not a full `Clean`

I borrowed the logic for calculating `$(_NuGetAssetsFile)` from dotnet#1741,
since it was working pretty well. I added a
`_CleanIntermediateIfNuGetsChange` MSBuild target that runs the
`_CleanMonoAndroidIntermediateDir` target as this was the smallest
deletion I could figure out that fixes the problem. At first I tried
deleting other subsets of files/directories, but couldn't find a
combination that worked.

This change does have some impact on build times, when
`_CleanIntermediateIfNuGetsChange` runs and deletes files, it takes
1-2 seconds on my machine for the test case. There will also be
further build time taken from other targets that run because files
were deleted. However, we would likely prefer a slightly slower,
correct build, than a faster incorrect one... It is certaining going
to be faster than the `Build`, error message, `Rebuild` cycle -- or
nuking `bin` and `obj`.

Down the road, we could consider some other change to refactor how
`obj\Debug\lp` is generated and make it resilient to NuGet changes.
@jonathanpeppers jonathanpeppers mentioned this pull request Jun 22, 2018
Merged
@adalon adalon closed this Jun 25, 2018
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jun 25, 2018
@jonathanpeppers
[Xamarin.Android.Build.Tasks] clean intermediates if NuGets change
0c78e36 
Context: dotnet#1741
Fixes: dotnet#1828

PR dotnet#1741 is a good attempt at fixing this, but...
- It is a little heavy handed and basically "nukes the world".
- It does not reproduce an issue in a test and "fix it", it fixes an
  "unknown" #deletebinobj problem.

I also have a few nitpicks about PR dotnet#1741:
- It only comes into effect if `BuildingInsideVisualStudio` is `True`.
  We should also fix command-line builds.
- The `$(ProjectLockFile).stamp` file isn't added to `FileWrites`.

So let's improve on dotnet#1741! As it had some good ideas, we just need to
narrow its focus.

First, I reproduced a real issue in a test with the following
scenario:
- Create a Xamarin.Forms 2.3.4 app that uses the 25.4.x support
  libraries
- Build it
- Update the NuGets to Xamarin.Forms 3.0.x and the 27.x support
  libraries
- Build again
- Stuff breaks... namely the following message:

    bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(2153,3): error MSB4018: The "GenerateJavaStubs" task failed unexpectedly.
        System.IO.FileNotFoundException: Could not load assembly 'System.IO, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. Perhaps it doesn't exist in the Mono for Android profile?
        File name: 'System.IO.dll'
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 241
        at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 191
        at Mono.Cecil.MetadataResolver.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\MetadataResolver.cs:line 101
        at Mono.Cecil.ModuleDefinition.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\ModuleDefinition.cs:line 774
        at Mono.Cecil.TypeReference.Resolve() in external\mono\external\cecil\Mono.Cecil\TypeReference.cs:line 280
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.GetBaseType(TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 14
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.<GetTypeAndBaseTypes>d__1.MoveNext() in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 21
        at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
        at Java.Interop.Tools.Cecil.TypeDefinitionRocks.IsSubclassOf(TypeDefinition type, String typeName) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 55
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.AddJavaTypes(List`1 javaTypes, TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 46
        at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.GetJavaTypes(IEnumerable`1 assemblies, IAssemblyResolver resolver) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 36
        at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res) in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 138
        at Xamarin.Android.Tasks.GenerateJavaStubs.Execute() in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 91
        at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
        at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [bin\TestDebug\temp\BuildAfterUpgradingNuget\UnnamedProject.csproj]

The real problem here though is that many of the intermediate files in
`obj\Debug\lp` (among others) are out of sync. Namely we can see
messages like this from the `ResolveLibraryProjectImports` MSBuild
task in the build log:

    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.animated.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Animated.Vector.Drawable.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.annotations\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Annotations.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.ui\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.UI.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.utils\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.Utils.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.design\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Design.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.fragment\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Fragment.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.media.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Media.Compat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.transition\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Transition.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v4\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v4.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.appcompat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.AppCompat.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.cardview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.CardView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.mediarouter\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.MediaRouter.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.palette\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.Palette.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.recyclerview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.RecyclerView.dll: extracted files are up to date
    Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Vector.Drawable.dll: extracted files are up to date

These are *bad*, since the extracted files were from the 25.4.x
support libraries! It is happening because the timestamps of these
system-wide `%UserProfile%\.nuget` NuGet packages are quite old.

So a solution for now, based on dotnet#1741:
- When NuGet packages change
- Clean some subset of files/directories -- not a full `Clean`

I borrowed the logic for calculating `$(_NuGetAssetsFile)` from dotnet#1741,
since it was working pretty well. I added a
`_CleanIntermediateIfNuGetsChange` MSBuild target that runs the
`_CleanMonoAndroidIntermediateDir` target as this was the smallest
deletion I could figure out that fixes the problem. At first I tried
deleting other subsets of files/directories, but couldn't find a
combination that worked.

This change does have some impact on build times, when
`_CleanIntermediateIfNuGetsChange` runs and deletes files, it takes
1-2 seconds on my machine for the test case. There will also be
further build time taken from other targets that run because files
were deleted. However, we would likely prefer a slightly slower,
correct build, than a faster incorrect one... It is certaining going
to be faster than the `Build`, error message, `Rebuild` cycle -- or
nuking `bin` and `obj`.

Down the road, we could consider some other change to refactor how
`obj\Debug\lp` is generated and make it resilient to NuGet changes.
jonpryor pushed a commit that referenced this pull request Jun 26, 2018
@jonathanpeppers @jonpryor
[Xamarin.Android.Build.Tasks] clean intermediates if NuGets change (#…
dd36614 
…1878)

Context: #1741
Fixes: #1828

PR #1741 is a good attempt at fixing this, but...

  - It is a little heavy handed and basically "nukes the world".
  - It does not reproduce an issue in a test and "fix it", it fixes
    an "unknown" #deletebinobj problem.

I also have a few nitpicks about PR #1741:

  - It only comes into effect if `BuildingInsideVisualStudio`=True.
    We should also fix command-line builds.
  - The `$(ProjectLockFile).stamp` file isn't added to
    `@(FileWrites)`.

So let's improve on #1741!  As it had some good ideas, we just need
to narrow its focus.

First, I reproduced a real issue in a test with this scenario:

  - Create a Xamarin.Forms 2.3.4 app that uses the 25.4.x support
    libraries
  - Build it
  - Update the NuGets to Xamarin.Forms 3.0.x and the 27.x support
    libraries
  - Build again

Result: the build breaks:

        bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(2153,3): error MSB4018: The "GenerateJavaStubs" task failed unexpectedly.
            System.IO.FileNotFoundException: Could not load assembly 'System.IO, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. Perhaps it doesn't exist in the Mono for Android profile?
            File name: 'System.IO.dll'
            at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 241
            at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 191
            at Mono.Cecil.MetadataResolver.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\MetadataResolver.cs:line 101
            at Mono.Cecil.ModuleDefinition.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\ModuleDefinition.cs:line 774
            at Mono.Cecil.TypeReference.Resolve() in external\mono\external\cecil\Mono.Cecil\TypeReference.cs:line 280
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.GetBaseType(TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 14
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.<GetTypeAndBaseTypes>d__1.MoveNext() in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 21
            at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.IsSubclassOf(TypeDefinition type, String typeName) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 55
            at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.AddJavaTypes(List`1 javaTypes, TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 46
            at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.GetJavaTypes(IEnumerable`1 assemblies, IAssemblyResolver resolver) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 36
            at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res) in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 138
            at Xamarin.Android.Tasks.GenerateJavaStubs.Execute() in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 91
            at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
            at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [bin\TestDebug\temp\BuildAfterUpgradingNuget\UnnamedProject.csproj]

The real problem here though is that many of the intermediate files
in `obj\Debug\lp` (among others) are out of sync.  Namely we can see
messages like this from the `<ResolveLibraryProjectImports/>` MSBuild
task in the build log:

        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.animated.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Animated.Vector.Drawable.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.annotations\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Annotations.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.ui\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.UI.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.utils\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.Utils.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.design\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Design.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.fragment\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Fragment.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.media.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Media.Compat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.transition\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Transition.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v4\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v4.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.appcompat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.AppCompat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.cardview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.CardView.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.mediarouter\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.MediaRouter.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.palette\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.Palette.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.recyclerview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.RecyclerView.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Vector.Drawable.dll: extracted files are up to date

These are *bad*, since the extracted files were from the 25.4.x
support libraries!  This is happening because the timestamps of these
system-wide `%UserProfile%\.nuget` NuGet packages are quite old.

So a solution for now, based on #1741:

  - When NuGet packages change,
  - Clean some subset of files/directories -- not a full `Clean`

I borrowed the logic for calculating `$(_NuGetAssetsFile)` from #1741,
since it was working pretty well. I added a
`_CleanIntermediateIfNuGetsChange` MSBuild target that runs the
`_CleanMonoAndroidIntermediateDir` target as this was the smallest
deletion I could figure out that fixes the problem.  At first I tried
deleting other subsets of files/directories, but couldn't find a
combination that worked.

This change does have some impact on build times, when
`_CleanIntermediateIfNuGetsChange` runs and deletes files, it takes
1-2 seconds on my machine for the test case. There will also be
further build time taken from other targets that run because files
were deleted.  However, we would likely prefer a slightly slower,
correct build, than a faster incorrect one... It is certainly going
to be faster than the `Build`, error message, `Rebuild` cycle -- or
nuking `bin` and `obj`.

Down the road, we could consider some other change to refactor how
`obj\Debug\lp` is generated and make it resilient to NuGet changes.
@jonpryor jonpryor deleted the BinObj branch July 27, 2018 21:07
jonpryor pushed a commit that referenced this pull request Aug 3, 2018
@jonathanpeppers @jonpryor
[Xamarin.Android.Build.Tasks] clean intermediates if NuGets change (#…
bd59cf0 
…1878)

Context: #1741
Fixes: #1828

PR #1741 is a good attempt at fixing this, but...

  - It is a little heavy handed and basically "nukes the world".
  - It does not reproduce an issue in a test and "fix it", it fixes
    an "unknown" #deletebinobj problem.

I also have a few nitpicks about PR #1741:

  - It only comes into effect if `BuildingInsideVisualStudio`=True.
    We should also fix command-line builds.
  - The `$(ProjectLockFile).stamp` file isn't added to
    `@(FileWrites)`.

So let's improve on #1741!  As it had some good ideas, we just need
to narrow its focus.

First, I reproduced a real issue in a test with this scenario:

  - Create a Xamarin.Forms 2.3.4 app that uses the 25.4.x support
    libraries
  - Build it
  - Update the NuGets to Xamarin.Forms 3.0.x and the 27.x support
    libraries
  - Build again

Result: the build breaks:

        bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(2153,3): error MSB4018: The "GenerateJavaStubs" task failed unexpectedly.
            System.IO.FileNotFoundException: Could not load assembly 'System.IO, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. Perhaps it doesn't exist in the Mono for Android profile?
            File name: 'System.IO.dll'
            at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference, ReaderParameters parameters) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 241
            at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference reference) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\DirectoryAssemblyResolver.cs:line 191
            at Mono.Cecil.MetadataResolver.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\MetadataResolver.cs:line 101
            at Mono.Cecil.ModuleDefinition.Resolve(TypeReference type) in external\mono\external\cecil\Mono.Cecil\ModuleDefinition.cs:line 774
            at Mono.Cecil.TypeReference.Resolve() in external\mono\external\cecil\Mono.Cecil\TypeReference.cs:line 280
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.GetBaseType(TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 14
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.<GetTypeAndBaseTypes>d__1.MoveNext() in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 21
            at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
            at Java.Interop.Tools.Cecil.TypeDefinitionRocks.IsSubclassOf(TypeDefinition type, String typeName) in external\Java.Interop\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil\TypeDefinitionRocks.cs:line 55
            at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.AddJavaTypes(List`1 javaTypes, TypeDefinition type) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 46
            at Java.Interop.Tools.JavaCallableWrappers.JavaTypeScanner.GetJavaTypes(IEnumerable`1 assemblies, IAssemblyResolver resolver) in external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\JavaTypeScanner.cs:line 36
            at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res) in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 138
            at Xamarin.Android.Tasks.GenerateJavaStubs.Execute() in src\Xamarin.Android.Build.Tasks\Tasks\GenerateJavaStubs.cs:line 91
            at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
            at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [bin\TestDebug\temp\BuildAfterUpgradingNuget\UnnamedProject.csproj]

The real problem here though is that many of the intermediate files
in `obj\Debug\lp` (among others) are out of sync.  Namely we can see
messages like this from the `<ResolveLibraryProjectImports/>` MSBuild
task in the build log:

        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.animated.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Animated.Vector.Drawable.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.annotations\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Annotations.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Compat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.ui\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.UI.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.core.utils\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Core.Utils.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.design\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Design.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.fragment\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Fragment.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.media.compat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Media.Compat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.transition\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Transition.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v4\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v4.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.appcompat\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.AppCompat.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.cardview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.CardView.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.mediarouter\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.MediaRouter.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.palette\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.Palette.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.v7.recyclerview\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.v7.RecyclerView.dll: extracted files are up to date
        Skipped resource lookup for C:\Users\myuser\.nuget\packages\xamarin.android.support.vector.drawable\27.0.2.1\lib\MonoAndroid81\Xamarin.Android.Support.Vector.Drawable.dll: extracted files are up to date

These are *bad*, since the extracted files were from the 25.4.x
support libraries!  This is happening because the timestamps of these
system-wide `%UserProfile%\.nuget` NuGet packages are quite old.

So a solution for now, based on #1741:

  - When NuGet packages change,
  - Clean some subset of files/directories -- not a full `Clean`

I borrowed the logic for calculating `$(_NuGetAssetsFile)` from #1741,
since it was working pretty well. I added a
`_CleanIntermediateIfNuGetsChange` MSBuild target that runs the
`_CleanMonoAndroidIntermediateDir` target as this was the smallest
deletion I could figure out that fixes the problem.  At first I tried
deleting other subsets of files/directories, but couldn't find a
combination that worked.

This change does have some impact on build times, when
`_CleanIntermediateIfNuGetsChange` runs and deletes files, it takes
1-2 seconds on my machine for the test case. There will also be
further build time taken from other targets that run because files
were deleted.  However, we would likely prefer a slightly slower,
correct build, than a faster incorrect one... It is certainly going
to be faster than the `Build`, error message, `Rebuild` cycle -- or
nuking `bin` and `obj`.

Down the road, we could consider some other change to refactor how
`obj\Debug\lp` is generated and make it resilient to NuGet changes.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonpryor jonpryor jonpryor left review comments

@jonathanpeppers jonathanpeppers jonathanpeppers left review comments

@dellis1972 dellis1972 Awaiting requested review from dellis1972

Assignees
No one assigned
Labels
do-not-merge PR should not be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adalon @dellis1972 @jonathanpeppers @jonpryor