-
Notifications
You must be signed in to change notification settings - Fork 816
fix: don't buffer when extracting binary #12621
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
Changes from 5 commits
b812a0c
f9cc0a2
415aab2
67052bc
5899ba4
e1924a3
1655240
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,7 +121,10 @@ export class Binary { | |
console.log(`Downloading release from ${getCompressedBinaryUrl()}`); | ||
try { | ||
const res = await axios({ url: getCompressedBinaryUrl(), responseType: 'stream' }); | ||
await pipeline(res.data, createGunzip(), this.extract()); | ||
// Dummy array to collect a promise from nested pipeline that extracts tar content to a file. | ||
const extractPromiseCollector: Array<Promise<void>> = []; | ||
await pipeline(res.data, createGunzip(), this.extract(extractPromiseCollector)); | ||
await extractPromiseCollector[0]; | ||
|
||
|
||
console.log('amplify has been installed!'); | ||
spawnSync(this.binaryPath, ['version'], { cwd: process.cwd(), stdio: 'inherit' }); | ||
|
@@ -151,29 +154,23 @@ export class Binary { | |
* | ||
* @returns tar.Extract | ||
*/ | ||
private extract(): tar.Extract { | ||
private extract(extractPromiseCollector: Array<Promise<void>>): tar.Extract { | ||
const extract = tar.extract(); | ||
const chunks: Uint8Array[] = []; | ||
extract.on('entry', (header, extractStream, next) => { | ||
if (header.type === 'file') { | ||
extractStream.on('data', (chunk) => { | ||
chunks.push(chunk); | ||
const fileWriteStream = fs.createWriteStream(this.binaryPath, { | ||
mode: 0o755, | ||
}); | ||
// pipe tar entry to file stream | ||
// and collect a promise so that top level process can await it | ||
extractPromiseCollector.push(pipeline(extractStream, fileWriteStream)); | ||
} | ||
extractStream.on('end', () => { | ||
next(); | ||
}); | ||
|
||
extractStream.resume(); | ||
}); | ||
extract.on('finish', () => { | ||
if (chunks.length) { | ||
const data = Buffer.concat(chunks); | ||
fs.writeFileSync(this.binaryPath, data, { | ||
mode: 0o755, | ||
}); | ||
} | ||
}); | ||
return extract; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
12.0.0 release

After applying fix

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice