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

cmake: allow to disable symbolize #620

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

sergiud
Copy link
Collaborator

@sergiud sergiud commented Mar 30, 2021

Closes #563

@drigz This change might break Bazel builds. Maybe there is an alternative to removal?

@google-cla google-cla bot added the cla: yes label Mar 30, 2021
@sergiud sergiud added this to the 0.5 milestone Mar 30, 2021
@sergiud sergiud self-assigned this Mar 30, 2021
@sergiud sergiud added the bug label Mar 30, 2021
Copy link
Member

@drigz drigz left a comment

Choose a reason for hiding this comment

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

Hmm, I can't be certain but I guess some people might rely on symbolize. Maybe add:

    linux_or_darwin_copts = wasm_copts + [
        # For src/symbolize.cc.
        "-DHAVE_SYMBOLIZE",

and

    windows_only_copts = [
        # For src/symbolize.cc.
        "-DHAVE_SYMBOLIZE",

to preserve the previous functionality?

linux_or_darwin_copts = wasm_copts + [

@sergiud sergiud force-pushed the cmake-allow-to-disable-symbolize branch from 33b7311 to 9c7cbb9 Compare April 7, 2021 20:49
@sergiud sergiud requested a review from drigz April 7, 2021 20:50
@sergiud sergiud force-pushed the cmake-allow-to-disable-symbolize branch from 9c7cbb9 to 27704b8 Compare April 7, 2021 20:58
@sergiud sergiud force-pushed the cmake-allow-to-disable-symbolize branch from 27704b8 to 39e8358 Compare April 7, 2021 20:59
@sergiud
Copy link
Collaborator Author

sergiud commented Apr 7, 2021

@drigz I reverted the breaking changes. Bazel builds are now unaffected.

Copy link
Member

@drigz drigz left a comment

Choose a reason for hiding this comment

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

Thanks!

@sergiud sergiud merged commit 663bb26 into google:master Apr 9, 2021
@sergiud sergiud deleted the cmake-allow-to-disable-symbolize branch April 9, 2021 13:06
@sergiud sergiud mentioned this pull request May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling 'Symbolize functionality' is not working
2 participants