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

build: fix v8 symbols for Windows on Arm #18592

Closed

Conversation

richard-townsend-arm
Copy link
Contributor

@richard-townsend-arm richard-townsend-arm commented Jun 3, 2019

Description of Change

Adjusts V8 symbol visibility so that symbols are kept private for the host toolchain and are exposed for the target toolchain, which means significantly fewer workarounds are needed for symbol visibility on Windows on Arm.

Checklist

Release Notes

Notes: no-notes

Excluding the host-toolchain from symbol exports means that the Windows
on Arm build requires fewer workarounds.
@richard-townsend-arm richard-townsend-arm requested a review from a team as a code owner June 3, 2019 16:55
@richard-townsend-arm
Copy link
Contributor Author

CC @MarshallOfSound @jkleinsc

@jkleinsc jkleinsc requested a review from nornagon June 3, 2019 20:31
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM, but this new check can be combined into the condition for macro HIDE_PRIVATE_SYMBOLS introduced in #18281 since they are trying to achieve the same.

@codebytere
Copy link
Member

@richard-townsend-arm please fix patch conflicts and address deepak's review above when you have a moment!

@richard-townsend-arm
Copy link
Contributor Author

Hi (apologies, this fell off the end of my to-do list), it seems that #18281 fixed the build error for me, so closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants