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

Fix clrjit exports on non-Windows platforms. #38254

Merged
merged 3 commits into from
Jun 23, 2020
Merged

Fix clrjit exports on non-Windows platforms. #38254

merged 3 commits into from
Jun 23, 2020

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Jun 23, 2020 •
edited
Loading

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 5.0.0 milestone Jun 23, 2020
@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Jun 23, 2020 •
edited
Loading

Thanks @jkotas for pressing me to root cause this. It was introduced in 43ae9af#diff-447ca31fa36a141e124dda8eab0a539c - #37753.

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Use pthreads directly on non-Windows platforms. Fix clrjit exports on non-Windows platforms. Jun 23, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-Interop-coreclr labels Jun 23, 2020
@AaronRobinsonMSFT
Copy link
Member Author

/cc @dotnet/jit-contrib

@jkotas
Copy link
Member

jkotas commented Jun 23, 2020

cc @janvorli

@jkotas
Copy link
Member

jkotas commented Jun 23, 2020

cc @am11

@am11
Copy link
Member

am11 commented Jun 23, 2020

It was introduced in

Thanks for the fix. Wonder why the CI didn't caught it there.. Second, I think it would be better to bring about consistency in names, to avoid such accidents. For cmake configuration, prefixing names with JIT_ is redundant when jit is already in file path.

@janvorli
Copy link
Member

@am11, I've actually wanted to ask you for one change that when I was reviewing your change - to add the name of the variable that we set the option as a parameter to the set_exports_linker_option instead of assuming it is EXPORTS_LINKER_OPTION. But I've somehow forgotten to push the comment button and then forgotten about it in the final review.
I think it would be good to do it that way - would you mind making such a change?

@AaronRobinsonMSFT
Copy link
Member Author

ARM64 Failure is known: #36681

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 69037c9 into dotnet:master Jun 23, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the runtime_38189 branch June 23, 2020 15:55
@jkotas jkotas mentioned this pull request Jun 26, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danmoseley danmoseley danmoseley left review comments

@jkotas jkotas jkotas approved these changes

@janvorli janvorli janvorli approved these changes

Assignees
No one assigned
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Interop/UnmanagedCallersOnly fails on OSX.
5 participants
@AaronRobinsonMSFT @jkotas @am11 @janvorli @danmoseley