Repository navigation
GenerateBindings and AddBindingsToCompile are not run for a project build happening under _ResolveAssemblies causing incremental builds to break #9007
Description
Android framework version
net8.0-android
Affected platform version
34.0.95
Description
In this binlog, there are two builds of Uno.UI.BindingHelper.Android.netcoremobile.csproj with IDs 351 and 88.
The build for 351 is fine and correct (it's resulting from ResolveProjectReferences on SamplesApp.netcoremobile.csproj)
The build for 88 is very problematic, it's resulting from _ResolveAssemblies which sets
Now that this build is happening with _ComputeFilesToPublishForRuntimeIdentifiers being true, the Microsoft.Android.Sdk.BuildOrder.targets isn't imported. Then, CompileDependsOn is missing the GenerateBindings and AddBindingsToCompile targets. This causes _GenerateCompileDependencyCache to not include some files, breaking things VERY badly.
In the above binlog, _GenerateCompileDependencyCache of build 351 is run first, then that of build 88 which overwritten Uno.UI.BindingHelper.Android.netcoremobile.csproj.CoreCompileInputs.cache causing Uno.UI.BindingHelper.Android.netcoremobile to be rebuilt unnecessarily, which then causes almost everything to be rebuilt.
Steps to Reproduce
Binlog shows it all I think. If more info is needed, let me know.
Did you find any workaround?
No response
Relevant log output
No response
Activity
dellis1972 commentedon Jun 6, 2024
@Youssef1313 this all happens within Visual Studio I assume?
Do you get the same issue from the command line?
Youssef1313 commentedon Jun 6, 2024
My testing was from command-line actually. But I assume it's an issue in VS as well.
dellis1972 commentedon Jun 7, 2024
Now that I think this over
AddBinginsToCompileshould only be run on the outer builds. Since the source code will not change (like all the other code). So I would expect to see it run before all the abi specific inner builds run. This is all assuming its an android build of course. I guess it gets more complicated when we have multiple platforms as the outer build, then a platform specific inner build then abi specific inner inner builds.I'm not even sure we have a unit test to cover that as integrating single projects like maui into our repo testing framework has proved very problematic since maui is always being updated and changing.
@jonathanpeppers do you have any thoughts on this one?
Youssef1313 commentedon Jun 7, 2024
I'm not very much following on "why".
To me, not running it will simply cause
_GenerateCompileDependencyCacheto not have all the.csinputs, which then causes a lot of trouble.jonathanpeppers commentedon Jun 7, 2024
Generally, the Android inner build only runs
<ILLink/>and the AOT compiler. I don't think it even would run the C# compiler at all? Doesn't seem likeAddBindingsToCompileshould run in an inner build?See this for some details:
I've not had a chance to look into this one to see what might be going wrong here. I've been focusing on MAUI, unfortunately.
Youssef1313 commentedon Jun 7, 2024
Well, as part of the build happening under
_ResolveAssembliesthere isResolveProjectReferenceswhich then invokesMSBuildhttps://github.com/dotnet/msbuild/blob/98c28cc33dc4671e6d502e41c80c610b54085139/src/Tasks/Microsoft.Common.CurrentVersion.targets#L2133-L2145
jonathanpeppers commentedon Jun 7, 2024
Ok, so maybe we need
$(_ComputeFilesToPublishForRuntimeIdentifiers)to always be unset duringResolveProjectReferences. This could go wrong depending on what order projects are built.There is a
RemovePropertiesargument for the<MSBuild/>task:Looking at:
https://github.com/dotnet/msbuild/blob/98c28cc33dc4671e6d502e41c80c610b54085139/src/Tasks/Microsoft.Common.CurrentVersion.targets#L2123
I guess we could try
$(_GlobalPropertiesToRemoveFromProjectReferences)? It's private...Youssef1313 commentedon Jun 7, 2024
To me it's quite scary to rely on private properties of MSBuild, but it could work (and, well, I sometimes rely on MSBuild internals myself 😄).
jonathanpeppers commentedon Jun 7, 2024
@Youssef1313 can you test if that works around your problem for now?
This is used in a few places:
Youssef1313 commentedon Jun 7, 2024
Sure. I'll test it and let you know.
Youssef1313 commentedon Jun 10, 2024
@jonathanpeppers Thanks. I tested it and it works. However, is it something that can be done on your side instead of consumers having to deal with it?
jonathanpeppers commentedon Jun 18, 2024
Do you have an idea of what project/solution structure would cause this to break? I think next step would be for us to write a test to reproduce the problem.
Youssef1313 commentedon Jun 18, 2024
@jonathanpeppers Unfortunately, I have no minimal repro. But maybe the binlog in the issue could help crafting a repro?