Skip to content

tools: fix installing headers on windows #40943

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Kukkimonsuta
Copy link

Installing headers on windows (python3 .\tools\install.py install) skips v8 headers in folders cppgc and libplatform caused by difference in directory separators returned by os.walk vs v8_headers whitelist. This change simply normalizes separators to match v8_headers whitelist.

@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. python PRs and issues that require attention from people who are familiar with Python. labels Nov 24, 2021
@Trott
Copy link
Member

Trott commented Nov 25, 2021

@nodejs/python @nodejs/platform-windows

@cclauss
Copy link
Contributor

cclauss commented Nov 25, 2021

The joys of https://docs.python.org/3/library/os.html#os.sep

I would be happier to see this file reworked to use the more intuitive and less error-prone pathlib rather than os.path.

Installing headers on windows skips v8 headers in folders cppgc and libplatform caused by difference in directory separators returned by os.walk vs v8_headers whitelist. This change simply normalizes separators to match v8_headers whitelist.
@Kukkimonsuta Kukkimonsuta force-pushed the fix-install-headers-on-windows branch from 9a294c6 to 9973ebc Compare November 25, 2021 10:09
@Kukkimonsuta
Copy link
Author

Fixed the code style based on suggested change.

I briefly looked into changing the file to pathlib, but I don't think it's something I'm able to do right now.

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. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants