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.

Sign up for GitHub

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

Jump to bottom

Convert all possible DllImports to LibraryImport for .NET 7+ #41573

Merged
merged 10 commits into from
May 31, 2022
Merged

Convert all possible DllImports to LibraryImport for .NET 7+ #41573

merged 10 commits into from
May 31, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented May 8, 2022 •
edited
Loading

This work converts all possible usages of DllImport to LibraryImport in the code base. Instances of DllImport may still exist because the project targets pre-NET7 TFMs or has unsupported arguments.

/cc @jkoritzinsky @elinor-fung @jkotas @davidfowl

@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as draft May 8, 2022 00:04
@AaronRobinsonMSFT
Copy link
Member Author

Sigh. Looks like we need some source generator feature work here.

@davidfowl
Copy link
Member

Sigh. Looks like we need some source generator feature work here.

That's a good thing! 😄

This was referenced May 8, 2022
Closed
Closed
@jkoritzinsky jkoritzinsky mentioned this pull request May 12, 2022
Closed
@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request May 15, 2022
Closed
1 task
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review May 26, 2022 03:09
@AaronRobinsonMSFT
Copy link
Member Author

/cc @adityamandaleeka

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-quarantined-pr

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-components-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dougbu
Copy link
Member

dougbu commented May 27, 2022

@AaronRobinsonMSFT please don't rerun until dotnet/arcade#9490 is resolved. We're down and retrying won't help, sorry.

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-quarantined-pr

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT
Copy link
Member Author

/azp run aspnetcore-components-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AaronRobinsonMSFT
Copy link
Member Author

All the errors are the same in this PR. I'm not sure they are related to any of my changes though. The general failure is the following. All are for some .css file that is missing. The log I am using is here.

2022-05-27T15:25:50.2352729Z  | Xunit.Sdk.TrueException: Expected /AspNet.V1voxwtx5efa.styles.css to have status 'OK' but it was 'NotFound'.
2022-05-27T15:25:50.2352763Z  | Expected: True
2022-05-27T15:25:50.2352797Z  | Actual:   False

@BrennanConroy
Copy link
Member

Same failures also happened on the latest build in main
https://dev.azure.com/dnceng/public/_build/results?buildId=1792855&view=results

@AaronRobinsonMSFT
Copy link
Member Author

@BrennanConroy Thank you sir! I was waiting on the DARC update PR to see if it had the same failures. Sounds like this PR is ready for a review.

adityamandaleeka
adityamandaleeka reviewed May 28, 2022
View reviewed changes
Copy link
Member

@adityamandaleeka adityamandaleeka left a comment

Choose a reason for hiding this comment

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

LGTM assuming it works 😄

@AaronRobinsonMSFT
Copy link
Member Author

LGTM assuming it works 😄

Aspersions are being cast!

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 332a6f9 into dotnet:main May 31, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the consume_libraryimport branch May 31, 2022 23:19
@ghost ghost added this to the 7.0-preview6 milestone May 31, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request Jun 1, 2022
Merged
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfoidl gfoidl gfoidl left review comments

@jkotas jkotas jkotas left review comments

@adityamandaleeka adityamandaleeka adityamandaleeka approved these changes

@Tratcher Tratcher Awaiting requested review from Tratcher

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

@JamesNK JamesNK Awaiting requested review from JamesNK JamesNK is a code owner

@dougbu dougbu Awaiting requested review from dougbu

@wtgodbe wtgodbe Awaiting requested review from wtgodbe wtgodbe is a code owner

@Pilchie Pilchie Awaiting requested review from Pilchie

Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
7.0-preview6
Development

Successfully merging this pull request may close these issues.
9 participants
@AaronRobinsonMSFT @davidfowl @dougbu @BrennanConroy @adityamandaleeka @gfoidl @jkotas @Pilchie @amcasey