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

Use the "ELECTRON_CUSTOM_VERSION" env var in build scripts #20879

Merged

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Jun 3, 2020

Requirements for Adding, Changing, or Removing a Feature

Issue or RFC Endorsed by Atom's Maintainers

Useful for #20764, but applicable to master branch 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-chromedriver and electron-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 install in the script/ 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-chromedriver and electron-mksnapshot download 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, if ELECTRON_CUSTOM_VERSION isn't set, the wrong Electron-vendored would be downloaded and would persist for further builds.)

Alternate Designs

Maybe script/redownload-electron-bins.js should be moved under script/libs/ instead?

Possible Drawbacks

Adds complexity to the build scripts.

Users manually running npm install in the script dir will download two versions each of chromedriver and mksnapshot, unless they have manually set ELECTRON_CUSTOM_VERSION to match electronVersion from the main Atom package.json beforehand.

  • (This is the intended behavior of script/redownload-electron-bins.js The second (correct) versions that get downloaded replace the first (incorrect) versions.)
  • (This doesn't happen if running script/bootstrap, because ELECTRON_CUSTOM_VERSION will always be set during the bootstrap process.)

Verification Process

  • Tested running script/bootstrap with these changes, on an Ubuntu machine and two Windows machines.
  • Also tested running npm install from inside the script/ directory, on the same three machines.
More testing details (click to expand).

You can observe ~/.cache/electron/ on Linux, and C:\Users\[User]\AppData\Local\electron\Cache on Windows to see which versions of chromedriver and mksnapshot are being downloaded.

You can also run script/node_modules/electron-chromedriver/bin/chromedriver --version to note what the output is. This should be different across major versions of Electron. e.g.:

# Electron 5.0.13
$ script/node_modules/electron-chromedriver/bin/chromedriver --version
ChromeDriver 2.45 (f2d9a15026dc9c754d5f8516c1bf2a1ae1f369a1)

# Electron 6.1.12
$ script/node_modules/electron-chromedriver/bin/chromedriver --version
ChromeDriver 76.0.3809.146 (02ca9653b91d5b2122cb221b499f6d5aabb5fdbf-refs/heads/master@{#758887})

Release Notes

N/A (this is an Atom-developer-oriented change, not an Atom-user-oriented change.)

DeeDeeG added 4 commits Jun 3, 2020
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.)
@DeeDeeG DeeDeeG mentioned this pull request Jun 3, 2020
@lkashef lkashef merged commit fdb76d4 into atom:master Jun 5, 2020
1 check passed
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.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants