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 errors with pointer compression #55327

Closed

Conversation

sapics
Copy link
Contributor

@sapics sapics commented Oct 9, 2024

This PR would fix the issue #54531 which reports build error in linux with pointer-compression.
#54531 reports that the PR #53884 code causes the error.
#53884 was fixed the error reported in #51339.

In #51339, the error is reported in macOS, therefore, I make to be v8_enable_sandbox=1 only if OS is macOS.

I am not good at English and python and clang, if there are anything wrong, please notice me.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Oct 9, 2024
@RedYetiDev RedYetiDev added the macos Issues and PRs related to the macOS platform / OSX. label Oct 9, 2024
@mhdawson
Copy link
Member

mhdawson commented Oct 9, 2024

I don't quite understand how this fixes the issue. It seems to just not enable except on Mac. From my quick read of the related issues I'm not sure how not enabling on linux but enabling on Mac makes sense.

@sapics can you explain a bit more as to why this is the right fix?

@sapics
Copy link
Contributor Author

sapics commented Oct 10, 2024

Thanks for your attension!
Sorry for the lack of explanation.

The reason for disabling except Mac

In #53884 (0c2aa7d), it changes

+ o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0

I changed as follows from the assumption that #53884 was probably effective on MacOS.

- o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0
+ o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression and flavor == 'mac' else 0

The other way

I have confirmed only in Linux, thus, I cannot judge if v8_enable_sandbox=1 is good for the other OS.

If the #51339 have not solved by #53884 merged, it might be better to fix

- o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0

or

- o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression else 0
+ o['variables']['v8_enable_sandbox'] = 1 if options.enable_pointer_compression and flavor != 'linux' else 0

@sapics
Copy link
Contributor Author

sapics commented Oct 14, 2024

Sorry, I close this PR, because I am not a specialist of this work.

@sapics sapics closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. macos Issues and PRs related to the macOS platform / OSX. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants