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

Revert " Fix the calling convention for P/Invokes and delegates to hostpolicy" #23269

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Mar 15, 2019

Reverts #23249

Fixes: #23268

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 3.0 milestone Mar 15, 2019

@AaronRobinsonMSFT AaronRobinsonMSFT requested a review from sandreenko Mar 15, 2019

@BruceForstall
Copy link
Member

BruceForstall left a comment

LGTM if the builds pass

@sandreenko
Copy link
Member

sandreenko left a comment

LGTM, thank you.
I think passed "Ubuntu arm Cross Checked crossgen_comparison Build and Test" should be enough to test this revert.

@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test please
@dotnet-bot test Ubuntu arm Cross Checked no_tiered_compilation_innerloop Build and Test please

@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@dotnet-bot test this please

@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@dotnet-bot test this please

1 similar comment
@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@dotnet-bot test this please

@sandreenko

This comment has been minimized.

Copy link
Member

sandreenko commented Mar 15, 2019

@mmitche @MattGal could somebody please take a look at arm32 ubuntu machines? They all are offline. Do they need reboot?

@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@sandreenko I would prefer to checking #23270 which is a better fix for the problem. The entire Azure pipeline is green - including ARM - is that enough to verify the fix worked?

@AaronRobinsonMSFT

This comment has been minimized.

Copy link
Member Author

AaronRobinsonMSFT commented Mar 15, 2019

@sandreenko The OSX failure is for #23224, other than that both this and #23270 are green Azure. Is there any concern that the ARM32 legs mentioned have something that the Azure pipeline does not?

@sandreenko

This comment has been minimized.

Copy link
Member

sandreenko commented Mar 15, 2019

@sandreenko I would prefer to checking #23270 which is a better fix for the problem. The entire Azure pipeline is green - including ARM - is that enough to verify the fix worked?

Yes, please merge which one you think is better. I completely lost in all our runs and infrastructure failures that I can't really say about difference between ADO/Jenkins arm ubuntu builds. They should be the same, using the same docker container but obviously the second failed after the original PR, and the first worked fine. We will check why it happened tomorrow.

@AaronRobinsonMSFT AaronRobinsonMSFT deleted the revert-23249-fix_cc_for_assembly_resolver branch Mar 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.