Skip to content
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

[runtime] Remove ObjCRuntime.nfloat in favor of System.Runtime.InteropServices.NFloat. #14197

Merged
merged 12 commits into from
Feb 24, 2022

Conversation

rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented Feb 18, 2022

  • Remove ObjCRuntime.nfloat (in favor of System.Runtime.InteropServices.NFloat).
  • Automatically add a reference to the System.Runtime.InteropServices.Internal
    package, so that developers get the new NFloat API (with operators) we've
    added post .NET 6 (but don't do this for .NET 7).
  • Automatically add a global using alias for
    System.Runtime.InteropServices.NFloat -> nfloat. This is not behind the
    usual ImplicitUsings condition our other implicit usings are, because
    they're off by default for existing projects, and the main target for the
    global using alias for nfloat is upgraded projects.
  • Automatically generate a global using alias (like above) in the generator
    for all code the generator compiles.
  • Update xtro entries to reference System.Runtime.InteropServices.NFloat
    instead of ObjCRuntime.nfloat.
  • Add a workaround for a hopefully temporary issue with .NET/CoreCLR where the
    wrong runtime pack is selected otherwise (without the new NFloat API, so
    nothing works at runtime).

Ref: #13087

…arin#12922.

We can't execute mono's C# compiler when using .NET, so we need to tell bgen
where csc is in that case.

Fixes xamarin#12922.
…loat.Internal so that the new System.Runtime.InteropServices.NFloat API can be used.

This is only for .NET 6, once we switch to .NET 7 we don't need this anymore.
…ct it into compilations.

This can be turned off by passing /no-nfloat-using:true to bgen (which can be done
by setting NoNFloatUsing=true in the project file).
@rolfbjarne rolfbjarne added not-notes-worthy Ignore for release notes run-all-tests Run all our tests. skip-device-tests Skip device tests labels Feb 18, 2022
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

❤️

@@ -38,7 +38,7 @@ namespace Introspection {

public abstract class ApiSignatureTest : ApiBaseTest {
#if NET
const string NFloatTypeName = "ObjCRuntime.nfloat";
const string NFloatTypeName = "System.Runtime.InteropServices.nfloat";
Copy link
Contributor

Choose a reason for hiding this comment

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

NFloat ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been fixed now.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

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

Looking good!

@rolfbjarne rolfbjarne marked this pull request as ready for review February 23, 2022 09:09
@vs-mobiletools-engineering-service2
Copy link
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

1 tests failed, 234 tests passed.

Failed tests

  • [NUnit] Mono Mac OS X BCL tests group 2/Mac Full/Debug: Failed (Test run failed.
    Tests run: 11943 Passed: 10499 Inconclusive: 0 Failed: 3 Ignored: 354)

Pipeline on Agent XAMBOT-1094.BigSur'
Merge f034a16 into 4fd770c

@rolfbjarne
Copy link
Member Author

Test failure is unrelated (https://github.com/xamarin/maccore/issues/2542)

@rolfbjarne rolfbjarne merged commit bd97933 into xamarin:main Feb 24, 2022
@rolfbjarne rolfbjarne deleted the dotnet-nfloat-removal branch February 24, 2022 15:51
tj-devel709 pushed a commit to tj-devel709/xamarin-macios that referenced this pull request Mar 8, 2022
…pServices.NFloat. (xamarin#14197)

* Remove ObjCRuntime.nfloat (in favor of   System.Runtime.InteropServices.NFloat).
* Automatically add a reference to the System.Runtime.InteropServices.Internal
  package, so that developers get the new NFloat API (with operators) we've
  added post .NET 6 (but don't do this for .NET 7).
* Automatically add a global using alias for
  System.Runtime.InteropServices.NFloat -> nfloat. This is not behind the
  usual `ImplicitUsings` condition our other implicit usings are, because
  they're off by default for existing projects, and the main target for the
  global using alias for nfloat is upgraded projects.
* Automatically generate a global using alias (like above) in the generator
  for all code the generator compiles.
* Update xtro entries to reference System.Runtime.InteropServices.NFloat
  instead of ObjCRuntime.nfloat.
* Add a workaround for a hopefully temporary issue with .NET/CoreCLR where the
  wrong runtime pack is selected otherwise (without the new NFloat API, so
  nothing works at runtime).

Ref: xamarin#13087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-notes-worthy Ignore for release notes run-all-tests Run all our tests. skip-device-tests Skip device tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants