[PassKit] Add support Xcode15 Beta 7 #19139

tj-devel709 · 2023-10-09T13:47:06Z

The bindings for PassKit Xcode15 Beta 7.

tj-devel709 · 2023-10-09T13:48:52Z

src/passkit.cs

+		[Export ("delegate", ArgumentSemantic.Assign)]
+		IPKPayLaterViewDelegate Delegate { get; set; }


I got a lot of 'extra-null' xtro errors when using the WeakDelegate and casting with different combinations of nullallowed. I saw an example similar to this - is this alright?

This is the typical pattern I think:

[Export ("delegate", ArgumentSemantic.Assign)] NSObject WeakDelegate { get; set; } [Wrap ("WeakDelegate")] IPKPayLaterViewDelegate Delegate { get; set; }

that doesn't work?

@rolfbjarne These are the results I received last week :/

[Wrap ("WeakDelegate")] IPKPayLaterViewDelegate Delegate { get; set; } [Export ("delegate", ArgumentSemantic.Assign)] NSObject WeakDelegate { get; set;

Gives me:
Build error: build/dotnet/ios/generated-sources/PassKit/PKPayLaterView.g.cs(215,20): error CS8601: Possible null reference assignment.

[NullAllowed, Wrap ("WeakDelegate")] IPKPayLaterViewDelegate Delegate { get; set; } [NullAllowed, Export ("delegate", ArgumentSemantic.Assign)] NSObject WeakDelegate { get; set; }

Gives me:
!extra-null-allowed! 'System.Void PassKit.PKPayLaterView::set_WeakDelegate(Foundation.NSObject)' has a extraneous [NullAllowed] on parameter #0

[Wrap ("WeakDelegate")] IPKPayLaterViewDelegate Delegate { get; set; } [NullAllowed, Export ("delegate", ArgumentSemantic.Assign)] NSObject WeakDelegate { get; set; }

Gives me:
!extra-null-allowed! 'System.Void PassKit.PKPayLaterView::set_WeakDelegate(Foundation.NSObject)' has a extraneous [NullAllowed] on parameter #0

[NullAllowed, Wrap ("WeakDelegate")] IPKPayLaterViewDelegate Delegate { get; set; } [Export ("delegate", ArgumentSemantic.Assign)] NSObject WeakDelegate { get; set; }

Gives me:
Build error: build/dotnet/ios/generated-sources/PassKit/PKPayLaterView.g.cs(215,20): error CS8601: Possible null reference assignment.

If helpful, I gisted the generated file in question with the current changes and the proposed change that gave it the nullable issue: https://gist.github.com/tj-devel709/25ab695c8b58e15ec1200c45f2a6322f

Alex suggested to use the Nullable on both and add the xtro failure to the .ignore file

github-actions · 2023-10-09T13:51:46Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

src/passkit.cs

tj-devel709 · 2023-10-09T14:07:31Z

src/passkit.cs

+
+		[iOS (17, 0), Mac (14, 0), Watch (10, 0), NoTV, MacCatalyst (17, 0)]
+		[Field ("PKPaymentNetworkTmoney")]
+		NSString Tmoney { get; }


Fun fact, 'TMoney' was my fake rapper name in Jr. Highschool

github-actions · 2023-10-09T21:35:54Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

github-actions · 2023-10-12T15:39:22Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

mandel-macaque

Blocking until we have some clear opinion about my comments.

mandel-macaque · 2023-10-19T16:24:05Z

src/passkit.cs

+#if XAMCORE_5_0
+		[Export ("currencyCode")]
+#else
 		[NullAllowed, Export ("currencyCode")]
+#endif
 		string CurrencyCode { get; set; }

+#if XAMCORE_5_0
+		[Export ("summaryItems", ArgumentSemantic.Copy)]
+#else
 		[NullAllowed, Export ("summaryItems", ArgumentSemantic.Copy)]
+#endif


I have mixed feelings about this two #if.. in one hand I do understand them, we moved from 'string?' to 'string'. Peres is not an API breaking change, and code will compile unless the developer has the nullability warnings sets as errors.

So the removal of the ? will move use from a possible runtime error (we passed null) to a compiler warning possible error. I prefer users to get a warning than a runtime error in their applications when they are deployed to iOS 15 and later, specially when we do not have a clear date for XAMCORE_5_0.

@dalexsoto @rolfbjarne thoughts?

The other side is that if the API is actually nullable (for instance by accident on Apple's side), removing the nullability on our side will now result in a C# exception (ArgumentNullException) when the code previously worked.

CurrencyCode might have not been an accident based on the documentation from the service https://developer.apple.com/documentation/passkit/apple_pay/payment_token_format_reference?language=objc

currencyCode | string | ISO 4217 numeric currency code, as a string to preserve leading zeros -- | -- | --

And it uses https://www.iso.org/iso-4217-currency-codes.html which AFAIK does not have a way to represent null.

Similar to the summary items https://developer.apple.com/documentation/passkit/pkdisbursementrequest/3112810-summaryitems?changes=l_4&language=objc

This array must contain one [PKPaymentSummaryItem](https://developer.apple.com/documentation/passkit/pkpaymentsummaryitem?changes=l_4&language=objc), which contains the amount the individual receives.

If we allow null values, the requests to the payment service won't be valid at all.

Either way works for me :)

My take is that we should try to expose the truth of the headers as much as we can, in this case:

-@property (nonatomic, copy, nullable) NSArray<PKPaymentSummaryItem *> *summaryItems API_AVAILABLE(ios(12.2)) API_UNAVAILABLE(tvos, watchos, macos); +@property (nonatomic, copy) NSArray<PKPaymentSummaryItem *> *summaryItems;

Apple in theory has audited this header for nullability (see NS_ASSUME_NONNULL_BEGIN at the beginning of the file) so I hope they know what they are doing with their API.

Also from the docs standpoint it changed:

-// The summary items are optional. The final amount summary item will be prepopulated by the amount specified above. +// Array of PKPaymentSummaryItem objects which should be presented to the user. +// The last item should be the total you wish to disburse, and should not be pending.

So from the wording I think the change is intentional, it kind of matches what it is being done with the API.

Sounds good! I went ahead and removed those Xamcore conditionals! @rolfbjarne @mandel-macaque

vs-mobiletools-engineering-service2 · 2023-10-20T16:21:13Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 984f7964ec99fb90e866af6a0a44b9dd8bb85c28 [PR build]

vs-mobiletools-engineering-service2 · 2023-10-20T16:28:33Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-10-20T16:29:30Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-10-20T16:31:49Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2023-10-20T18:30:46Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
~~tvOS~~ (no change detected)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
~~tvOS~~: (empty diff detected)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 984f7964ec99fb90e866af6a0a44b9dd8bb85c28 [PR build]

vs-mobiletools-engineering-service2 · 2023-10-20T19:09:11Z

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: simulator tests

0 tests crashed, 1 tests failed, 234 tests passed.

Failures

❌ dotnettests tests

1 tests failed, 0 tests passed.

DotNet tests: Failed (Execution failed with exit code 1)

Html Report (VSDrops) Download

Successes

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 41 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 984f7964ec99fb90e866af6a0a44b9dd8bb85c28 [PR build]

tj-devel709 · 2023-10-23T16:13:18Z

Unrelated Test Failure:

DotNet tests: https://github.com/xamarin/maccore/issues/2688

mandel-macaque · 2023-10-23T16:54:39Z

/sudo backport release/8.0.1xx

vs-mobiletools-engineering-service2 · 2023-10-23T16:55:15Z

Backport Job to branch release/8.0.1xx Created! The magic is happening here

vs-mobiletools-engineering-service2 · 2023-10-23T16:56:58Z

Oh no! Backport failed! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8578782 for more details.

tj-devel709 · 2023-10-23T17:45:41Z

/sudo backport release/8.0.1xx

vs-mobiletools-engineering-service2 · 2023-10-23T17:46:15Z

Backport Job to branch release/8.0.1xx Created! The magic is happening here

vs-mobiletools-engineering-service2 · 2023-10-23T17:47:57Z

Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8579127 for more details.

The bindings for PassKit Xcode15 Beta 7. Backport of #19139 --------- Co-authored-by: tj_devel709 <antlambe@microsoft.com>

tj-devel709 added the note-highlight Worth calling out specifically in release notes label Oct 9, 2023

tj-devel709 added this to the xcode15 milestone Oct 9, 2023

tj-devel709 commented Oct 9, 2023

View reviewed changes

src/passkit.cs Show resolved Hide resolved

tj-devel709 commented Oct 9, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

rolfbjarne approved these changes Oct 19, 2023

View reviewed changes

haritha-mohan approved these changes Oct 19, 2023

View reviewed changes

mandel-macaque requested changes Oct 19, 2023

View reviewed changes

comment out PKPayLaterValidateAmount bits until Apple replies

049c2de

This comment has been minimized.

Sign in to view

remove the xamcore5 nullability removal

984f796

dalexsoto approved these changes Oct 20, 2023

View reviewed changes

mandel-macaque approved these changes Oct 23, 2023

View reviewed changes

vs-mobiletools-engineering-service2 mentioned this pull request Oct 23, 2023

[release/8.0.1xx] [PassKit] Add support Xcode15 Beta 7 #19306

Merged

tj-devel709 merged commit 6d30e58 into xamarin:main Oct 23, 2023
76 of 79 checks passed

mandel-macaque pushed a commit that referenced this pull request Oct 24, 2023

[release/8.0.1xx] [PassKit] Add support Xcode15 Beta 7 (#19306)

6667bb3

The bindings for PassKit Xcode15 Beta 7. Backport of #19139 --------- Co-authored-by: tj_devel709 <antlambe@microsoft.com>

		[Export ("delegate", ArgumentSemantic.Assign)]
		IPKPayLaterViewDelegate Delegate { get; set; }

[PassKit] Add support Xcode15 Beta 7 #19139

[PassKit] Add support Xcode15 Beta 7 #19139

Conversation

tj-devel709 commented Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tj-devel709 Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2023

⚠️ Your code has been reformatted. ⚠️

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Oct 9, 2023

⚠️ Your code has been reformatted. ⚠️

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Oct 12, 2023

⚠️ Your code has been reformatted. ⚠️

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mandel-macaque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Oct 20, 2023

💻 [CI Build] Windows Integration Tests passed 💻

vs-mobiletools-engineering-service2 commented Oct 20, 2023

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

vs-mobiletools-engineering-service2 commented Oct 20, 2023

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

vs-mobiletools-engineering-service2 commented Oct 20, 2023

📚 [PR Build] Artifacts 📚

Packages generated

vs-mobiletools-engineering-service2 commented Oct 20, 2023

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Oct 20, 2023

🔥 [CI Build] Test results 🔥

Test results

Failures

❌ dotnettests tests

Successes

tj-devel709 commented Oct 23, 2023

mandel-macaque commented Oct 23, 2023

vs-mobiletools-engineering-service2 commented Oct 23, 2023

vs-mobiletools-engineering-service2 commented Oct 23, 2023

tj-devel709 commented Oct 23, 2023

vs-mobiletools-engineering-service2 commented Oct 23, 2023

vs-mobiletools-engineering-service2 commented Oct 23, 2023

tj-devel709 commented Oct 9, 2023 •

edited

Loading

tj-devel709 Oct 18, 2023 •

edited

Loading