-
Notifications
You must be signed in to change notification settings - Fork 514
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
[Submission] Fix all the selectors that apple warns about. #9268
[Submission] Fix all the selectors that apple warns about. #9268
Conversation
We have noticed the following message from Apple when performing submissions with Xamarin.iOS: > ITMS-90338: Non-public API usage - The app references non-public > selectors in WcBc.iOS: behaviorTypes, convolutionState, > discoverAllContactUserInfosWithCompletionHandler:, > discoverAllContactsCompletionBlock, > discoverUserInfoWithEmailAddress:completionHandler:, > discoverUserInfoWithUserRecordID:completionHandler:, > discoverUserInfosCompletionBlock, displayContact, drawableResizesAsynchronously, > encodeToCommandBuffer:sourceImage:convolutionState:, > encodeToCommandBuffer:sourceImage:destinationImage:state:, > getProperty:onChannel:responseHandler:, hasProperty:onChannel:responseHandler:, > initWithEmailAddresses:userRecordIDs:, initWithMIDIEntity:dataReadyHandler:, > initWithZoneID:options:, initWithZoneID:subscriptionID:options:, > isPublicDatabase, mouseUpAction, newDrawable, propertyChangedCallback, > removeAllAppearanceStreams, replaceTextStorage:, retrieveConnectedPeripherals, > retrievePeripherals:, setDiscoverAllContactsCompletionBlock:, > setDiscoverUserInfosCompletionBlock:, setDrawableResizesAsynchronously:, > setEditedMask:, setMouseUpAction:, setMovieControlMode:, > setProperty:onChannel:responseHandler:, setPropertyChangedCallback:, > setSocketFamily:, setTemporaryAttributes:forCharacterRange:, setUserRecordIDs:, > sourceOffset, subscriptionOptions, takeBackgroundColorFrom:, takePasswordFrom:, > temporalAntialiasingEnabled, userRecordIDs. If method names in your source code > match the private Apple APIs listed above, altering your method names will help > prevent this app from being flagged in future submissions. In addition, note > that one or more of the above APIs may be located in a static library that was > included with your app. If so, they must be removed. For further information, > visit the Technical Support Information at http://developer.apple.com/support/technical/ All of them have been removed but without a break in the API excep "initWithMIDIEntity:dataReadyHandler:" wich does look like an error on Apples side. Empty stubs are used as much as possible except on those cases in which a handler is called or an output variable should be modified (buffer, out param) to minimize the users surprise at runtime.
I'm getting the following xtro issues:
|
Looks that we did not have any issues with submissions: https://gist.github.com/mandel-macaque/1da51e922bd75e4340a0491a1ea3b3e0 |
src/XKit/XKitCompat.cs
Outdated
=> throw new NotSupportedException (); | ||
|
||
|
||
[Obsolete ("Always throws NotSupportedException (). (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's public for macOS, but nowhere else:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hasn't been fixed.
src/scenekit.cs
Outdated
[Abstract] // this protocol existed before iOS 9 (or OSX 10.11) and we cannot add abstract members to it (breaking changes) | ||
#endif | ||
[Watch (6,0), TV (13,0), Mac (10,15), iOS (13,0)] | ||
[Export ("temporalAntialiasingEnabled")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -216,17 +214,5 @@ interface MidiCISession | |||
|
|||
[NullAllowed, Export ("profileChangedCallback", ArgumentSemantic.Assign)] | |||
MidiCIProfileChangedHandler ProfileChangedCallback { get; set; } | |||
|
|||
[Export ("hasProperty:onChannel:responseHandler:")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These show up in the headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yet submissions complain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only ref I can find is when it was added. But nothing else.
[NoWatch] | ||
[Deprecated (PlatformName.iOS, 10,0, message: "Use 'CKQuerySubscriptionOptions'.")] | ||
[Deprecated (PlatformName.MacOSX, 10,12, message: "Use 'CKQuerySubscriptionOptions'.")] | ||
[Export ("subscriptionOptions", ArgumentSemantic.UnsafeUnretained)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still seems valid if you're targeting iOS 8 - 9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The submission was complaining about this. Was not deleted due to the deprecation warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
Build failure Test results18 tests failed, 70 tests passed.Failed tests
|
Hard to tell if everything is alright, push to origin I guess so it builds packages in case someone is on a hurry and maybe try again submission tests with this branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String content are minor.
However API Diff is reporting breaking changes. It should not for this type of change. Please review that.
src/CloudKit/CKCompat.cs
Outdated
public partial class CKContainer { | ||
|
||
[iOS (8, 0), Mac (10, 10)] | ||
[Obsolete ("Use 'DiscoverAllIdentities' instead. NotSupportedException'")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too many different patterns in the PR
Use "Always throw a 'NotSupportedException'. Use 'X' instead."
i.e. start with the fact it won't work, then suggest alternative.
Starting with a suggestion might be discarded before reaching the not-working part, e.g. horizontal scrolling build warnings inside an IDE.
src/CloudKit/CKCompat.cs
Outdated
} | ||
|
||
public partial class CKDiscoverUserInfosOperation { | ||
[Obsolete ("Always throws NotSupportedException (). (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is close to an existing string "Always throw a 'NotSupportedException' (not a public API)."
and this pattern is unlike others.
Having different string is bad since they can't be shared inside the assembly, leading to larger size apps. The linker can remove some but it's not always enabled (e.g. interpreter). I'll file an issue on this.
|
||
public partial class CAMetalLayer { | ||
|
||
[Obsolete ("Always throw a 'NotSupportedException' (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that one is better 😸
src/CoreBluetooth/CBCompat.cs
Outdated
@@ -33,6 +33,12 @@ public partial class CBMutableService { | |||
} | |||
} | |||
|
|||
public partial class CBCentralManager { | |||
|
|||
[Obsolete ("Empty stub. (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, close but not identical to "Empty stub (not a public API)."
(extra .
)
src/CoreBluetooth/GuidWrapper.cs
Outdated
@@ -30,28 +30,13 @@ public void ConnectPeripheral (CBPeripheral peripheral, PeripheralConnectionOpti | |||
} | |||
|
|||
#if !TVOS && !WATCH | |||
[Deprecated (PlatformName.iOS, 7, 0)] | |||
[Obsoleted (PlatformName.iOS, 9, 0, message : "Use 'RetrievePeripheralsWithIdentifiers' instead.")] | |||
[Obsolete ("Always throws NotSupportedException (). (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obsolete code should be under #if !XAMCORE_4_0
so we don't carry those over when a breaking change happens
src/CoreMidi/MidiCompat.cs
Outdated
[Obsolete ("Empty stub. (not a public API).")] | ||
public virtual MidiCIPropertyChangedHandler PropertyChangedCallback { get; set; } | ||
|
||
[Obsolete ("Always throws NotSupportedException (). (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments above, they don't match existing strings
|
||
public partial class MPSCnnConvolutionState { | ||
|
||
[Obsolete ("Empty stub. (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same (all above)
@spouliot @rolfbjarne @dalexsoto submission tests with 2e15f5f can be found here: https://gist.github.com/mandel-macaque/2a3058146522fd17e53f7a3680522b29 |
Build failure Test results17 tests failed, 71 tests passed.Failed tests
|
@mandel-macaque there are still breaking changes in API diff |
@spouliot ill take a second look, I should have ran them locally |
Build failure Test results1 tests failed, 87 tests passed.Failed tests
|
src/XKit/XKitCompat.cs
Outdated
=> throw new NotSupportedException (); | ||
|
||
|
||
[Obsolete ("Always throws NotSupportedException (). (not a public API).")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hasn't been fixed.
Build failure Test results2 tests failed, 86 tests passed.Failed tests
|
@mandel-macaque please double check those. There should not be any addition of obsolete members iOS
tvOS
watchOS
macOS
|
Build success |
@spouliot @rolfbjarne results for the submission tests for dd1ed58 are here. |
Build success |
@monojenkins backport d16-8 |
@monojenkins backport to xcode12 |
@spouliot backporting to xcode12 failed, the patch results in conflicts:
Please backport manually! |
) We have noticed the following message from Apple when performing submissions with Xamarin.iOS: > ITMS-90338: Non-public API usage - The app references non-public > selectors in WcBc.iOS: behaviorTypes, convolutionState, > discoverAllContactUserInfosWithCompletionHandler:, > discoverAllContactsCompletionBlock, > discoverUserInfoWithEmailAddress:completionHandler:, > discoverUserInfoWithUserRecordID:completionHandler:, > discoverUserInfosCompletionBlock, displayContact, drawableResizesAsynchronously, > encodeToCommandBuffer:sourceImage:convolutionState:, > encodeToCommandBuffer:sourceImage:destinationImage:state:, > getProperty:onChannel:responseHandler:, hasProperty:onChannel:responseHandler:, > initWithEmailAddresses:userRecordIDs:, initWithMIDIEntity:dataReadyHandler:, > initWithZoneID:options:, initWithZoneID:subscriptionID:options:, > isPublicDatabase, mouseUpAction, newDrawable, propertyChangedCallback, > removeAllAppearanceStreams, replaceTextStorage:, retrieveConnectedPeripherals, > retrievePeripherals:, setDiscoverAllContactsCompletionBlock:, > setDiscoverUserInfosCompletionBlock:, setDrawableResizesAsynchronously:, > setEditedMask:, setMouseUpAction:, setMovieControlMode:, > setProperty:onChannel:responseHandler:, setPropertyChangedCallback:, > setSocketFamily:, setTemporaryAttributes:forCharacterRange:, setUserRecordIDs:, > sourceOffset, subscriptionOptions, takeBackgroundColorFrom:, takePasswordFrom:, > temporalAntialiasingEnabled, userRecordIDs. If method names in your source code > match the private Apple APIs listed above, altering your method names will help > prevent this app from being flagged in future submissions. In addition, note > that one or more of the above APIs may be located in a static library that was > included with your app. If so, they must be removed. For further information, > visit the Technical Support Information at http://developer.apple.com/support/technical/ All of them have been removed but without a break in the API excep "initWithMIDIEntity:dataReadyHandler:" wich does look like an error on Apples side. Empty stubs are used as much as possible except on those cases in which a handler is called or an output variable should be modified (buffer, out param) to minimize the users surprise at runtime.
…9408) * [Submission] Fix all the selectors that apple warns about. (#9268) We have noticed the following message from Apple when performing submissions with Xamarin.iOS: > ITMS-90338: Non-public API usage - The app references non-public > selectors in WcBc.iOS: behaviorTypes, convolutionState, > discoverAllContactUserInfosWithCompletionHandler:, > discoverAllContactsCompletionBlock, > discoverUserInfoWithEmailAddress:completionHandler:, > discoverUserInfoWithUserRecordID:completionHandler:, > discoverUserInfosCompletionBlock, displayContact, drawableResizesAsynchronously, > encodeToCommandBuffer:sourceImage:convolutionState:, > encodeToCommandBuffer:sourceImage:destinationImage:state:, > getProperty:onChannel:responseHandler:, hasProperty:onChannel:responseHandler:, > initWithEmailAddresses:userRecordIDs:, initWithMIDIEntity:dataReadyHandler:, > initWithZoneID:options:, initWithZoneID:subscriptionID:options:, > isPublicDatabase, mouseUpAction, newDrawable, propertyChangedCallback, > removeAllAppearanceStreams, replaceTextStorage:, retrieveConnectedPeripherals, > retrievePeripherals:, setDiscoverAllContactsCompletionBlock:, > setDiscoverUserInfosCompletionBlock:, setDrawableResizesAsynchronously:, > setEditedMask:, setMouseUpAction:, setMovieControlMode:, > setProperty:onChannel:responseHandler:, setPropertyChangedCallback:, > setSocketFamily:, setTemporaryAttributes:forCharacterRange:, setUserRecordIDs:, > sourceOffset, subscriptionOptions, takeBackgroundColorFrom:, takePasswordFrom:, > temporalAntialiasingEnabled, userRecordIDs. If method names in your source code > match the private Apple APIs listed above, altering your method names will help > prevent this app from being flagged in future submissions. In addition, note > that one or more of the above APIs may be located in a static library that was > included with your app. If so, they must be removed. For further information, > visit the Technical Support Information at http://developer.apple.com/support/technical/ All of them have been removed but without a break in the API excep "initWithMIDIEntity:dataReadyHandler:" wich does look like an error on Apples side. Empty stubs are used as much as possible except on those cases in which a handler is called or an output variable should be modified (buffer, out param) to minimize the users surprise at runtime.
We have noticed the following message from Apple when performing
submissions with Xamarin.iOS:
All of them have been removed but without a break in the API excep
"initWithMIDIEntity:dataReadyHandler:" wich does look like an error on
Apples side.
Empty stubs are used as much as possible except on those cases in which
a handler is called or an output variable should be modified (buffer,
out param) to minimize the users surprise at runtime.