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

feat: e pr download-dist <number> #679

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.nyc_output/
artifacts
configs
coverage
node_modules
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ git cherry-pick --continue
git push

# create pull request
e pr --backport 1234
e pr open --backport 1234
```

## Common Usage
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"@marshallofsound/chrome-cookies-secure": "^2.1.1",
"@octokit/auth-oauth-device": "^3.1.1",
"@octokit/rest": "^18.5.2",
"adm-zip": "^0.5.16",
"ajv": "^8.11.0",
"ajv-formats": "^2.1.1",
"chalk": "^2.4.1",
Expand Down
2 changes: 1 addition & 1 deletion src/e
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ program
.command('backport [pr]', 'Assists with manual backport processes')
.command('show <subcommand>', 'Show info about the current build config')
.command('test [specRunnerArgs...]', `Run Electron's spec runner`)
.command('pr [options]', 'Open a GitHub URL where you can PR your changes')
.command('pr [subcommand]', 'Work with PRs to electron/electron')
.command('patches <target>', 'Refresh the patches in $root/src/electron/patches/$target')
.command('open <sha1|PR#>', 'Open a GitHub URL for the given commit hash / pull # / issue #')
.command('auto-update', 'Check for build-tools updates or enable/disable automatic updates')
Expand Down
222 changes: 206 additions & 16 deletions src/e-pr.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
#!/usr/bin/env node

const childProcess = require('child_process');
const fs = require('fs');
const path = require('path');

const AdmZip = require('adm-zip');
const querystring = require('querystring');
const semver = require('semver');

const open = require('open');
const program = require('commander');
const { Octokit } = require('@octokit/rest');
const inquirer = require('inquirer');

const { getGitHubAuthToken } = require('./utils/github-auth');
const { current } = require('./evm-config');
const { color, fatal } = require('./utils/logging');

const d = require('debug')('build-tools:pr');

// Adapted from https://github.com/electron/clerk
function findNoteInPRBody(body) {
const onelineMatch = /(?:(?:\r?\n)|^)notes: (.+?)(?:(?:\r?\n)|$)/gi.exec(body);
Expand Down Expand Up @@ -134,27 +141,23 @@ function pullRequestSource(source) {
}

program
.command('open', null, { isDefault: true })
.description('Open a GitHub URL where you can PR your changes')
.option(
'-s, --source <source_branch>',
'Where the changes are coming from',
guessPRSource(current()),
)
.option(
'-t, --target <target_branch>',
'Where the changes are going to',
guessPRTarget(current()),
)
.option('-s, --source [source_branch]', 'Where the changes are coming from')
.option('-t, --target [target_branch]', 'Where the changes are going to')
.option('-b, --backport <pull_request>', 'Pull request being backported')
.action(async (options) => {
if (!options.source) {
const source = options.source || guessPRSource(current());
const target = options.target || guessPRTarget(current());

if (!source) {
fatal(`'source' is required to create a PR`);
} else if (!options.target) {
} else if (!target) {
fatal(`'target' is required to create a PR`);
}

const repoBaseUrl = 'https://github.com/electron/electron';
const comparePath = `${options.target}...${pullRequestSource(options.source)}`;
const comparePath = `${target}...${pullRequestSource(source)}`;
const queryParams = { expand: 1 };

if (!options.backport) {
Expand Down Expand Up @@ -188,5 +191,192 @@ program
}

return open(`${repoBaseUrl}/compare/${comparePath}?${querystring.stringify(queryParams)}`);
})
.parse(process.argv);
});

