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

[ci] Fail build if any git tracked files were modified. #1288

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jan 9, 2025

Context: dotnet/android#9661

Scenarios where the build unintentionally modifies git tracked files on some platforms are frustrating, so let's prevent them from happening in the first place.

Add a CI check to fail a build if any git tracked files were modified.

Also contains a fix for <GenAPITask/> task always generates files with Windows line endings, which causes git status to show the file as modified on other platforms.

jonpryor added a commit that referenced this pull request Jan 9, 2025
Context: #1288

We're considering adding a check to CI if the build changes any files,
and `src/Java.Base-ref.cs` is one of the primary instigators for such
in-tree changes.

Flush the current output of `Java.Base-ref.cs` when building against
JDK-11, which is the current JDK used on dotnet/java-interop CI.
@jpobst jpobst force-pushed the fail-on-dirty-tree branch from 4924d5b to 1dc667c Compare January 13, 2025 19:00
@jonpryor
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jpobst jpobst marked this pull request as ready for review January 14, 2025 19:25
@jonpryor jonpryor merged commit c86ae26 into main Jan 14, 2025
4 checks passed
@jonpryor jonpryor deleted the fail-on-dirty-tree branch January 14, 2025 20:01
jonpryor added a commit to dotnet/android that referenced this pull request Jan 17, 2025
Context: #9636
Context: dotnet/java-interop@d5dfa0a
Context: xamarin/monodroid@e318861
Context: 130905e
Context: de04316

Changes: dotnet/java-interop@4f06201...d5dfa0a

  * dotnet/java-interop@d5dfa0aa: [Java.Interop] `Java.Lang.Object, Mono.Android` Unification Changes (dotnet/java-interop#1293)
  * dotnet/java-interop@c86ae26c: [ci] Fail build if any git tracked files were modified. (dotnet/java-interop#1288)

In the beginning there was Mono for Android, which had a set of
`Mono.Android.dll` assemblies (one per supported API level), each of
which contained "duplicated" binding logic: each API level had its
own `Java.Lang.Object`, `Android.Runtime.JNIEnv`, etc.

dotnet/java-interop started, in part, as a way to "split out" the
core integration logic, so that it *wouldn't* need to be duplicated
across every assembly.  As part of this, it introduced its own core
abstractions, notably `Java.Interop.IJavaPeerable` and
`Java.Interop.JavaObject`.

When dotnet/java-interop was first introduced into Xamarin.Android,
with xamarin/monodroid@e318861e, the integration was incomplete.
Integration continued with 130905e, allowing unit tests within
`Java.Interop-Tests.dll` to run within Xamarin.Android and
construction of instances of e.g. `JavaInt32Array`, but one large
piece of integration remained:

Move GC bridge code *out* of `Java.Lang.Object`, and instead rely on
`Java.Interop.JavaObject`, turning this:

	namespace Java.Lang {
	    public partial class Object : System.Object, IJavaPeerable /* … */ {
	    }
	}

into this:

	namespace Java.Lang {
	    public partial class Object : Java.Interop.JavaObject, IJavaPeerable /* … */ {
	    }
	}

*Why*?  In part because @jonpryor has wanted to do this for literal
years at this point, but also in part because of #9636
and related efforts to use Native AOT, which involves avoiding /
bypassing `DllImportAttribute` invocations (for now, everything
touched by Native AOT becomes a single `.so` binary, which we don't
know the name of).  Avoiding P/Invoke means *embracing* and extending
existing Java.Interop constructs (e.g. de04316).

In addition to altering the base types of `Java.Lang.Object` and
`Java.Lang.Throwable`:

  * Remove `handle` and related fields from `Java.Lang.Object` and
    `Java.Lang.Throwable`.

  * Update `PreserveLists/Mono.Android.xml` et al. so that the
    removed fields are not preserved.

  * Rename `JNIenvInit.AndroidValueManager` to
    `JNIEnvInit.ValueManager`, and change its type to
    `JniRuntime.JniValueManager`.  This is to help "force" usage of
    `JnIRuntime.JniValueManager` in more places, as we can't
    currently use `AndroidValueManager` in Native AOT (P/Invokes!).

  * Cleanup: Remove `JNIEnv.Exit()` and related code.  These were
    used by the Android Designer, which is no longer supported.

  * Update (`internal`) interface `IJavaObjectEx` to remove
    constructs present on `IJavaPeerable.`

  * Update the `Java.Lang.Object(IntPtr, JniHandleOwnership)` and
    `Java.Lang.Throwable(IntPtr, JniHandleOwnership)` constructors to
    follow dotnet/java-interop convention, and patch over the
    differences that exist between the two paradigms.

  * Update `ExceptionTest.CompareStackTraces()` to use
    `System.Diagnostics.StackTrace(ex, fNeedFileInfo:true)`
    so that when the `debug.mono.debug` system property is set, the
    `ExceptionTest.InnerExceptionIsSet()` unit test doesn't fail.
    Also, increase assertion message utility.

  * Update `AndroidObjectReferenceManager` so that
    dotnet/java-interop -initiated JNI object reference log messages
    are appropriately captured.

  * Update `JNIEnv.IsGCUserPeer()` to also consider types which
    implement `net.dot.jni.GCUserPeerable` to be "GC User Peers".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants