Skip to content

Conversation

@jpobst
Copy link
Contributor

@jpobst jpobst commented Aug 1, 2022

Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6496140

Due to multitargeting of Java.Interop.csproj and parallel builds (no -m:1), we can write the file Java.Interop/JniEnvironment.g.cs twice in a build:

// net7.0
Command = /Users/builder/azdo/_work/1/s/xamarin-android/bin/Release/dotnet/dotnet "/Users/builder/azdo/_work/1/s/xamarin-android/external/Java.Interop/bin/BuildRelease-net7.0/jnienv-gen.dll" Java.Interop/JniEnvironment.g.cs obj/Release/net7.0/jni.c

// netstandard2.0
Command = mono "/Users/builder/azdo/_work/1/s/xamarin-android/external/Java.Interop/bin/BuildRelease/jnienv-gen.exe" Java.Interop/JniEnvironment.g.cs obj/Release/netstandard2.0/jni.c

As a race condition, this can cause the generation invocations to clobber each other, or to re-generate the file while the other is trying to compile with it:

/Users/builder/azdo/_work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniEnvironment.g.cs(6926,21): error CS1027: #endif directive expected [/Users/builder/azdo/_work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop.csproj]

Fix this by moving the file to $(IntermediateOutputPath) so that each can have their own copy.

/>
<ItemGroup>
<Compile Include="$([System.IO.Path]::Combine('Java.Interop','JniEnvironment.g.cs'))" KeepDuplicates="false" />
<Compile Include="$(IntermediateOutputPath)JniEnvironment.g.cs" KeepDuplicates="false" />
Copy link
Member

Choose a reason for hiding this comment

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

Reading 99f8990, we might not need the %(KeepDuplicates) anymore. I don't think the default C# wildcards would include it.

But we could also just leave this for now.

@jpobst jpobst marked this pull request as ready for review August 1, 2022 22:11
@jpobst
Copy link
Contributor Author

jpobst commented Aug 2, 2022

Test XA run: dotnet/android#7226

There are test failures, but I'm betting any effects from this would break the build, not fail tests.

@jpobst jpobst merged commit a5756ca into main Aug 2, 2022
@jpobst jpobst deleted the parallel-jnienv branch August 2, 2022 14:04
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

3 participants