Skip to content

[NET Attribute Conversion] Rerun with fixes to catch old/unlisted availability - #14128

Closed
chamons wants to merge 9 commits into
dotnet:mainfrom
chamons:attribute_super_convert
Closed

[NET Attribute Conversion] Rerun with fixes to catch old/unlisted availability#14128
chamons wants to merge 9 commits into
dotnet:mainfrom
chamons:attribute_super_convert

Conversation

@chamons

@chamons chamons commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

This re-run of of mellite was powered by two changes:

After that, I could re-run it against all of xamarin-macios (repeating the annoying one time work), and then fixup the test to work.

@chamons chamons added not-notes-worthy Ignore for release notes run-dotnet-tests Run all the .NET tests labels Feb 11, 2022
@chamons chamons added the do-not-merge Do not merge this pull request label Feb 11, 2022
@chamons

chamons commented Feb 11, 2022

Copy link
Copy Markdown
Contributor Author

Further work on the generator suggests this PR may be incomplete (due to attributes not being seen).

Going to verify before closing or adding to this.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 146 tests passed.

Failed tests

  • introspection/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 37 Passed: 34 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed

Pipeline on Agent XAMBOT-1096.BigSur'
Merge d2ab11e into 39d52ba

Comment thread tests/cecil-tests/AttributeTest.cs Outdated
Comment thread tests/cecil-tests/AttributeTest.cs Outdated
Comment thread tests/cecil-tests/AttributeTest.cs Outdated
Comment thread tests/cecil-tests/AttributeTest.cs Outdated
Comment thread tests/cecil-tests/AttributeTest.cs Outdated
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

3 tests failed, 145 tests passed.

Failed tests

  • introspection/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 37 Passed: 34 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • link sdk/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
    No test log file was produced)

Pipeline on Agent XAMBOT-1097.BigSur'
Merge 0705887 into 764e797

@chamons chamons removed the do-not-merge Do not merge this pull request label Feb 16, 2022
@chamons

chamons commented Feb 16, 2022

Copy link
Copy Markdown
Contributor Author

Re-ran converter with new missing attribute logic to detect tv/catalyst attributes and use "what exists in the assembly" not "What namespace are you in".

It's even larger, but should only be supported/unsupported attributes.

I'm going to test this with the generator changes in flight and see if it checks out finally...

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

6 tests failed, 140 tests passed.

Failed tests

  • introspection/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 37 Passed: 34 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 44 Passed: 41 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • introspection/tvOS - simulator/Debug [dotnet]: Failed
  • monotouch-test/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 2708 Passed: 2513 Inconclusive: 11 Failed: 3 Ignored: 192)
  • link sdk/iOS Unified 64-bits - simulator/Debug: Failed

Pipeline on Agent XAMBOT-1100.BigSur'
Merge ddab47f into 404b6e9

Comment thread src/UIKit/UIAccessibility.cs Outdated
}

#if !NET
#if NET

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is re-introducing a member to .NET that we don't want (the #if !NET surrounded the entire property before the change, but after the change only the availability attributes).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

@mandel-macaque mandel-macaque left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The issue I see is as follows, we have obsoleted APIs in iOS exposed in MacCa1talyst that are missing the MacCatalyst Obsoleted OR they show Obsoleted with the iOS version (since there is no #if MACCATALYST we default to the iOS message AFAIK.

#if NET
[SupportedOSPlatform ("maccatalyst14.0")]
[SupportedOSPlatform ("ios")]
[SupportedOSPlatform ("macos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This has an obsolete on iOS, should it have one in maccatalyst?

#if NET
[SupportedOSPlatform ("maccatalyst14.0")]
[SupportedOSPlatform ("ios")]
[SupportedOSPlatform ("macos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

#if NET
[SupportedOSPlatform ("ios8.0")]
[SupportedOSPlatform ("maccatalyst")]
[SupportedOSPlatform ("tvos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same

[SupportedOSPlatform ("macos11.0")]
[SupportedOSPlatform ("ios")]
[SupportedOSPlatform ("maccatalyst")]
[SupportedOSPlatform ("tvos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same.

[SupportedOSPlatform ("tvos")]
[UnsupportedOSPlatform ("ios6.0")]
#if IOS
[Obsolete ("Starting with ios6.0 use 'AVAudioSession.Notification.ObserveAudioRouteChange' instead.", DiagnosticId = "BI1234", UrlFormat = "https://github.com/xamarin/xamarin-macios/wiki/Obsolete")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same.

[SupportedOSPlatform ("macos11.0")]
[SupportedOSPlatform ("ios")]
[SupportedOSPlatform ("maccatalyst")]
[SupportedOSPlatform ("tvos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same.

[SupportedOSPlatform ("macos11.0")]
[SupportedOSPlatform ("ios")]
[SupportedOSPlatform ("maccatalyst")]
[SupportedOSPlatform ("tvos")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same.

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
@chamons

chamons commented Feb 17, 2022

Copy link
Copy Markdown
Contributor Author

@mandel-macaque - I filed #14178 to look at those deprecated outside of this PR.

@chamons

chamons commented Feb 17, 2022

Copy link
Copy Markdown
Contributor Author

I was able to run Cecil.Tests.AttributeTest.ChildElementsListAvailabilityForAllPlatformsOnParent with this diff and a few test changes, which suggests the API changes are good.

@dalexsoto dalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could not find anything else, great stuff 👍

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff
  • ⚠️ Generator diff comments have not been provided.

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

9 tests failed, 137 tests passed.

Failed tests

  • introspection/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 37 Passed: 34 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 44 Passed: 41 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • introspection/tvOS - simulator/Debug [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Debug (static registrar) [dotnet]: Failed
  • monotouch-test/watchOS 32-bits - simulator/Debug: TimedOut
  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Crashed

Pipeline on Agent XAMBOT-1106.BigSur'
Merge 61a60e9 into 1273190

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API Current PR diff

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff
  • ⚠️ Generator diff comments have not been provided.

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

72 tests failed, 74 tests passed.

Failed tests

  • introspection/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 37 Passed: 34 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 44 Passed: 41 Inconclusive: 0 Failed: 1 Ignored: 2)
  • introspection/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • introspection/iOS Unified 64-bits - simulator/Debug: TimedOut
  • introspection/tvOS - simulator/Debug [dotnet]: Failed
  • introspection/tvOS - simulator/Debug: TimedOut
  • introspection/watchOS 32-bits - simulator/Debug: Crashed
  • monotouch-test/Mac Catalyst [dotnet]/Debug [dotnet]: Crashed (No test log file was produced)
  • monotouch-test/iOS Unified 64-bits - simulator/Debug [dotnet]: TimedOut
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar) [dotnet]: TimedOut
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk): TimedOut
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations): TimedOut
  • monotouch-test/tvOS - simulator/Debug [dotnet]: TimedOut
  • monotouch-test/tvOS - simulator/Debug (static registrar) [dotnet]: TimedOut
  • monotouch-test/tvOS - simulator/Debug (LinkSdk): TimedOut
  • monotouch-test/tvOS - simulator/Release (all optimizations): TimedOut
  • monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Crashed
  • dont link/Mac [dotnet]/Debug [dotnet]: TimedOut
  • dont link/Mac [dotnet]/Release [dotnet]: TimedOut
  • dont link/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut
  • dont link/Mac Catalyst [dotnet]/Release [dotnet]: TimedOut
  • dont link/iOS Unified 64-bits - simulator/Release [dotnet]: TimedOut
  • dont link/iOS Unified 64-bits - simulator/Release: TimedOut
  • dont link/tvOS - simulator/Release [dotnet]: TimedOut
  • dont link/tvOS - simulator/Release: TimedOut
  • dont link/watchOS 32-bits - simulator/Debug: Crashed
  • dont link/watchOS 32-bits - simulator/Release: Crashed
  • trimmode copy/Mac [dotnet]/Debug [dotnet]: TimedOut
  • trimmode copy/Mac [dotnet]/Release [dotnet]: TimedOut
  • trimmode copy/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut
  • trimmode copy/Mac Catalyst [dotnet]/Release [dotnet]: TimedOut
  • trimmode copy/iOS Unified 64-bits - simulator/Release [dotnet]: TimedOut
  • trimmode copy/tvOS - simulator/Release [dotnet]: TimedOut
  • fsharp/Mac [dotnet]/Debug [dotnet]: TimedOut
  • fsharp/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut
  • fsharp/iOS Unified 64-bits - simulator/Debug: TimedOut
  • fsharp/tvOS - simulator/Debug: TimedOut
  • fsharp/watchOS 32-bits - simulator/Debug: Crashed
  • framework-test/Mac [dotnet]/Debug [dotnet]: TimedOut
  • framework-test/Mac Catalyst [dotnet]/Debug [dotnet]: TimedOut
  • framework-test/iOS Unified 64-bits - simulator/Debug [dotnet]: TimedOut
  • framework-test/tvOS - simulator/Debug [dotnet]: TimedOut
  • framework-test/watchOS 32-bits - simulator/Debug: Crashed
  • link sdk/iOS Unified 64-bits - simulator/Release [dotnet]: TimedOut
  • link sdk/iOS Unified 64-bits - simulator/Release: TimedOut
  • link sdk/tvOS - simulator/Release [dotnet]: TimedOut
  • link sdk/tvOS - simulator/Release: TimedOut
  • link sdk/watchOS 32-bits - simulator/Debug: Crashed
  • link sdk/watchOS 32-bits - simulator/Release: Crashed
  • link all/iOS Unified 64-bits - simulator/Release [dotnet]: TimedOut
  • link all/iOS Unified 64-bits - simulator/Release: TimedOut
  • link all/tvOS - simulator/Release [dotnet]: TimedOut
  • link all/tvOS - simulator/Release: TimedOut
  • link all/watchOS 32-bits - simulator/Debug: Crashed
  • link all/watchOS 32-bits - simulator/Release: Crashed
  • trimmode link/iOS Unified 64-bits - simulator/Release [dotnet]: TimedOut
  • trimmode link/tvOS - simulator/Release [dotnet]: TimedOut
  • xcframework-test/iOS Unified 64-bits - simulator/Debug [dotnet]: TimedOut
  • xcframework-test/iOS Unified 64-bits - simulator/Debug: TimedOut
  • xcframework-test/tvOS - simulator/Debug [dotnet]: TimedOut
  • xcframework-test/tvOS - simulator/Debug: TimedOut
  • xcframework-test/watchOS 32-bits - simulator/Debug: Crashed
  • mono-native-compat/iOS Unified Today Extension 64-bits - simulator/Debug: TimedOut
  • mono-native-compat/watchOS 32-bits - simulator/Debug: Crashed
  • mono-native-unified/iOS Unified Today Extension 64-bits - simulator/Debug: TimedOut
  • mono-native-unified/tvOS - simulator/Debug: TimedOut
  • mono-native-unified/watchOS 32-bits - simulator/Debug: Crashed
  • interdependent-binding-projects/watchOS 32-bits - simulator/Debug: Crashed
  • DotNet tests: Failed (Execution failed with exit code 1)

Pipeline on Agent XAMBOT-1109.BigSur'
Merge 61a60e9 into 1273190

@chamons

chamons commented Feb 18, 2022

Copy link
Copy Markdown
Contributor Author

Looking into test failures now that it's passing and the one test last night.

@chamons chamons added the do-not-merge Do not merge this pull request label Feb 18, 2022
@chamons

chamons commented Feb 18, 2022

Copy link
Copy Markdown
Contributor Author

In addition to failure, do-not-merge since I figured out that I get to re-run the converter again due to:

namespace VideoToolbox {
    [iOS (8,0)]
    [TV (10,2)]
    public partial class VTCompressionSession {
    }
}

Not getting conversion on the type itself.

@chamons

chamons commented Mar 1, 2022

Copy link
Copy Markdown
Contributor Author

This required a lot of work to fix and improve on, so I'm closing and opening a new PR.

@chamons chamons closed this Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Do not merge this pull request not-notes-worthy Ignore for release notes run-dotnet-tests Run all the .NET tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants