Skip to content

Commit

Permalink
API compatibility fixes after enumification.
Browse files Browse the repository at this point in the history
It does not mean everything in this change is about enumification - actually
only few changes are relevant.
It is wrong attitute because it mixes different fixes, but that's what I
was asked to do.

Here is the list of the reported breaking changes:

   Perceptible = <span class='removed removed-inline removed-breaking-inline'>130</span> <span class='added '>230</span>

Google changed the value!

    public RemoteInput.Builder SetAllowFreeFormInput (bool <span class='removed removed-inline removed-breaking-inline'>allowFreeFormInput</span> <span class='added '>allowFreeFormTextInput</span>)

The bare parameter name changes that we already resolved as "renaming to the new name is the way what we do". api-diff most likely needs fix to not report this kind of changes.

	<span class='removed removed-method breaking' data-is-breaking>public virtual void OnServiceAdded (ProfileState status, BluetoothGattService service);</span>

It is actually a bugfix: https://bugzilla.xamarin.com/show_bug.cgi?id=55473

	public <span class='removed removed-inline removed-breaking-inline'>AdvertiseTx</span> <span class='added '>int</span> TxPowerLevel { get; }

It is also a bugfix for https://bugzilla.xamarin.com/show_bug.cgi?id=51293

	public virtual Android.Database.ICursor Query (Android.Net.Uri <span class='removed removed-inline removed-breaking-inline'>url</span> <span class='added '>uri</span>, string[] projection, string selection, string[] selectionArgs, string sortOrder, Android.OS.CancellationSignal cancellationSignal)
	public virtual void OnError (string <span class='removed removed-inline removed-breaking-inline'>itemId</span> <span class='added '>mediaId</span>)
	public virtual bool EnableNetwork (int netId, bool <span class='removed removed-inline removed-breaking-inline'>disableOthers</span> <span class='added '>attemptConnect</span>)
	public virtual SubscriptionInfo GetActiveSubscriptionInfoForSimSlotIndex (int <span class='removed removed-inline removed-breaking-inline'>slotIdx</span> <span class='added '>slotIndex</span>)
	public virtual string GetDeviceId (int <span class='removed removed-inline removed-breaking-inline'>slotId</span> <span class='added '>slotIndex</span>)
	public virtual void SetMaxEms (int <span class='removed removed-inline removed-breaking-inline'>maxems</span> <span class='added '>maxEms</span>)
	public virtual void SetMaxHeight (int <span class='removed removed-inline removed-breaking-inline'>maxHeight</span> <span class='added '>maxPixels</span>)
	(... and all those relevant methods on Android.Widget.TextView)
	public bool IsAnnotationPresent (Class <span class='removed removed-inline removed-breaking-inline'>annotationType</span> <span class='added '>annotationClass</span>)
	public bool IsAssignableFrom (Class <span class='removed removed-inline removed-breaking-inline'>c</span> <span class='added '>cls</span>)
	public bool IsInstance (Object <span class='removed removed-inline removed-breaking-inline'>object</span> <span class='added '>obj</span>)
	public virtual bool IsAnnotationPresent (Class <span class='removed removed-inline removed-breaking-inline'>annotationType</span> <span class='added '>annotationClass</span>)
	public virtual void TraceInstructions (bool <span class='removed removed-inline removed-breaking-inline'>enable</span> <span class='added '>on</span>)
	public virtual void TraceMethodCalls (bool <span class='removed removed-inline removed-breaking-inline'>enable</span> <span class='added '>on</span>)
	public virtual bool IsAnnotationPresent (Java.Lang.Class <span class='removed removed-inline removed-breaking-inline'>annotationType</span> <span class='added '>annotationClass</span>)
	public Java.Lang.Object NewInstance (Java.Lang.Object[] <span class='removed removed-inline removed-breaking-inline'>args</span> <span class='added '>initargs</span>)
	public string GetDisplayScript (Locale <span class='removed removed-inline removed-breaking-inline'>locale</span> <span class='added '>inLocale</span>)
	public string GetDisplayVariant (Locale <span class='removed removed-inline removed-breaking-inline'>locale</span> <span class='added '>inLocale</span>)
	public virtual int NextInt (int <span class='removed removed-inline removed-breaking-inline'>n</span> <span class='added '>bound</span>)
	public CopyOnWriteArrayList (Java.Lang.Object[] <span class='removed removed-inline removed-breaking-inline'>array</span> <span class='added '>toCopyIn</span>)
	(... and all those lines in Java.Util.Concurrent.CopyOnWriteArrayList)

They are all parameter name fixes in Android API.

	MaskFlags = <span class='removed removed-inline removed-breaking-inline'>4080</span> <span class='added '>65520</span>

Google changed the constant value here too. Well, it should.

	<span class='added added-interface breaking' data-is-breaking>Java.Lang.IAutoCloseable</span>

This is a bogus report. It sure adds another interface but it does not bring in any breaking change because the derived interface XmlResourceParser also contains the only member in AutoCloseable i.e. close() !

api-check must not report this as a breaking change. This applies to all other interfaces that report the same error too.

	<span class='added added-method breaking' data-is-breaking>public virtual View KeyboardNavigationClusterSearch (View p0, FocusSearchDirection p1);</span>

Why is this happening? Because they are static methods that should not be part of the interface member.

Why does this happen? Because this fix is not merged! dotnet/java-interop#159

That fix really needs to be merged otherwise we keep generating these API incomatibility forever. The issue on java.time.chrono API mentioned there is actually due to totally different issue from the static <-> instance changes, so there is no excuse to not fix the static issue.

	<span class='removed removed-method breaking' data-is-breaking>public IAppendable Append (string s, int start, int end);</span>

Fixed: see src/Mono.Android/Java.Lang/StringBuffer.cs and src/Mono.Android/Java.Lang/StringBuilder.cs for details.

	<h3>Removed Type <span class='breaking' data-is-breaking>Java.Lang.Reflect.Constructor.InterfaceConsts</span></h3>
	<h3>Removed Type <span class='breaking' data-is-breaking>Java.Lang.Reflect.Method.InterfaceConsts</span></h3>

They must be brought be intrusive Google API changes due to the step-by-step migration from Harmony to OpenJDK. There are no members that used those consts and should be simply killed even without changes. We can take bugs IF ANYONE EVER used it. I don't waste my time on "fixing" this.

	<span class='removed removed-method breaking' data-is-breaking>public virtual bool IsAnnotationPresent (Java.Lang.Class annotationClass);</span>
	<span class='removed removed-method breaking' data-is-breaking>public virtual void AddPropertyChangeListener (Java.Beans.IPropertyChangeListener listener);</span>
	<span class='removed removed-method breaking' data-is-breaking>public virtual void RemovePropertyChangeListener (Java.Beans.IPropertyChangeListener listener);</span>
	(in both Pack200.IPacker and Pack200.IUnpacker)
	<span class='removed removed-property breaking' data-is-breaking>public virtual bool IsDestroyed { get; }</span>
	<span class='removed removed-method breaking' data-is-breaking>public virtual void Destroy ();</span>

They are all api-merge bug: why does api-merge preserve this method as a default interface method?? It had been non-default in earlier API Levels, therefore api-merge must generate this method as non-default.

	<span class='added added-interface breaking' data-is-breaking>IAnnotatedElement</span>

This is new in java.lang.reflect.GenericDeclaration interface and should also be part of huge Harmony - OpenJDK migration. And unlike other new additional interfaces, those methods in AnnotatedElement are not default, so they are indeed breaking changes.

At this state, I don't think there is anything we can help the situation. We cannot remove this base interface type because that will generate JCW  that doesn't compile.

The only idea I can think of is to completely kill this type. It's just like we give up EntityIterator.

	<span class='removed removed-method breaking' data-is-breaking>public virtual FileChannel Position (long newPosition);</span>
	<span class='removed removed-method breaking' data-is-breaking>public virtual FileChannel Truncate (long size);</span>
	<span class='removed removed-method breaking' data-is-breaking>public System.Threading.Tasks.Task&lt;FileChannel&gt; TruncateAsync (long size);</span>

This was actually newly introduced breakage for OpenJDK migration too. FileChannel now implements SeekableByteChannel, which never existed, and they require those methods to return ISeekableByteChannel in C#, not FileChannel whereas FileChannel **is** ISeekableByteChannel.

So they are changed in the metadata fixup, but then this breakage. Now Position() and Truncate() are added as explicit interface implementation.

However, what can we do for TruncateAsync() ? NOTHING! It is (semi-)automatically generated by `generateAsyncWrapper='true'` which is NOT defined in the interface and therefore it cannot be explicitly implemented. And it's all about return types.

	<span class='added added-interface breaking' data-is-breaking>Javax.Security.Auth.IDestroyable</span>