program
.command('download-dist <pull_request_number>')
.description('Download a pull request dist')
.option(
'--platform [platform]',
'Platform to download dist for. Defaults to current platform.',
process.platform,
)
.option(
'--arch [arch]',
'Architecture to download dist for. Defaults to current arch.',
process.arch,
)
.option(
'-o, --output <output_directory>',
'Specify the output directory for downloaded artifacts. ' +
'Defaults to ~/.electron_build_tools/artifacts/pr_{number}_{platform}_{arch}',
)
.option('-s, --skip-confirmation', 'Skip the confirmation prompt before downloading the dist.')
.action(async (pullRequestNumber, options) => {
if (!pullRequestNumber) {
fatal(`Pull request number is required to download a PR`);
}

d('checking auth...');
const octokit = new Octokit({
auth: await getGitHubAuthToken(['repo']),
});

d('fetching pr info...');
let pullRequest;
try {
const { data } = await octokit.pulls.get({
owner: 'electron',
repo: 'electron',
pull_number: pullRequestNumber,
});
pullRequest = data;
} catch (error) {
console.error(`Failed to get pull request: ${error}`);
return;
}

if (!options.skipConfirmation) {
const isElectronRepo = pullRequest.head.repo.full_name !== 'electron/electron';
const { proceed } = await inquirer.prompt([
{
type: 'confirm',
name: 'proceed',
message: `You are about to download artifacts from:

“${pullRequest.title} (#${pullRequest.number})” by ${pullRequest.user.login}
${pullRequest.head.repo.html_url}${isElectronRepo ? ' (fork)' : ''}

Proceed?`,
},
]);

if (!proceed) return;
}

d('fetching workflow runs...');
let workflowRuns;
try {
const { data } = await octokit.actions.listWorkflowRunsForRepo({
owner: 'electron',
repo: 'electron',
branch: pullRequest.head.ref,
name: 'Build',
event: 'pull_request',
status: 'completed',
per_page: 10,
sort: 'created',
direction: 'desc',
});
dsanders11 marked this conversation as resolved.
Show resolved Hide resolved
workflowRuns = data.workflow_runs;
} catch (error) {
console.error(`Failed to list workflow runs: ${error}`);
return;
}

const latestBuildWorkflowRun = workflowRuns.find((run) => run.name === 'Build');
if (!latestBuildWorkflowRun) {
fatal(`No 'Build' workflow runs found for pull request #${pullRequestNumber}`);
return;
}

d('fetching artifacts...');
let artifacts;
try {
const { data } = await octokit.actions.listWorkflowRunArtifacts({
owner: 'electron',
repo: 'electron',
run_id: latestBuildWorkflowRun.id,
});
artifacts = data.artifacts;
} catch (error) {
console.error(`Failed to list artifacts: ${error}`);
return;
}

const artifactName = `generated_artifacts_${options.platform}_${options.arch}`;
const artifact = artifacts.find((artifact) => artifact.name === artifactName);
if (!artifact) {
console.error(`Failed to find artifact: ${artifactName}`);
return;
}

let outputDir;

if (options.output) {
outputDir = path.resolve(options.output);

if (!(await fs.promises.stat(outputDir).catch(() => false))) {
fatal(`The output directory '${options.output}' does not exist`);
return;
}
} else {
const artifactsDir = path.resolve(__dirname, '..', 'artifacts');
const defaultDir = path.resolve(
artifactsDir,
`pr_${pullRequest.number}_${options.platform}_${options.arch}`,
);

// Clean up the directory if it exists
try {
await fs.promises.rm(defaultDir, { recursive: true, force: true });
} catch (error) {
if (error.code !== 'ENOENT') {
throw error;
}
}

// Create the directory
await fs.promises.mkdir(defaultDir, { recursive: true });

outputDir = defaultDir;
}

console.log(
`Downloading artifact '${artifactName}' from pull request #${pullRequestNumber}...`,
);

const response = await octokit.actions.downloadArtifact({
owner: 'electron',
repo: 'electron',
artifact_id: artifact.id,
archive_format: 'zip',
});

// Extract artifact zip in-memory
const artifactZip = new AdmZip(Buffer.from(response.data));

const distZipEntry = artifactZip.getEntry('dist.zip');
if (!distZipEntry) {
fatal('dist.zip not found in build artifact.');
return;
}

// Extract dist.zip in-memory
const distZipContents = artifactZip.readFile(distZipEntry);
const distZip = new AdmZip(distZipContents);

const platformExecutables = {
win32: 'electron.exe',
darwin: 'Electron.app/',
linux: 'electron',
};

const executableName = platformExecutables[options.platform];
if (!executableName) {
fatal(`Unable to find executable for platform '${options.platform}'`);
return;
}

if (!distZip.getEntry(executableName)) {
fatal(`${executableName} not found within dist.zip.`);
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Mac versions, do we want to automatically remove the quarantine with xattr -c Electron.app since the app will be unsigned, or do we want to leave that to the developer?

I suppose we could either prompt the caller, or add a CLI flag if it doesn't seem safe to automatically remove it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also wondering about this. When I run the script, and double-click the .app, it doesn't seem to show me the popup about it being downloaded from the internet 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe when I manually downloaded the artifact, that attribute was added by Chrome to the .zip and then propagated to the .app when I extracted the zip. If downloading it via a script doesn't encounter the same problem, then all the better :)


// Extract dist.zip to the output directory
await distZip.extractAllToAsync(outputDir);

console.info(`Downloaded to ${outputDir}`);
});

program.parse(process.argv);
36 changes: 8 additions & 28 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,11 @@ acorn@^8.8.0:
resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.8.2.tgz#1b2f25db02af965399b9776b0c2c391276d37c4a"
integrity sha512-xjIYgE8HBrkpd/sJqOGNspf8uHG+NOHGOw6a/Urj8taM2EXfdNAH2oFcPeIFfsv3+kz/mJrS5VuMqbNLjCa2vw==

adm-zip@^0.5.16:
version "0.5.16"
resolved "https://registry.yarnpkg.com/adm-zip/-/adm-zip-0.5.16.tgz#0b5e4c779f07dedea5805cdccb1147071d94a909"
integrity sha512-TGw5yVi4saajsSEgz25grObGHEUaDrniwvA2qwSC060KfqGPdglhvPMA2lPIoxs3PQIItj2iag35fONcQqgUaQ==

agent-base@6, agent-base@^6.0.2:
version "6.0.2"
resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-6.0.2.tgz#49fff58577cfee3f37176feab4c22e00f86d7f77"
Expand Down Expand Up @@ -4698,16 +4703,7 @@ string-argv@~0.3.2:
resolved "https://registry.yarnpkg.com/string-argv/-/string-argv-0.3.2.tgz#2b6d0ef24b656274d957d54e0a4bbf6153dc02b6"
integrity sha512-aqD2Q0144Z+/RqG52NeHEkZauTAUWJO8c6yTftGJKO3Tja5tUgIfmIl6kExvhtxSDP7fXB6DvzkfMpCd/F3G+Q==

"string-width-cjs@npm:string-width@^4.2.0":
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -4782,14 +4778,7 @@ string_decoder@^1.1.1:
dependencies:
safe-buffer "~5.2.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^6.0.0, strip-ansi@^6.0.1:
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
Comment on lines -4785 to +4825
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're using an older version of Yarn so these are getting folded back into one, which can actually cause some weird issues (not sure it will in this specific case). If I revert these changes and run yarn install with v1.22.22 the only change I get is the adm-zip addition.

version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand Down Expand Up @@ -5333,7 +5322,7 @@ word-wrap@^1.2.3:
resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.4.tgz#cb4b50ec9aca570abd1f52f33cd45b6c61739a9f"
integrity sha512-2V81OA4ugVo5pRo46hAoD2ivUJx8jXmWXfUkY4KFNw0hEptvN0QfH3K4nHiwzGeKl5rFKedV48QVoqYavy4YpA==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand All @@ -5351,15 +5340,6 @@ wrap-ansi@^6.2.0:
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"
Expand Down