-
Notifications
You must be signed in to change notification settings - Fork 525
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
Make sure _GenerateJavaStubs
always generates native assembly files
#9001
base: main
Are you sure you want to change the base?
Conversation
_GenerateJavaStubs
always generates native assembly_GenerateJavaStubs
always generates native assembly files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes a test failure:
GenerateJavaStubsAndAssembly
_GenerateJavaStubs should be skipped!
Expected: True
But was: False
I think a build with no changes should be skipped:
https://github.com/xamarin/xamarin-android/blob/f5fcd4ddc5bacdb9699c50f6dad80627b80d72c8/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs#L976
But it said:
Input file "obj/Debug/android/assets/armeabi-v7a/UnnamedProject.dll" is newer than output file "obj/Debug/android/typemaps.armeabi-v7a.ll".
If we took this change, I think we'd need to touch all the .ll
files, too? Does it currently use some "copy if changed" logic?
@jonathanpeppers yep, it uses |
@@ -1514,6 +1514,7 @@ because xbuild doesn't support framework reference assemblies. | |||
</ItemGroup> | |||
|
|||
<Touch Files="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp" AlwaysCreate="True" /> | |||
<Touch Files="@(_TypeMapAssemblySource)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, do we know what deleted @(_TypeMapAssemblySource)
? Are they missing from FileWrites
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the problem, we have no idea why they are gone. The guess is that some earlier build deleted them (DTB perhaps?) which we don't have build logs for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless of what made the files disappear, I don't think a stamp file in Outputs
is enough, at least in this case. The task inside the target produces files, and we must make sure that those files exist (whether they were removed by us for some reason, or by the developer) and so they must be part of the outputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I wonder if it is this.
Because the _TypeMapAssemblySource
are generated via a Task. If the task does not run, the _TypeMapAssemblySource
will be empty.
On a DTB is _GeneratedJavaStubs
is skipped the task filling in the _TypeMapAssemblySource
ItemGroup will be empty. As a result the FileWrites
entry will not include the files.
In the case where the Task is not run, we need a way to populate the _TypeMapAssemblySource
from the files on disk to make sure they are not deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this chain of dependencies:
<Target Name="_PrepareNativeAssemblySources">
<PrepareAbiItems
BuildTargetAbis="@(_BuildTargetAbis)"
NativeSourcesDir="$(_NativeAssemblySourceDir)"
InstantRunEnabled="$(_InstantRunEnabled)"
Debug="$(AndroidIncludeDebugSymbols)"
Mode="typemap">
<Output TaskParameter="AssemblySources" ItemName="_TypeMapAssemblySource" />
<Output TaskParameter="AssemblyIncludes" ItemName="_TypeMapAssemblyInclude" />
</PrepareAbiItems>
</Target>
<PropertyGroup>
<_GenerateJavaStubsDependsOnTargets>
_SetLatestTargetFrameworkVersion;
_PrepareAssemblies;
_PrepareNativeAssemblySources;
$(_AfterPrepareAssemblies);
</_GenerateJavaStubsDependsOnTargets>
</PropertyGroup>
<Target Name="_GenerateJavaStubs"
DependsOnTargets="$(_GenerateJavaStubsDependsOnTargets);$(BeforeGenerateAndroidManifest)"
Inputs="@(_AndroidMSBuildAllProjects);@(_ResolvedUserMonoAndroidAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"
Outputs="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp">
Will _PrepareNativeAssemblySources
be ignored when _GenerateJavaStubs
Outputs
are deemed up to date?
Fixes: #8967 For some reason, sometimes the `typemap*.ll` files are sometimes removed from the `obj/` directory which leads to build errors similar to: typemaps.x86_64.ll: error: Could not open input file: no such file or directory Files are generated by the `GenerateJavaStubs` tasks, which is invoked by the `_GenerateJavaStubs` target. However, the target doesn't specify the `*.ll` files in its `Outputs` parameter and, therefore, whenever the files are removed but the `_GenerateJavaStubs.stamp` file is newer than the items/files specified in the target's `Inputs` parameter, the native assembly files aren't regenerated leading to the above error. To fix this, we need to add the `typemap*.ll` files to the target's `Outputs` set, thus forcing their regeneration should they be no longer where they are expected to be.
968baaf
to
dc1bf55
Compare
Fixes: #8967
For some reason, sometimes the
typemap*.ll
files are sometimes removed from theobj/
directory which leads to build errors similar to:Files are generated by the
GenerateJavaStubs
tasks, which is invoked by the_GenerateJavaStubs
target. However, the target doesn't specify the*.ll
files in itsOutputs
parameter and, therefore, whenever the files are removed but the_GenerateJavaStubs.stamp
file is newer than the items/files specified in the target'sInputs
parameter, the native assembly files aren't regenerated leading to the above error.To fix this, we need to add the
typemap*.ll
files to the target'sOutputs
set, thus forcing their regeneration should they be no longer where they are expected to be.