-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
tools, build: make v8 build pythonic #58304
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
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
I don't think that's accurate, |
Yea, that's what I meant. I've updated the description. |
I'm lukewarm on changes like these. This PR adds more lines of code than it removes, while the current script works fine, no one is complaining. |
I wouldn't really say the current script "works fine": it only works on POSIX terminals, whereas on other terminals, like PowerShell, it won't execute. |
On a side note, the Windows prerequisites section explicitly lists a POSIX-compatible shell as a prerequisite, even if only for tests:
Is it common to install Python and Git on Windows but not Git Bash? Or will the existing script fail in Git Bash? |
cc @guybedford might have some opinions on this, since he's the only one I know personally who builds node.js on windows |
Seems to work, although when I tried |
FWIW The original script also expected arguments |
|
I like the idea of using more of Python for specific scripts that we use - and specifically try to avoid relying on I am not sure PowerShell support is worthy of a goal, we already require Git Bash on Windows. I'd be more optimistic about supporting Make run in e.g. MSYS2 shell/Git Bash instead, considering PowerShell is yet another tool that most people aren't familiar with. |
While investigating #57964, I noticed that some of our tooling assumes a POSIX environment. To enhance cross-platform compatibility, this PR replaces
make-v8.sh
withbuild_v8.py
, allowing the build process to run more reliably across different systems.I've tested this on my Windows setup.