This should not be reported because all those interface methods are default!

	<h3>Removed Type <span class='breaking' data-is-breaking>Android.Service.Quicksettings.TileState</span></h3></div> <!-- end namespace Android.Service.Quicksettings -->

I explicitly removed this. It had been marked as [Obsolete].
  • Loading branch information
atsushieno committed Jul 13, 2017
1 parent 0a0aa95 commit bb12445
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 12 deletions.
4 changes: 2 additions & 2 deletions build-tools/enumification-helpers/methodmap.ext.csv
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,8 @@
26, android.telecom, Call.Callback, onRttInitiationFailure, reason, Android.Telecom.RttSessionModifyResult
26, android.telephony, TelephonyManager.UssdResponseCallback, onReceiveUssdResponseFailed, failureCode, Android.Telephony.UssdResultCode

26, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType
// cannot change this at this state.
// 24, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType
26, android.icu.util, UniversalTimeScale, bigDecimalFrom, timeScale, Android.Icu.Util.UniversalTimeScaleType
26, android.icu.util, UniversalTimeScale, from, timeScale, Android.Icu.Util.UniversalTimeScaleType
26, android.icu.util, UniversalTimeScale, getTimeScaleValue, scale, Android.Icu.Util.UniversalTimeScaleType
Expand Down Expand Up @@ -1720,7 +1721,6 @@
26, android.content, Intent, removeFlags, flags, Android.Content.ActivityFlags
26, android.content.pm, ApplicationInfo, getCategoryTitle, category, Android.Content.PM.ApplicationCategories
26, android.content.pm, LauncherApps, getApplicationInfo, flags, Android.Content.PM.PackageInfoFlags
26, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType

26, android.media, AudioAttributes, getVolumeControlStream, return, Android.Media.Stream
26, android.media, AudioAttributes, getVolumeControlStream, return, Android.Media.Stream
Expand Down
2 changes: 1 addition & 1 deletion build-tools/scripts/BuildEverything.mk
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ALL_PLATFORM_IDS = 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
# supported api levels
ALL_FRAMEWORKS = _ _ _ _ _ _ _ _ _ v2.3 _ _ _ _ v4.0.3 v4.1 v4.2 v4.3 v4.4 v4.4.87 v5.0 v5.1 v6.0 v7.0 v7.1 v8.0
API_LEVELS = 10 15 16 17 18 19 20 21 22 23 24 25 26
STABLE_API_LEVELS = 10 15 16 17 18 19 20 21 22 23 24 25
STABLE_API_LEVELS = 10 15 16 17 18 19 20 21 22 23 24 25 26

## The preceding values *must* use SPACE, **not** TAB, to separate values.

Expand Down
19 changes: 19 additions & 0 deletions src/Mono.Android/Java.Lang/StringBuffer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#if ANDROID_26

// It is introduced since API 26 not because of new member in StringBuffer
// but in AbstractStringBuilder which is nonpublic and affects the generated API.

using System;

namespace Java.Lang
{
public partial class StringBuffer
{
public IAppendable Append (string s, int start, int end)
{
return Append (new Java.Lang.String (s), start, end);
}
}
}

#endif
19 changes: 19 additions & 0 deletions src/Mono.Android/Java.Lang/StringBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#if ANDROID_26

// It is introduced since API 26 not because of new member in StringBuffer
// but in AbstractStringBuilder which is nonpublic and affects the generated API.

using System;

namespace Java.Lang
{
public partial class StringBuilder
{
public IAppendable Append (string s, int start, int end)
{
return Append (new Java.Lang.String (s), start, end);
}
}
}

#endif
34 changes: 34 additions & 0 deletions src/Mono.Android/Java.Nio/FileChannel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#if ANDROID_25

using System;

namespace Java.Nio.Channels
{
public partial class FileChannel
{
/*
This had to be added for API compatibility with earlier API Levels.
It is a newly introduced breakage for OpenJDK migration.
FileChannel now implements SeekableByteChannel, which never existed, and
they require those methods to return ISeekableByteChannel in C#, not
FileChannel whereas FileChannel is ISeekableByteChannel.
So they were first changed in the metadata fixup first, but then it resulted
in the API breakage. Therefore, I'm reverting the changes in metadata
and adding explicit interface methods here instead.
*/
ISeekableByteChannel ISeekableByteChannel.Position (long newPosition)
{
return Position (newPosition);
}

ISeekableByteChannel ISeekableByteChannel.Truncate (long size)
{
return Truncate (size);
}
}
}

#endif

3 changes: 3 additions & 0 deletions src/Mono.Android/Mono.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,14 @@
<Compile Include="Java.Lang\RuntimeException.cs" />
<Compile Include="Java.Lang\Short.cs" />
<Compile Include="Java.Lang\String.cs" />
<Compile Include="Java.Lang\StringBuffer.cs" />
<Compile Include="Java.Lang\StringBuilder.cs" />
<Compile Include="Java.Lang\Thread.cs" />
<Compile Include="Java.Lang\Throwable.cs" />
<Compile Include="Java.Lang.Reflect\InvocationTargetException.cs" />
<Compile Include="Java.Nio\Buffer.cs" />
<Compile Include="Java.Nio\CharBuffer.cs" />
<Compile Include="Java.Nio\FileChannel.cs" />
<Compile Include="Java.Security\IdentityScope.cs" />
<Compile Include="Java.Util\Spliterators.cs" />
<Compile Include="Java.Util.Concurrent.Atomic\AtomicInteger.cs" />
Expand Down
11 changes: 4 additions & 7 deletions src/Mono.Android/metadata
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@
Please use SetSystemScope() instead. This setter is not really public in Android API and will vanish in the future versions.
</attr>

<!-- return type in a method in a class cannot be more specific in C# -->
<attr path="/api/package[@name='java.nio.channels']/class[@name='FileChannel']/method[@name='position' and parameter[1]/@type='long']" name="managedReturn" api-since="24">Java.Nio.Channels.ISeekableByteChannel</attr>
<attr path="/api/package[@name='java.nio.channels']/class[@name='FileChannel']/method[@name='truncate']" name="managedReturn" api-since="24">Java.Nio.Channels.ISeekableByteChannel</attr>

<!-- Also, it seems to lack some non-public types even in android.jar but the derived types still reference...!
- java.lang.ClassNotFoundException is marked as derived from java.lang.ReflectiveOperationException which does not exist.
Also, this class has disappeared at some stage. -->
Expand Down Expand Up @@ -1362,8 +1358,9 @@
former non-generic version. So they should be regarded as identical.
Also, for this method we have *manually* bound generic method.

We could make fixes in api-merge, but manual fix is 100x simpler. -->
<remove-node path="/api/package[@name='android.view']/class/method[@name='findViewById']/typeParameters" api-since="26" />
<attr path="/api/package[@name='android.view']/class/method[@name='findViewById']" name="return" api-since="26">android.view.View</attr>
We could make fixes in api-merge, but manual fix is 100x simpler.
Sadly api-merge is processed before metadata fixup, so it cannot be simple overload removal. -->
<remove-node path="/api/package/class/method[@name='findViewById' or @name='onFindViewById' or @name='findViewWithTag']/typeParameters" api-since="26" />
<attr path="/api/package/class/method[@name='findViewById' or @name='onFindViewById' or @name='findViewWithTag']" name="return" api-since="26">android.view.View</attr>

</metadata>
4 changes: 2 additions & 2 deletions src/Mono.Android/methodmap.csv
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,8 @@
26, android.telecom, Call.Callback, onRttInitiationFailure, reason, Android.Telecom.RttSessionModifyResult
26, android.telephony, TelephonyManager.UssdResponseCallback, onReceiveUssdResponseFailed, failureCode, Android.Telephony.UssdResultCode

26, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType
// cannot change this at this state.
// 24, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType
26, android.icu.util, UniversalTimeScale, bigDecimalFrom, timeScale, Android.Icu.Util.UniversalTimeScaleType
26, android.icu.util, UniversalTimeScale, from, timeScale, Android.Icu.Util.UniversalTimeScaleType
26, android.icu.util, UniversalTimeScale, getTimeScaleValue, scale, Android.Icu.Util.UniversalTimeScaleType
Expand Down Expand Up @@ -2316,7 +2317,6 @@
26, android.content, Intent, removeFlags, flags, Android.Content.ActivityFlags
26, android.content.pm, ApplicationInfo, getCategoryTitle, category, Android.Content.PM.ApplicationCategories
26, android.content.pm, LauncherApps, getApplicationInfo, flags, Android.Content.PM.PackageInfoFlags
26, android.icu.util, TimeZone, getTimeZone, type, Android.Icu.Util.TimeZoneType

26, android.media, AudioAttributes, getVolumeControlStream, return, Android.Media.Stream
26, android.media, AudioAttributes, getVolumeControlStream, return, Android.Media.Stream
Expand Down

0 comments on commit bb12445

Please sign in to comment.