-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Move Msquic over to Source-generated delegate interop. #65204
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsI missed this when turning on DisableRuntimeMarshalling because we don't run any Quic tests in our regular CI that actually use MsQuic and we don't have the DisableRuntimeMarshalling analyzer yet.
|
why there is so much more code? shouldn't "generator" have the opposite effect? |
This code was generated from a generator that runs in a different mode and adapted to this use case. A source generator to capture this scenario is tracked in #63590. When that feature is implemented, we will move this code back to being source-generated. |
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.
Do we have a test to run or some other form of confirmation that the related issues in ASP.NET are fixed with this?
@BrennanConroy is there any validation we can do to make sure I caught all the cases that are causing problems? |
All we're doing in ASP.NET Core is calling Edit: No guarantee that the actual usage of MSQuic has issues after this change of course, but that'll be verified once we consume the fix. |
It looks like there might be some more non-blittable marshalling places in MsQuic. Marking this as no-merge for now. |
just a note that msquic now produce low-level generated binding for c#. There is desire to abandon ours and use theirs in 7.0 time frame. |
I've fixed the last few cases that were causing issues. Once this is green, it's ready to merge. |
if eventually it will be replaced, what value this temporary excessive glue would bring (other than burdening the git history)? |
The library is currently broken without this change, so it is necessary to enable ASP.NET to consume newer dotnet/runtime builds |
I missed this when turning on DisableRuntimeMarshalling because we don't run any Quic tests in our regular CI that actually use MsQuic and we don't have the DisableRuntimeMarshalling analyzer yet.
cc: @dotnet/interop-contrib