Skip to content

Conversation

@moljac
Copy link
Contributor

@moljac moljac commented Dec 16, 2021

Does this change any of the generated binding API's?

no API changes for androidx.window:*-1.0.0

new version of kotlin-stdlib

Describe your contribution

  • update of androidx.window:*-1.0.0
  • necessary dependencies updates and metadata fixes

related:

#446

Updated

  • androidx.window:window - 1.0.0-beta04 -> 1.0.0
  • androidx.window:window-java - 1.0.0-beta04 -> 1.0.0
  • org.jetbrains.kotlin:kotlin-stdlib - 1.5.31 -> 1.6.0
  • org.jetbrains.kotlin:kotlin-stdlib-common - 1.5.31 -> 1.6.0
  • org.jetbrains.kotlin:kotlin-stdlib-jdk7 - 1.5.31 -> 1.6.0
  • org.jetbrains.kotlin:kotlin-stdlib-jdk8 - 1.5.31 -> 1.6.0

@moljac moljac linked an issue Dec 16, 2021 that may be closed by this pull request
@moljac moljac marked this pull request as draft December 16, 2021 14:01
conceptdev added a commit to microsoft/surface-duo-sdk-xamarin-samples that referenced this pull request Dec 17, 2021
@conceptdev
Copy link
Contributor

conceptdev commented Dec 17, 2021

I pulled the NuGet packages from the CI and added them to a Xamarin.Android test project - seems to work 😊

microsoft/surface-duo-sdk-xamarin-samples#33

Will these NuGets get published? We had a PR for 1.0.0.5-beta04 but I don't see it on https://www.nuget.org/packages/Xamarin.AndroidX.Window.WindowJava/

@conceptdev
Copy link
Contributor

@moljac what's the status of this PR? did the stdlib update cause issues?

@moljac moljac marked this pull request as ready for review January 26, 2022 14:44
@moljac moljac requested a review from jpobst January 27, 2022 12:22
@jpobst
Copy link
Contributor

jpobst commented Jan 27, 2022

I think we're going to have to hold off on this until we investigate the breaking changes to Kotlin 1.6 and determine if they're on our side or Kotlin's side:
#436 (comment)

@conceptdev
Copy link
Contributor

Thanks @jpobst - let me know how it goes?

I've created another branch and PR #470 so I can (try to) test the latest, stable, release of the package. Obviously, we can't move forward until the Kotlin 1.6 issue is resolved though.

@moljac
Copy link
Contributor Author

moljac commented Jan 31, 2022

Thanks @jpobst - let me know how it goes?

I've created another branch and PR #470 so I can (try to) test the latest, stable, release of the package. Obviously, we can't move forward until the Kotlin 1.6 issue is resolved though.

I downgraded Kotlin StdLib, skipped rc01, so stable is here

Building, cleaning up and preparing for release. Other stable bindings are in:

#468

and that one will be nasty.

@moljac moljac changed the title manual update mu 20211216 androidx.window:*-1.0.0-rc1 manual update mu 20211216 androidx.window:*-1.0.0(stable) Feb 1, 2022
@moljac
Copy link
Contributor Author

moljac commented Feb 2, 2022

Thanks @jpobst - let me know how it goes?

I've created another branch and PR #470 so I can (try to) test the latest, stable, release of the package. Obviously, we can't move forward until the Kotlin 1.6 issue is resolved though.

I think I got it with:

  <attr path="/api/package/class/method/parameter[@name='arg0']" name="name">$this</attr>

There are still removed APIs (some obsoleted), but api-diff looks better

@moljac
Copy link
Contributor Author

moljac commented Feb 2, 2022

The Kotlin 1.6 issue is being tracked here: dotnet/java-interop#945.

@jpobst
Copy link
Contributor

jpobst commented Feb 3, 2022

I don't think changing the parameter name is going to fix it:

Type Changed: Kotlin.UIntArray

Removed methods:

public static uint Get (int[] _this, int index);
public static void Set (int[] _this, int index, uint value);

Added methods:

public static int Get (int[] _this, int index);
public static void Set (int[] _this, int index, int value);

I think the only way to fix this without binding tooling changes would be to change the type of every changed uint/ushort/ulong/ubyte back to the unsigned versions.

@moljac
Copy link
Contributor Author

moljac commented Feb 7, 2022

I think the only way to fix this without binding tooling changes would be to change the type of every changed uint/ushort/ulong/ubyte back to the unsigned versions.

I changed types back to unsigned versions.

Let us see if that helps, so this is unblocked.

Copy link
Contributor

@jpobst jpobst left a comment

Choose a reason for hiding this comment

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

Nice! I was afraid fixing this was going to require a lot more metadata than this.

I think we need to switch to type and then we're good to go.

</attr>
<attr
path="/api/package[@name='kotlin']/class[@name='UIntArray']/method[@name='set-VXSXFK8' and count(parameter)=3 and parameter[1][@type='int[]'] and parameter[2][@type='int'] and parameter[3][@type='int']]/parameter[3]"
name="managedType"
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize you should always use managedType and are never supposed to use type, but I think in these cases we actually do want type. There is some custom marshalling code that gets generated for Kotlin unsigned types, and I suspect it won't get triggered with managedType. Thus the code may not work correctly at runtime.

Copy link
Contributor Author

@moljac moljac Feb 8, 2022

Choose a reason for hiding this comment

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

I realize you should always use managedType and are never supposed to use type, but I think in these cases we actually do want type. There is some custom marshalling code that gets generated for Kotlin unsigned types, and I suspect it won't get triggered with managedType. Thus the code may not work correctly at runtime.

You know I try not to remove-node and to not change native type and return.

OK. Trying only type (only for parameter)!!

Copy link
Contributor

@jpobst jpobst left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@moljac moljac merged commit 92d48b9 into main Feb 8, 2022
@moljac moljac deleted the mu-20211216-androidx-window-1.0.0-rc1 branch February 8, 2022 21:42
conceptdev added a commit to microsoft/surface-duo-sdk-xamarin-samples that referenced this pull request Feb 23, 2022
* [WindowManager] update to rc01

test NuGets from CI dotnet/android-libraries#449

* [all] update Jetpack Window Manager (rc01)

* [all] update Jetpack Window Manager (1.0.0 stable)

* tweaks

* add comments re 1.0.0.7 stable version update

* [windowmanager] config tweaks

* update README for Jetpack Window Manager support
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.

Update "AndroidX.Window" to "1.0.0-rc01"

4 participants