Skip to content

Commit 548e76f

Browse files
committed
Fix an issue with incremental builds.
We have been getting on and off reports of the following errors in Android for quite a while now. ``` Resources/values/styles.xml(2): error APT2260: resource attr/colorPrimary (aka Microsoft.Maui:attr/colorPrimary) not found. ``` It was always very confusing as to why this was happening. No one ever seemed to be able to figure out why these files were deleted. Well it turns out they were not deleted, just not included. While building maui I came across this issue and managed to capture a `.binlog`. It turns out that the `_CleanMonoAndroidIntermediateDir` target was running. And it was deleting certain files. This target was running because `_CleanIntermediateIfNeeded` was running. That was running because the `build.props` file had changed... That was because the NuGet `project.assets.json` file had a new timestamp! On an incremental build.... So it looks like NuGet sometimes touches the `project.assets.json` file even if it does not change. We can't blame them for that as we do similar things. The next confusing thing and the principal cause is that the `libraryprojectimports.cache` was being deleted, probably buy the call to `_CleanMonoAndroidIntermediateDir`. However the `_ResolveLibraryProjectImports.stamp` file was left in place. This means the `_ResolveLibraryProjectImports` does NOT run. So we end up NOT including ANY of the resource `res.zip` files when calling `aapt2`. The `_ResolveLibraryProjectImports` target did not include `libraryprojectimports.cache` in its `Outputs`. So if the file did not exist, but the stamp file did the target would not run. It is confusing because in the `_CleanMonoAndroidIntermediateDir` we delete the `libraryprojectimports.cache` and the entire `$(IntermediateOutputPath)stamp` directory.. So lets include `libraryprojectimports.cache` in the outputs for starters. Then update the `_CreatePropertiesCache` target to use a hash of the `project.assets.json` file rather than a timestamp. This way we really only run the build if something actually changes. One last thing is to put in a backup into `RemoveDirFixed` Task. I was seeing the following error when building maui ``` bin/dotnet/packs/Microsoft.Android.Sdk.Darwin/34.0.113/tools/Xamarin.Android.Common.targets(2582,2): error XARDF7024: System.IO.IOException: Directory not empty : 'artifacts/obj/Controls.Maps/Debug/net8.0-android34.0/lp/' at System.IO.FileSystem.RemoveEmptyDirectory(String fullPath, Boolean topLevel, Boolean throwWhenNotEmpty) at System.IO.FileSystem.RemoveDirectoryRecursive(String fullPath) at Xamarin.Android.Tasks.RemoveDirFixed.RunTask() [src/Controls/Maps/src/Controls.Maps.csproj] ``` Which is apparently something that the call `Directory.Delete("", recursive: true)` can raise! So lets put in a backup path to try to delete the files manually.
1 parent 99ba813 commit 548e76f

File tree

4 files changed

+50
-4
lines changed

4 files changed

+50
-4
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ public override bool RunTask ()
7979
} else {
8080
Log.LogUnhandledException (TaskPrefix, ex);
8181
}
82+
} catch (IOException ex) {
83+
try {
84+
DeleteDirectoryManually (fullPath);
85+
} catch (Exception inner) {
86+
Log.LogUnhandledException (TaskPrefix, ex);
87+
Log.LogUnhandledException (TaskPrefix, inner);
88+
}
8289
} catch (Exception ex) {
8390
Log.LogUnhandledException (TaskPrefix, ex);
8491
}
@@ -90,6 +97,17 @@ public override bool RunTask ()
9097
return !Log.HasLoggedErrors;
9198
}
9299

100+
void DeleteDirectoryManually (string directory)
101+
{
102+
foreach (string file in Directory.GetFiles (directory)) {
103+
File.Delete(file);
104+
}
105+
foreach (string subDirectory in Directory.GetDirectories (directory)) {
106+
DeleteDirectoryManually(subDirectory);
107+
}
108+
Directory.Delete(directory, recursive: true);
109+
}
110+
93111
[Required]
94112
public ITaskItem [] Directories { get; set; }
95113

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ public void CheckNothingIsDeletedByIncrementalClean ([Values (true, false)] bool
105105
File.SetLastWriteTimeUtc (file, DateTime.UtcNow);
106106
}
107107
Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Second should have succeeded");
108+
b.Output.AssertTargetIsNotSkipped ("_CleanMonoAndroidIntermediateDir");
109+
var stampFiles = Path.Combine (intermediate, "stamp", "_ResolveLibraryProjectImports.stamp");
110+
FileAssert.Exists (stampFiles, $"{stampFiles} should exists!");
111+
var libraryProjectImports = Path.Combine (intermediate, "libraryprojectimports.cache");
112+
FileAssert.Exists (libraryProjectImports, $"{libraryProjectImports} should exists!");
108113

109114
//No changes
110115
Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Third should have succeeded");
@@ -827,6 +832,7 @@ public void ResolveLibraryProjectImports ()
827832

828833
b.BuildLogFile = "build2.log";
829834
Assert.IsTrue (b.Build (proj), "second build should have succeeded.");
835+
b.Output.AssertTargetIsNotSkipped ("_ResolveLibraryProjectImports");
830836
FileAssert.Exists (cacheFile);
831837
var actual = ReadCache (cacheFile);
832838
CollectionAssert.AreEqual (actual.Jars.Select (j => j.ItemSpec).OrderBy (j => j),
@@ -867,6 +873,22 @@ public void ResolveLibraryProjectImports ()
867873
foreach (var targetName in targets) {
868874
Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
869875
}
876+
877+
var filesToTouch = new [] {
878+
Path.Combine (intermediate, "build.props"),
879+
Path.Combine (intermediate, $"{proj.ProjectName}.pdb"),
880+
};
881+
foreach (var file in filesToTouch) {
882+
FileAssert.Exists (file);
883+
File.SetLastWriteTimeUtc (file, DateTime.UtcNow);
884+
}
885+
886+
b.BuildLogFile = "build5.log";
887+
Assert.IsTrue (b.Build (proj), "fifth build should have succeeded.");
888+
b.Output.AssertTargetIsNotSkipped ("_CleanMonoAndroidIntermediateDir");
889+
b.Output.AssertTargetIsNotSkipped ("_ResolveLibraryProjectImports");
890+
FileAssert.Exists (cacheFile);
891+
FileAssert.Exists (stamp);
870892
}
871893
}
872894

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -907,13 +907,19 @@ because xbuild doesn't support framework reference assemblies.
907907
<AndroidLinkResources Condition="'$(AndroidLinkResources)' == '' And '$(AndroidIncludeDebugSymbols)' != 'True'">False</AndroidLinkResources>
908908
<_AndroidBuildPropertiesCacheExists Condition=" Exists('$(_AndroidBuildPropertiesCache)') ">True</_AndroidBuildPropertiesCacheExists>
909909
<_NuGetAssetsFile Condition=" Exists('$(ProjectAssetsFile)') ">$(ProjectAssetsFile)</_NuGetAssetsFile>
910-
<_NuGetAssetsFile Condition=" Exists('$(ProjectLockFile)') ">$(ProjectLockFile)</_NuGetAssetsFile>
910+
<_NuGetAssetsFile Condition=" '$(_NuGetAssetsFile)' == '' and Exists('$(ProjectLockFile)') ">$(ProjectLockFile)</_NuGetAssetsFile>
911911
<_NuGetAssetsFile Condition=" '$(_NuGetAssetsFile)' == '' and Exists('packages.config') ">packages.config</_NuGetAssetsFile>
912-
<_NuGetAssetsTimestamp Condition=" '$(_NuGetAssetsFile)' != '' ">$([System.IO.File]::GetLastWriteTime('$(_NuGetAssetsFile)').Ticks)</_NuGetAssetsTimestamp>
913912
<_TypeMapKind Condition=" '$(AndroidIncludeDebugSymbols)' != 'True' ">mvid</_TypeMapKind>
914913
<_TypeMapKind Condition=" '$(AndroidIncludeDebugSymbols)' == 'True' And '$(_InstantRunEnabled)' == 'True' ">strings-files</_TypeMapKind>
915914
<_TypeMapKind Condition=" '$(AndroidIncludeDebugSymbols)' == 'True' And '$(_InstantRunEnabled)' != 'True' ">strings-asm</_TypeMapKind>
916915
</PropertyGroup>
916+
<ComputeHash
917+
Condition=" Exists('$(_NuGetAssetsFile)') "
918+
CopyMetadata="False"
919+
Source="$(_NuGetAssetsFile)"
920+
>
921+
<Output TaskParameter="Output" ItemName="_NuGetAssetsFileHash" />
922+
</ComputeHash>
917923
<ItemGroup>
918924
<!-- List of items we want to trigger a build if changed -->
919925
<_PropertyCacheItems Include="AotAssemblies=$(AotAssemblies)" />
@@ -942,7 +948,7 @@ because xbuild doesn't support framework reference assemblies.
942948
<_PropertyCacheItems Include="OS=$(OS)" />
943949
<_PropertyCacheItems Include="AndroidIncludeDebugSymbols=$(AndroidIncludeDebugSymbols)" />
944950
<_PropertyCacheItems Include="AndroidPackageNamingPolicy=$(AndroidPackageNamingPolicy)" />
945-
<_PropertyCacheItems Include="_NuGetAssetsTimestamp=$(_NuGetAssetsTimestamp)" />
951+
<_PropertyCacheItems Include="_NuGetAssetsFileHash=%(_NuGetAssetsFileHash.Hash)" />
946952
<_PropertyCacheItems Include="TypeMapKind=$(_TypeMapKind)" />
947953
<_PropertyCacheItems Include="AndroidManifestPlaceholders=$(AndroidManifestPlaceholders)" />
948954
<_PropertyCacheItems Include="ProjectFullPath=$(MSBuildProjectFullPath)" />

src/Xamarin.Android.Build.Tasks/Xamarin.Android.EmbeddedResource.targets

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ This file is used by all project types, including binding projects.
3535
<Target Name="_ResolveLibraryProjectImports"
3636
DependsOnTargets="$(CoreResolveReferencesDependsOn)"
3737
Inputs="$(ProjectAssetsFile);$(MSBuildProjectFullPath);@(_MonoAndroidReferencePath);@(_MonoAndroidReferenceDependencyPaths);@(AndroidAarLibrary);$(_AndroidBuildPropertiesCache)"
38-
Outputs="$(_AndroidStampDirectory)_ResolveLibraryProjectImports.stamp">
38+
Outputs="$(_AndroidStampDirectory)_ResolveLibraryProjectImports.stamp;$(_AndroidLibraryProjectImportsCache)">
3939
<ResolveLibraryProjectImports
4040
ContinueOnError="$(DesignTimeBuild)"
4141
AndroidApplication="$(AndroidApplication)"

0 commit comments

Comments
 (0)