Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Commit 9b8ed5d

Browse files
authored
Merge pull request #688 from atom/ku-renderer-shell-out
Run git commands in dedicated renderer process
2 parents d3c8b5b + e708668 commit 9b8ed5d

13 files changed

+808
-62
lines changed

.eslintrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"MouseEvent": true,
2121
"IDBKeyRange": true,
2222
"beforeEach": true,
23+
"after": true,
2324
"afterEach": true,
2425
"describe": true,
2526
"fdescribe": true,

lib/async-queue.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ export default class AsyncQueue {
3939
}
4040

4141
push(fn, {parallel} = {parallel: true}) {
42-
if (this.disposed) {
43-
throw new Error('AsyncQueue is disposed');
44-
}
4542
const task = new Task(fn, parallel);
4643
this.queue.push(task);
4744
this.processQueue();

lib/controllers/git-tab-controller.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ export default class GitTabController {
226226
const pathsToIgnore = [];
227227
const repository = this.getActiveRepository();
228228
for (const filePath of filePaths) {
229-
if (await repository.pathHasMergeMarkers(filePath)) { // eslint-disable-line babel/no-await-in-loop
229+
if (await repository.pathHasMergeMarkers(filePath)) { // eslint-disable-line no-await-in-loop
230230
const choice = atom.confirm({
231231
message: 'File contains merge markers: ',
232232
detailedMessage: `Do you still want to stage this file?\n${filePath}`,

lib/git-shell-out-strategy.js

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ import path from 'path';
22
import os from 'os';
33

44
import {CompositeDisposable} from 'event-kit';
5-
65
import {GitProcess} from 'dugite';
76
import {parse as parseDiff} from 'what-the-diff';
87

98
import GitPromptServer from './git-prompt-server';
109
import AsyncQueue from './async-queue';
11-
import {getPackageRoot, getDugitePath, readFile, fileExists, writeFile, isFileExecutable} from './helpers';
10+
import {getPackageRoot, getDugitePath, readFile, fileExists, fsStat, writeFile, isFileExecutable} from './helpers';
1211
import GitTimingsView from './views/git-timings-view';
12+
import WorkerManager from './worker-manager';
1313

1414
const LINE_ENDING_REGEX = /\r?\n/;
1515

@@ -57,6 +57,7 @@ export default class GitShellOutStrategy {
5757
}
5858

5959
this.prompt = options.prompt || (query => Promise.reject());
60+
this.workerManager = options.workerManager;
6061
}
6162

6263
/*
@@ -131,6 +132,7 @@ export default class GitShellOutStrategy {
131132
const options = {
132133
env,
133134
processCallback: child => {
135+
// TODO: move callback to renderer process. send child.pid back to add cancel listener
134136
child.on('error', err => {
135137
console.warn('Error executing: ' + formattedArgs + ':');
136138
console.warn(err.stack);
@@ -156,51 +158,68 @@ export default class GitShellOutStrategy {
156158
if (process.env.PRINT_GIT_TIMES) {
157159
console.time(`git:${formattedArgs}`);
158160
}
159-
return new Promise(resolve => {
160-
timingMarker.mark('nexttick');
161-
setImmediate(() => {
162-
timingMarker.mark('execute');
163-
resolve(GitProcess.exec(args, this.workingDir, options)
164-
.then(({stdout, stderr, exitCode}) => {
165-
timingMarker.finalize();
166-
if (process.env.PRINT_GIT_TIMES) {
167-
console.timeEnd(`git:${formattedArgs}`);
168-
}
169-
if (gitPromptServer) {
170-
gitPromptServer.terminate();
171-
}
172-
subscriptions.dispose();
173-
174-
if (diagnosticsEnabled) {
175-
const headerStyle = 'font-weight: bold; color: blue;';
176-
177-
console.groupCollapsed(`git:${formattedArgs}`);
178-
console.log('%cexit status%c %d', headerStyle, 'font-weight: normal; color: black;', exitCode);
179-
console.log('%cstdout', headerStyle);
180-
console.log(stdout);
181-
console.log('%cstderr', headerStyle);
182-
console.log(stderr);
183-
console.groupEnd();
184-
}
185-
186-
if (exitCode) {
187-
const err = new GitError(
188-
`${formattedArgs} exited with code ${exitCode}\nstdout: ${stdout}\nstderr: ${stderr}`,
189-
);
190-
err.code = exitCode;
191-
err.stdErr = stderr;
192-
err.stdOut = stdout;
193-
err.command = formattedArgs;
194-
return Promise.reject(err);
195-
}
196-
return stdout;
197-
}));
198-
});
161+
return new Promise(async (resolve, reject) => {
162+
const {stdout, stderr, exitCode, timing} = await this.executeGitCommand(args, options, timingMarker);
163+
if (timing) {
164+
const {execTime, spawnTime, ipcTime} = timing;
165+
const now = performance.now();
166+
timingMarker.mark('nexttick', now - execTime - spawnTime - ipcTime);
167+
timingMarker.mark('execute', now - execTime - ipcTime);
168+
timingMarker.mark('ipc', now - ipcTime);
169+
}
170+
timingMarker.finalize();
171+
if (process.env.PRINT_GIT_TIMES) {
172+
console.timeEnd(`git:${formattedArgs}`);
173+
}
174+
if (gitPromptServer) {
175+
gitPromptServer.terminate();
176+
}
177+
subscriptions.dispose();
178+
179+
if (diagnosticsEnabled) {
180+
const headerStyle = 'font-weight: bold; color: blue;';
181+
182+
console.groupCollapsed(`git:${formattedArgs}`);
183+
console.log('%cexit status%c %d', headerStyle, 'font-weight: normal; color: black;', exitCode);
184+
console.log('%cstdout', headerStyle);
185+
console.log(stdout);
186+
console.log('%cstderr', headerStyle);
187+
console.log(stderr);
188+
console.groupEnd();
189+
}
190+
191+
if (exitCode) {
192+
const err = new GitError(
193+
`${formattedArgs} exited with code ${exitCode}\nstdout: ${stdout}\nstderr: ${stderr}`,
194+
);
195+
err.code = exitCode;
196+
err.stdErr = stderr;
197+
err.stdOut = stdout;
198+
err.command = formattedArgs;
199+
reject(err);
200+
}
201+
resolve(stdout);
199202
});
200203
}, {parallel: !writeOperation});
201204
/* eslint-enable no-console */
202205
}
203206

207+
executeGitCommand(args, options, marker = null) {
208+
if (process.env.ATOM_GITHUB_INLINE_GIT_EXEC || !WorkerManager.getInstance().isReady()) {
209+
marker && marker.mark('nexttick');
210+
const promise = GitProcess.exec(args, this.workingDir, options);
211+
marker && marker.mark('execute');
212+
return promise;
213+
} else {
214+
const workerManager = this.workerManager || WorkerManager.getInstance();
215+
return workerManager.request({
216+
args,
217+
workingDir: this.workingDir,
218+
options,
219+
});
220+
}
221+
}
222+
204223
/**
205224
* Execute a git command that may create a commit. If the command fails because the GPG binary was invoked and unable
206225
* to acquire a passphrase (because the pinentry program attempted to use a tty), retry with a `GitPromptServer`.
@@ -220,6 +239,7 @@ export default class GitShellOutStrategy {
220239

221240
async isGitRepository() {
222241
try {
242+
await fsStat(this.workingDir); // fails if folder doesn't exist
223243
await this.exec(['rev-parse', '--resolve-git-dir', path.join(this.workingDir, '.git')]);
224244
return true;
225245
} catch (e) {

lib/github-package.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import Switchboard from './switchboard';
1919
import yardstick from './yardstick';
2020
import GitTimingsView from './views/git-timings-view';
2121
import AsyncQueue from './async-queue';
22+
import WorkerManager from './worker-manager';
2223

2324
const defaultState = {
2425
firstRun: true,
@@ -216,6 +217,7 @@ export default class GithubPackage {
216217
this.subscriptions.dispose();
217218
this.contextPool.clear();
218219
await yardstick.flush();
220+
WorkerManager.reset();
219221
}
220222

221223
@autobind

lib/renderer.html

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<title></title>
6+
<script type="text/javascript">
7+
const qs = require('querystring')
8+
const jsPath = qs.parse(window.location.search.substr(1)).js
9+
require(jsPath)
10+
</script>
11+
</head>
12+
<body>
13+
<h1>GitHub Package Git Execution Window</h1>
14+
<p>
15+
Hi there! I'm a window used by the GitHub package to execute Git commands in the background. My PID is <script>document.write(process.pid)</script>.
16+
</p>
17+
<p>Last command: <span id='command'></span></p>
18+
</body>
19+
</html>

lib/views/git-timings-view.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ class Marker {
4343
return this.end;
4444
}
4545

46-
mark(sectionName) {
47-
this.markers.push({name: sectionName, start: performance.now()});
46+
mark(sectionName, start) {
47+
this.markers.push({name: sectionName, start: start || performance.now()});
4848
}
4949

5050
finalize() {
@@ -98,6 +98,7 @@ const COLORS = {
9898
prepare: 'cyan',
9999
nexttick: 'yellow',
100100
execute: 'green',
101+
ipc: 'pink',
101102
};
102103
class MarkerSpan extends React.Component {
103104
static propTypes = {

0 commit comments

Comments
 (0)