This repository was archived by the owner on Mar 3, 2023. It is now read-only.
Use the "ELECTRON_CUSTOM_VERSION" env var in build scripts#20879
Merged
lkashef merged 5 commits intoatom:masterfrom Jun 5, 2020
Merged
Use the "ELECTRON_CUSTOM_VERSION" env var in build scripts#20879lkashef merged 5 commits intoatom:masterfrom
lkashef merged 5 commits intoatom:masterfrom
Conversation
New feature as of electron-chromedriver >= 9.0.0 and electron-mksnapshot >= 9.0.2: an environment variable "ELECTRON_CUSTOM_VERSION", which allows downloading the specified (Electron-vendored) version of chromedriver and mksnapshot, irrespective of the versions of electron-chromedriver or electron-mksnapshot (node modules) used to download them. We can use the latest electron-chromedriver and electron-mksnapshot now, if we want. Just set ELECTRON_CUSTOM_VERSION to the right version (handled automatically based on "electronVersion" in package.json).
If users manually run `npm install`, we want to make sure the correct Electron-vendored chromedriver and mksnapshot are downloaded. (Requires ELECTRON_CUSTOM_VERSION to be set properly.) This postinstall script sets that var and gets the right binaries, even if the ELECTRON_CUSTOM_VERSION env var wasn't manually set. (The bootstrap script already handles this for bootstrapping, but not for manually running "npm install" in the scripts dir.)
Closed
ghost
reviewed
Jun 3, 2020
DeeDeeG
added a commit
to DeeDeeG/atom
that referenced
this pull request
Sep 14, 2021
"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.)
DeeDeeG
added a commit
to DeeDeeG/atom
that referenced
this pull request
Sep 18, 2021
"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.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements for Adding, Changing, or Removing a Feature
Issue or RFC Endorsed by Atom's Maintainers
Useful for #20764, but applicable to
masterbranch as well.Description of the Change
Use an environment variable,
ELECTRON_CUSTOM_VERSION, to decouple the version of Electron-vendored binaries we want to download (chromedriver,mksnapshot), from the versions of the download helper modules we use to do the downloading (electron-chromedriverandelectron-mksnapshot).Implementation details (click to expand).
This is handled during the bootstrap process here:
https://github.com/DeeDeeG/atom/blob/3e7b532/script/lib/install-script-dependencies.js#L7
Also, make sure this is handled if an Atom developer manually runs
npm installin thescript/directory. Handled here:https://github.com/DeeDeeG/atom/blob/3e7b532/script/redownload-electron-bins.js
and
https://github.com/DeeDeeG/atom/blob/3e7b532/script/package.json#L53
(This is important, because
electron-chromedriverandelectron-mksnapshotdownload chromedriver and mksnapshot as npm postinstall scripts, which are only run the first time they are downloaded. Installing the scripts npm packages the first time, ifELECTRON_CUSTOM_VERSIONisn't set, the wrong Electron-vendored would be downloaded and would persist for further builds.)Alternate Designs
Maybe
script/redownload-electron-bins.jsshould be moved underscript/libs/instead?Possible Drawbacks
Adds complexity to the build scripts.
Users manually running
npm installin thescriptdir will download two versions each ofchromedriverandmksnapshot, unless they have manually setELECTRON_CUSTOM_VERSIONto matchelectronVersionfrom the main Atom package.json beforehand.script/redownload-electron-bins.jsThe second (correct) versions that get downloaded replace the first (incorrect) versions.)script/bootstrap, becauseELECTRON_CUSTOM_VERSIONwill always be set during the bootstrap process.)Verification Process
script/bootstrapwith these changes, on an Ubuntu machine and two Windows machines.npm installfrom inside thescript/directory, on the same three machines.More testing details (click to expand).
You can observe
~/.cache/electron/on Linux, andC:\Users\[User]\AppData\Local\electron\Cacheon Windows to see which versions ofchromedriverandmksnapshotare being downloaded.You can also run
script/node_modules/electron-chromedriver/bin/chromedriver --versionto note what the output is. This should be different across major versions of Electron. e.g.:Release Notes
N/A (this is an Atom-developer-oriented change, not an Atom-user-oriented change.)