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

Make sure _GenerateJavaStubs always generates native assembly files #9001

Closed
wants to merge 3 commits into from

Conversation

grendello
Copy link
Contributor

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.

@grendello grendello changed the title Make sure _GenerateJavaStubs always generates native assembly Make sure _GenerateJavaStubs always generates native assembly files Jun 4, 2024
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.

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?

@grendello
Copy link
Contributor Author

@jonathanpeppers yep, it uses Files.CopyIfStreamChanged

@@ -1514,6 +1514,7 @@ because xbuild doesn't support framework reference assemblies.
</ItemGroup>

<Touch Files="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp" AlwaysCreate="True" />
<Touch Files="@(_TypeMapAssemblySource)" />
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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?

@grendello grendello force-pushed the dev/grendel/javastubs-typemap-deps branch from 968baaf to dc1bf55 Compare June 5, 2024 16:29
@grendello grendello force-pushed the dev/grendel/javastubs-typemap-deps branch from dc1bf55 to 9fd777d Compare July 8, 2024 09:50
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.
@grendello grendello force-pushed the dev/grendel/javastubs-typemap-deps branch from 9fd777d to 5b3cbd1 Compare July 26, 2024 09:24
@grendello
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Aug 6, 2024
Fixes: dotnet#8967
Context: dotnet#9001

You can cause a build error in .NET 8 by doing:

* Start an x86 or x86_64 emulator
* `dotnet build -t:Run`
* Close the emulator, attach an arm or arm64 device
* `dotnet build -t:Run`

Emits the error:

    C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\Xamarin.Android.Common.targets(2063,3): error XA3006: Could not compile native assembly file: typemaps.x86_64.ll
    stderr | C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: error: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: typemaps.x86_64.ll: error: Could not open input file: no such file or directory

This works fine in Visual Studio, but not at the command-line.

The underlying problem is due to `_GeneratePackageManagerJava` target
running, while the `_GenerateJavaStubs` target *did not*:

    Skipping target "_GenerateJavaStubs" because all output files are up-to-date with respect to the input files.
    ...
    Input file "obj\Debug\net8.0-android\resolvedassemblies.hash" is newer than output file "obj\Debug\net8.0-android\stamp\_GeneratePackageManagerJava.stamp".

These two targets should almost always run together, so we should
ensure that they do.

This problem also doesn't happen .NET 9, as both targets are
invalidated for a different reason:

    Target Name=_LinkAssembliesNoShrink Project=UnnamedProject.csproj
    Building target "_LinkAssembliesNoShrink" completely.
    Output file "obj\Debug\android\assets\x86_64\UnnamedProject.dll" does not exist.

Since, we build per-RID in .NET 9, `obj\Debug\android\assets\x86_64\UnnamedProject.dll`
has an `x86_64` in the path, which is not present in the .NET 8 build.

Reviewing the two sets of `Inputs`:

    <Target Name="_GenerateJavaStubs"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);@(_ResolvedUserMonoAndroidAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"
    ...
    <Target Name="_GeneratePackageManagerJava"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);$(_ResolvedUserAssembliesHashFile);$(MSBuildProjectFile);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"

Let's align them more closely by:

* Track assembly changes in the exact same way:

    $(_ResolvedUserAssembliesHashFile);@(_ResolvedUserMonoAndroidAssemblies);

This way if an assembly is added, removed, or timestamp changed: both
targets are in sync.

* Don't track `$(MSBuildProjectFile)`, this should already be in
  `@(_AndroidMSBuildAllProjects)`.

With this change in place manually, I'm not able to reproduce the
problem any longer.

PR dotnet#9001 may also have fixed this issue, but it could cause targets to
run on every incremental build -- a performance issue.
@jonathanpeppers
Copy link
Member

Closing in favor of #9174

jonathanpeppers added a commit that referenced this pull request Aug 6, 2024
…ets (#9174)

Fixes: #8967
Context: #9001

You can cause a build error in .NET 8 by doing:

* Start an x86 or x86_64 emulator
* `dotnet build -t:Run`
* Close the emulator, attach an arm or arm64 device
* `dotnet build -t:Run`

Emits the error:

    Xamarin.Android.Common.targets(2063,3): error XA3006: Could not compile native assembly file: typemaps.x86_64.ll
    stderr | C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: error: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: typemaps.x86_64.ll: error: Could not open input file: no such file or directory

This works fine in Visual Studio, but not at the command-line.

The underlying problem is due to `_GeneratePackageManagerJava` target
running, while the `_GenerateJavaStubs` target *did not*:

    Skipping target "_GenerateJavaStubs" because all output files are up-to-date with respect to the input files.
    ...
    Input file "obj\Debug\net8.0-android\resolvedassemblies.hash" is newer than output file "obj\Debug\net8.0-android\stamp\_GeneratePackageManagerJava.stamp".

These two targets should almost always run together, so we should
ensure that they do.

This problem also doesn't happen .NET 9, as both targets are
invalidated for a different reason:

    Target Name=_LinkAssembliesNoShrink Project=UnnamedProject.csproj
    Building target "_LinkAssembliesNoShrink" completely.
    Output file "obj\Debug\android\assets\x86_64\UnnamedProject.dll" does not exist.

Since, we build per-RID in .NET 9, `obj\Debug\android\assets\x86_64\UnnamedProject.dll`
has an `x86_64` in the path, which is not present in the .NET 8 build.

Reviewing the two sets of `Inputs`:

    <Target Name="_GenerateJavaStubs"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);@(_ResolvedUserMonoAndroidAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"
    ...
    <Target Name="_GeneratePackageManagerJava"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);$(_ResolvedUserAssembliesHashFile);$(MSBuildProjectFile);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"

Let's align them more closely by:

* Track assembly changes in the exact same way:

    $(_ResolvedUserAssembliesHashFile);@(_ResolvedUserMonoAndroidAssemblies);

This way if an assembly is added, removed, or timestamp changed: both
targets are in sync.

* Don't track `$(MSBuildProjectFile)`, this should already be in
  `@(_AndroidMSBuildAllProjects)`.

With this change in place manually, I'm not able to reproduce the
problem any longer.

PR #9001 may also have fixed this issue, but it could cause targets to
run on every incremental build -- a performance issue.
jonathanpeppers added a commit that referenced this pull request Aug 6, 2024
…ets (#9174)

Fixes: #8967
Context: #9001

You can cause a build error in .NET 8 by doing:

* Start an x86 or x86_64 emulator
* `dotnet build -t:Run`
* Close the emulator, attach an arm or arm64 device
* `dotnet build -t:Run`

Emits the error:

    Xamarin.Android.Common.targets(2063,3): error XA3006: Could not compile native assembly file: typemaps.x86_64.ll
    stderr | C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: error: C:\Program Files\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.113\tools\binutils\bin\llc.exe: typemaps.x86_64.ll: error: Could not open input file: no such file or directory

This works fine in Visual Studio, but not at the command-line.

The underlying problem is due to `_GeneratePackageManagerJava` target
running, while the `_GenerateJavaStubs` target *did not*:

    Skipping target "_GenerateJavaStubs" because all output files are up-to-date with respect to the input files.
    ...
    Input file "obj\Debug\net8.0-android\resolvedassemblies.hash" is newer than output file "obj\Debug\net8.0-android\stamp\_GeneratePackageManagerJava.stamp".

These two targets should almost always run together, so we should
ensure that they do.

This problem also doesn't happen .NET 9, as both targets are
invalidated for a different reason:

    Target Name=_LinkAssembliesNoShrink Project=UnnamedProject.csproj
    Building target "_LinkAssembliesNoShrink" completely.
    Output file "obj\Debug\android\assets\x86_64\UnnamedProject.dll" does not exist.

Since, we build per-RID in .NET 9, `obj\Debug\android\assets\x86_64\UnnamedProject.dll`
has an `x86_64` in the path, which is not present in the .NET 8 build.

Reviewing the two sets of `Inputs`:

    <Target Name="_GenerateJavaStubs"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);@(_ResolvedUserMonoAndroidAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"
    ...
    <Target Name="_GeneratePackageManagerJava"
        ...
        Inputs="@(_AndroidMSBuildAllProjects);$(_ResolvedUserAssembliesHashFile);$(MSBuildProjectFile);$(_AndroidBuildPropertiesCache);@(AndroidEnvironment);@(LibraryEnvironments)"

Let's align them more closely by:

* Track assembly changes in the exact same way:

    $(_ResolvedUserAssembliesHashFile);@(_ResolvedUserMonoAndroidAssemblies);

This way if an assembly is added, removed, or timestamp changed: both
targets are in sync.

* Don't track `$(MSBuildProjectFile)`, this should already be in
  `@(_AndroidMSBuildAllProjects)`.

With this change in place manually, I'm not able to reproduce the
problem any longer.

PR #9001 may also have fixed this issue, but it could cause targets to
run on every incremental build -- a performance issue.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2024
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.

build issue typemaps.x86_64.ll: error: Could not open input file: no such file or directory
3 participants