Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

bootstrap: Update Node requirement to 10.12+ in system requirement checker #23001

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Sep 18, 2021

Requirements for Contributing a Bug Fix (from template, click to expand)

Identify the Bug

Companion PR to #22979.

Description of the Change

Alternate Designs

I suppose we could hard require Node 10.12+, but "deprecate" anything less than currently, officially upstream-supported Node versions? (12.x is the oldest Node still supported by the Node maintainers as of my writing this. See: https://nodejs.org/en/about/releases/).

(I considered not updating the required Node, but the build script errors out with older Node, so it's not a useful check if we don't update it to 10.12+. I do think Node 10.12+ is the sensible minimum requirement now.)

Possible Drawbacks

Atom might be somewhat functional without a chromedriver binary, or without a startup cache? In which case this PR makes it harder to "mostly" build Atom on certain legacy or exotic OSes that we already don't support.

Verification Process

I ran script/build --ci with Node 9, Node 10.11, and Node 10.12.

  • Before this PR, Node versions 4 or newer were allowed... with only a "deprecation warning" for Node 4 or 5... and with Node 6 and above being allowed without even a warning.
  • I confirmed that Node 10.11 or lower will not actually work to build Atom though, due to extract-zip@2 using the recursive option with fs.mkdir, which is only supported in Node 10.12 or newer, and can silently fail on older Node, leading to "file exists" errors in those older versions of Node.
    • Confirmed that Node 10.12 is perfectly capable of running the build scripts and producing a working build of Atom.
    • Now when attempting to bootstrap Atom on Node older than 10.12, the system requirements checker script throws a useful error message about Node being too old.

Release Notes

N/A

"Node 10.12 or newer" has been a hard requirement since this PR:
atom#20879,
due to newer versions of electron-chromedriver and electron-mksnapshot
relying on extract-zip@2 as an indirect dependency.

(extract-zip@2 requires Node 10.12 or newer for its recursive mkdir.
Using extract-zip@2 with Node older than 10.12 results in errors.

That leads to a lack of electron-vendored
chromedriver or mksnapshot binaries where they're supposed to be.
Which in turn causes startup blob creation (via mksnapshot) to fail
toward the end of the Atom build scripts.)
@darangi
Copy link
Contributor

darangi commented Sep 20, 2021

Thanks for the contribution 🙇🏾 @DeeDeeG

@darangi darangi merged commit 925e59f into atom:master Sep 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants