refactor: switch to @electron/get by codebytere · Pull Request #59 · joaomoreno/gulp-atom-electron

codebytere · 2020-04-22T04:52:14Z

This PR updates gulp-atom-electron to use Electron's recommended @electron/get for simpler downloading and automatic caching. Some other benefits include the ability to download nightly releases as well as specify custom mirrors.

Tested locally with Electron versions v7.2.1 and v7.2.3 to confirm efficacy of change when running yarn electron in vscode itself.

cc @deepak1556

deepak1556 · 2020-04-22T04:53:37Z

/cc @joaomoreno @bpasero

deepak1556 · 2020-04-22T04:59:55Z

-
 function download(opts, cb) {
-	var repo = opts.repo || 'atom/electron';
-	var github = new GitHub({ repo: repo, token: opts.token });


We still need to support this configuration for the internal builds.

Ah I see that we can specify a mirror, that should do the trick

How to do that?

deepak1556 · 2020-04-22T05:01:58Z

Thanks for working on this @codebytere

bpasero · 2020-04-22T05:36:29Z

Cool!

joaomoreno · 2020-04-22T09:43:56Z

@deepak1556 Would you like to drive this PR's adoption in VS Code? I've invited you as a collaborator.

deepak1556 · 2020-04-22T10:24:05Z

Thanks @joaomoreno , would be happy to do it.

joaomoreno · 2020-04-22T14:14:27Z

Another idea @bpasero and I just discussed about: how about just dropping this? Do we still need this module at all?

bpasero · 2020-04-22T14:49:02Z

I was referring to https://github.com/electron-userland/electron-forge in the standup today.

codebytere · 2020-04-22T15:20:56Z

What do you all envision the alternate path looking like, should we choose to take it? I'm happy to switch up my approach or try to PR something into vscode itself

joaomoreno · 2020-04-23T09:11:17Z

@codebytere No, no, this was more of a comment for us, VS Code team.

codebytere · 2020-05-04T03:32:31Z

👋 hey folks, is there anything i can do to help move this PR forward this week?

deepak1556 · 2020-05-04T04:43:29Z

@codebytere last week was testing in vscode during which the master branch will be frozen, I plan to move this forward tomorrow.

deepak1556 · 2020-05-06T03:19:03Z

Thanks @codebytere for the fast turnaround, the following changes are also needed

diff --git a/src/download.js b/src/download.js
index f408555..5363fdd 100644
--- a/src/download.js
+++ b/src/download.js
@@ -1,7 +1,8 @@
 'use strict';
 
 var path = require('path');
-const { downloadArtifact, getDownloadUrl } = require('@electron/get');
+const { downloadArtifact } = require('@electron/get');
+const { getDownloadUrl } = require('./util')
 var semver = require('semver');
 var rename = require('gulp-rename');
 var es = require('event-stream');
@@ -31,15 +32,20 @@ function download(opts, cb) {
 		version: opts.version,
 		platform: opts.platform,
 		arch,
-		token
+		token: opts.token
 	};
 
 	if (opts.repo) {
 		getDownloadUrl(opts.repo, downloadOptions)
-		.then(({err, downloadUrl}) => {
+		.then(({err, downloadUrl, assetName}) => {
 			if (err) return cb(err)
 	
-			downloadOptions['mirrorOptions'] = { mirror: downloadUrl }
+			downloadOptions['mirrorOptions'] = {
+				mirror: downloadUrl,
+				customFilename: assetName
+			};
+			downloadOptions['artifactName'] = assetName;
+			downloadOptions['unsafelyDisableChecksums'] = true;
 	
 			downloadArtifact(downloadOptions).then(zipFilePath => {
 				return cb(null, zipFilePath)
diff --git a/src/util.js b/src/util.js
index 54499fa..2ace0fb 100644
--- a/src/util.js
+++ b/src/util.js
@@ -7,8 +7,8 @@ function startsWith (haystack, needle) {
 	return haystack.substr(0, needle.length) === needle;
 };
 
-async function getDownloadUrl(repo, { version, platform, arch, token }) {
-	const [owner, repo] = repo.split('/');
+async function getDownloadUrl(repo_url, { version, platform, arch, token }) {
+	const [owner, repo] = repo_url.split('/');
 	const octokit = new Octokit({ auth: token });
 
 	const { data: release } = await octokit.repos.getReleaseByTag({
@@ -46,7 +46,7 @@ async function getDownloadUrl(repo, { version, platform, arch, token }) {
 	});
 
 	const { url, headers } = requestOptions;
-  headers.authorization = `token ${token}`;
+	headers.authorization = `token ${token}`;
 
 	const response = await got(url, {
 		followRedirect: false,
@@ -54,7 +54,7 @@ async function getDownloadUrl(repo, { version, platform, arch, token }) {
 		headers
 	});
 
-	return { error: null, location: response.headers.location };
+	return { error: null, location: response.headers.location, assetName: asset.name };
 }
 
 module.exports = {

deepak1556 · 2020-05-06T03:23:21Z

features from the previous version are preserved now,

Caching
downloads from custom repo

There are two issues that still needs addressing

the artifact filename in the cache folder has mangled various names together and looks like
httpsgithub.comelectronelectronreleasesdownloadv7.2.4electron-v7.2.4-darwin-x64.zip when downloaded from custom repo
Although the default behavior is to display progress bar only if the download is greater than 30s , which is mostly not the case here as I don't see it in my testing. I would still like some feedback when its less than 30s.

deepak1556

LGTM with minor style changes.

deepak1556

Thanks for working through my review 😄

Good to go now. @joaomoreno if you can make a new release, will adopt this in VSCode. Thanks!

This reverts commit 93e6619, reversing changes made to 4ff185e.

deepak1556 reviewed Apr 22, 2020

View reviewed changes

codebytere force-pushed the swap-to-electron-get branch from c2bcdf1 to 44ca6ab Compare April 22, 2020 21:09

refactor: switch to @electron/get

444e748

codebytere force-pushed the swap-to-electron-get branch from 44ca6ab to 444e748 Compare April 22, 2020 21:10

joaomoreno assigned deepak1556 May 4, 2020

deepak1556 suggested changes May 6, 2020

View reviewed changes

Comment thread src/download.js Outdated

Comment thread src/download.js

Comment thread src/download.js Outdated

Comment thread src/download.js Outdated

Comment thread src/util.js Outdated

Comment thread src/util.js Outdated

Comment thread src/util.js Outdated

codebytere force-pushed the swap-to-electron-get branch from 27972f5 to cc5b4e9 Compare May 6, 2020 00:35

codebytere requested a review from deepak1556 May 6, 2020 00:39

codebytere force-pushed the swap-to-electron-get branch from cc5b4e9 to e942403 Compare May 6, 2020 03:22

codebytere force-pushed the swap-to-electron-get branch from e942403 to 97bfded Compare May 6, 2020 03:23

Address feedback

e0222ba

codebytere force-pushed the swap-to-electron-get branch 2 times, most recently from 99a2415 to d9a070a Compare May 6, 2020 20:28

deepak1556 reviewed May 7, 2020

View reviewed changes

Comment thread package.json Outdated

Comment thread package.json Outdated

Comment thread src/download.js Outdated

Comment thread src/download.js Outdated

Comment thread src/download.js Outdated

codebytere force-pushed the swap-to-electron-get branch 2 times, most recently from 9708956 to e582f5e Compare May 7, 2020 03:44

chore: fix progressBar logging

f3f50e7

codebytere force-pushed the swap-to-electron-get branch from e582f5e to f3f50e7 Compare May 7, 2020 20:58

deepak1556 reviewed May 8, 2020

View reviewed changes

Comment thread src/download.js Outdated

deepak1556 reviewed May 8, 2020

View reviewed changes

Comment thread src/download.js Outdated

Display progress bar for OSS electron

1f3bee0

codebytere force-pushed the swap-to-electron-get branch from 15302b2 to 1f3bee0 Compare May 8, 2020 02:51

codebytere requested a review from deepak1556 May 8, 2020 02:51

deepak1556 approved these changes May 8, 2020

View reviewed changes

deepak1556 merged commit 93e6619 into joaomoreno:master May 8, 2020

kieferrm mentioned this pull request May 10, 2020

Iteration Plan for May 2020 microsoft/vscode#97432

Closed

61 tasks

deepak1556 added a commit that referenced this pull request Aug 14, 2020

Revert "Merge pull request #59 from codebytere/swap-to-electron-get"

8cb24b1

This reverts commit 93e6619, reversing changes made to 4ff185e.

joaomoreno mentioned this pull request Jan 25, 2021

Downloading from custom repo always downloads electron.zip #67

Open

Conversation

codebytere commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepak1556 commented Apr 22, 2020

Uh oh!

deepak1556 Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

deepak1556 Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

joaomoreno Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

deepak1556 commented Apr 22, 2020

Uh oh!

bpasero commented Apr 22, 2020

Uh oh!

joaomoreno commented Apr 22, 2020

Uh oh!

deepak1556 commented Apr 22, 2020

Uh oh!

joaomoreno commented Apr 22, 2020

Uh oh!

bpasero commented Apr 22, 2020

Uh oh!

codebytere commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joaomoreno commented Apr 23, 2020

Uh oh!

codebytere commented May 4, 2020

Uh oh!

deepak1556 commented May 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deepak1556 commented May 6, 2020

Uh oh!

deepak1556 commented May 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codebytere commented Apr 22, 2020 •

edited

Loading

codebytere commented Apr 22, 2020 •

edited

Loading

deepak1556 commented May 6, 2020 •

edited

Loading