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

Moving "Generic Math" in box and making it no longer experimental #65731

Merged
merged 3 commits into from
Mar 16, 2022

Conversation

tannergooding
Copy link
Member

@tannergooding tannergooding commented Feb 22, 2022

This is pending WPF uptaking a new C++/CLI toolset so they won't break when encountering types that have static abstract interfaces.

This is just moving the preview surface area to be non-preview, it is explicitly not the "final shape" for many of the interfaces and APIs.

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Feb 22, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: tannergooding
Assignees: tannergooding
Labels:

area-System.Numerics, new-api-needs-documentation

Milestone: -

@tannergooding tannergooding added the blocked Issue/PR is blocked on something - see comments label Feb 23, 2022
@tannergooding
Copy link
Member Author

Marking as blocked until we confirm this won't break WPF. CC. @jeffhandley

@tannergooding
Copy link
Member Author

Rebased to remove conflict

@steveisok
Copy link
Member

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tannergooding tannergooding marked this pull request as ready for review March 8, 2022 19:59
@jeffhandley jeffhandley added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Mar 9, 2022
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Mar 9, 2022
@ghost

This comment was marked as resolved.

@tannergooding
Copy link
Member Author

@akoeplinger could you please check/confirm if the mono failures here are related to Static Abstracts in Interfaces?

@jeffhandley jeffhandley removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Mar 9, 2022
@steveisok
Copy link
Member

@akoeplinger could you please check/confirm if the mono failures here are related to Static Abstracts in Interfaces?

The android failures look infra related and the wasm ones I believe are flaky. Not sure about the windows build failure:

LINK : fatal error LNK1181: cannot open input file 'ijwhost.lib'

@MaximLipnin
Copy link
Contributor

MaximLipnin commented Mar 9, 2022

Not sure about the windows build failure:

LINK : fatal error LNK1181: cannot open input file 'ijwhost.lib'

It was hit on the rolling build as well - #66380

@tannergooding
Copy link
Member Author

The android failures look infra related

@steveisok, any way we could confirm this? I'd prefer to not have to back this out after merging since its unblocking a lot of work related to generic math and it shipping in .NET 7.

Most of the failures look to be showing up as:

System.AggregateException : One or more errors occurred. (Failed to install mobile app.
Expected: True
Actual: False) (The following constructor parameters did not have matching fixture data: _Global globalVar)
---- Failed to install mobile app.
Expected: True
Actual: False
---- The following constructor parameters did not have matching fixture data: _Global globalVar

Also seeing a few failures like

[21:17:57] info: Attempting to install /datadisks/disk1/work/A1EA0909/w/B8F90A60/e/System.Linq.Tests.apk
[21:17:57] dbug: Executing command: '/datadisks/disk1/work/A1EA0909/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.22153.2/runtimes/any/native/adb/linux/adb -s emulator-5556 install /datadisks/disk1/work/A1EA0909/w/B8F90A60/e/System.Linq.Tests.apk'
[21:17:57] fail: Error:
Exit code: 1
Std out:
Performing Streamed Install

            Std err:
            adb: failed to install /datadisks/disk1/work/A1EA0909/w/B8F90A60/e/System.Linq.Tests.apk: cmd: Can't find service: package

And then for WASM:

DevOpsReportFailure: Failed to upload results: Error occurred in request., RetryError: HTTPSConnectionPool(host='dev.azure.com', port=443): Max retries exceeded with url: /dnceng/public/_apis/test/Runs/45564706/Results (Caused by ResponseError('too many 503 error responses',))

@steveisok
Copy link
Member

@tannergooding I'm aware of both problems and they are unrelated to your change.

@tannergooding
Copy link
Member Author

Thanks!

@agocke, @marek-safar, @jeffhandley are there any other tests or things that could block this from merging on your end?

Everything looks to be passing with the Mono failures being unrelated as indicated above. I don't see any of the tests disabled for NAOT/Mono under https://github.com/dotnet/runtime/blob/main/src/tests/issues.targets either.

@marek-safar
Copy link
Contributor

@tannergooding
Copy link
Member Author

check out https://github.com/dotnet/runtime/blob/main/src/libraries/tests.proj

Likewise don't see any disablement here. Notably the tests also moved from System.Runtime.Experimental to System.Runtime so I think existing disablement (if it had existed) would no longer be working.

@jeffhandley jeffhandley added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 9, 2022
@jeffhandley
Copy link
Member

We'll hold off on merging this until VS 17.2 Preview 2 is available. 🔜

@tannergooding tannergooding removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) blocked Issue/PR is blocked on something - see comments labels Mar 16, 2022
@tannergooding
Copy link
Member Author

Merging as per offline conversation with @jeffhandley. Now that VS 17.2 Preview 2 is publicly available and contains the C++/CLI fixes, external contributors will no longer be blocked.

dotnet/arcade#8633 tracks the helix support and should only impact WPF for our known dependents.

@tannergooding tannergooding merged commit e34e8dd into dotnet:main Mar 16, 2022
@jeffhandley
Copy link
Member

radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
…tnet#65731)

* Moving "Generic Math" in box and making it no longer experimental

* Removing an unused using from the generic math interfaces

* Updating the ordering of the checked keyword for the commented out checked operators
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants