Skip to content

Commit b21e845

Browse files
dsanders11dsanders11
committed
fix: extract tarball to temp directory on Windows
1 parent 70bcef0 commit b21e845

2 files changed

Lines changed: 120 additions & 60 deletions

File tree

‎lib/install.js‎

Lines changed: 119 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
const fs = require('graceful-fs')
44
const os = require('os')
5+
const { backOff } = require('exponential-backoff')
6+
const rm = require('rimraf')
57
const tar = require('tar')
68
const path = require('path')
79
const util = require('util')
@@ -144,72 +146,129 @@ async function install (fs, gyp, argv) {
144146

145147
// download the tarball and extract!
146148

147-
if (tarPath) {
148-
await tar.extract({
149-
file: tarPath,
150-
strip: 1,
151-
filter: isValid,
152-
onwarn,
153-
cwd: devDir
154-
})
155-
} else {
156-
try {
157-
const res = await download(gyp, release.tarballUrl)
149+
let tarExtractDir = devDir
158150

159-
if (res.status !== 200) {
160-
throw new Error(`${res.status} response downloading ${release.tarballUrl}`)
161-
}
151+
// on Windows there can be file errors from tar if parallel installs
152+
// are happening (not uncommon with multiple native modules) so
153+
// extract the tarball to a temp directory first and then swap
154+
if (win) {
155+
// place the temp directory next to devDir rather than in os.tmp()
156+
// since fs.rename() doesn't work if the source and destination
157+
// are on different drives, which might be the case here
158+
tarExtractDir = await fs.promises.mkdtemp(`${devDir}-tmp-`)
159+
}
162160

163-
await streamPipeline(
164-
res.body,
165-
// content checksum
166-
new ShaSum((_, checksum) => {
167-
const filename = path.basename(release.tarballUrl).trim()
168-
contentShasums[filename] = checksum
169-
log.verbose('content checksum', filename, checksum)
170-
}),
171-
tar.extract({
172-
strip: 1,
173-
cwd: devDir,
174-
filter: isValid,
175-
onwarn
176-
})
177-
)
178-
} catch (err) {
179-
// something went wrong downloading the tarball?
180-
if (err.code === 'ENOTFOUND') {
181-
throw new Error('This is most likely not a problem with node-gyp or the package itself and\n' +
182-
'is related to network connectivity. In most cases you are behind a proxy or have bad \n' +
183-
'network settings.')
161+
try {
162+
if (tarPath) {
163+
await tar.extract({
164+
file: tarPath,
165+
strip: 1,
166+
filter: isValid,
167+
onwarn,
168+
cwd: tarExtractDir
169+
})
170+
} else {
171+
try {
172+
const res = await download(gyp, release.tarballUrl)
173+
174+
if (res.status !== 200) {
175+
throw new Error(`${res.status} response downloading ${release.tarballUrl}`)
176+
}
177+
178+
await streamPipeline(
179+
res.body,
180+
// content checksum
181+
new ShaSum((_, checksum) => {
182+
const filename = path.basename(release.tarballUrl).trim()
183+
contentShasums[filename] = checksum
184+
log.verbose('content checksum', filename, checksum)
185+
}),
186+
tar.extract({
187+
strip: 1,
188+
cwd: tarExtractDir,
189+
filter: isValid,
190+
onwarn
191+
})
192+
)
193+
} catch (err) {
194+
// something went wrong downloading the tarball?
195+
if (err.code === 'ENOTFOUND') {
196+
throw new Error('This is most likely not a problem with node-gyp or the package itself and\n' +
197+
'is related to network connectivity. In most cases you are behind a proxy or have bad \n' +
198+
'network settings.')
199+
}
200+
throw err
184201
}
185-
throw err
186202
}
187-
}
188203

189-
// invoked after the tarball has finished being extracted
190-
if (extractErrors || extractCount === 0) {
191-
throw new Error('There was a fatal problem while downloading/extracting the tarball')
192-
}
204+
// invoked after the tarball has finished being extracted
205+
if (extractErrors || extractCount === 0) {
206+
throw new Error('There was a fatal problem while downloading/extracting the tarball')
207+
}
193208

194-
log.verbose('tarball', 'done parsing tarball')
195-
196-
const installVersionPath = path.resolve(devDir, 'installVersion')
197-
await Promise.all([
198-
// need to download node.lib
199-
...(win ? downloadNodeLib() : []),
200-
// write the "installVersion" file
201-
fs.promises.writeFile(installVersionPath, gyp.package.installVersion + '\n'),
202-
// Only download SHASUMS.txt if we downloaded something in need of SHA verification
203-
...(!tarPath || win ? [downloadShasums()] : [])
204-
])
205-
206-
log.verbose('download contents checksum', JSON.stringify(contentShasums))
207-
// check content shasums
208-
for (const k in contentShasums) {
209-
log.verbose('validating download checksum for ' + k, '(%s == %s)', contentShasums[k], expectShasums[k])
210-
if (contentShasums[k] !== expectShasums[k]) {
211-
throw new Error(k + ' local checksum ' + contentShasums[k] + ' not match remote ' + expectShasums[k])
209+
log.verbose('tarball', 'done parsing tarball')
210+
211+
const installVersionPath = path.resolve(tarExtractDir, 'installVersion')
212+
await Promise.all([
213+
// need to download node.lib
214+
...(win ? downloadNodeLib() : []),
215+
// write the "installVersion" file
216+
fs.promises.writeFile(installVersionPath, gyp.package.installVersion + '\n'),
217+
// Only download SHASUMS.txt if we downloaded something in need of SHA verification
218+
...(!tarPath || win ? [downloadShasums()] : [])
219+
])
220+
221+
log.verbose('download contents checksum', JSON.stringify(contentShasums))
222+
// check content shasums
223+
for (const k in contentShasums) {
224+
log.verbose('validating download checksum for ' + k, '(%s == %s)', contentShasums[k], expectShasums[k])
225+
if (contentShasums[k] !== expectShasums[k]) {
226+
throw new Error(k + ' local checksum ' + contentShasums[k] + ' not match remote ' + expectShasums[k])
227+
}
212228
}
229+
230+
// swap in the temp tarball extract directory for devDir;
231+
// with parallel installs this may cause file errors on Windows
232+
// so use an exponential backoff to resolve collisions.
233+
// while graceful-fs has built-in retry logic for rename on
234+
// Windows, it's not exponential backoff and it doesn't handle
235+
// the case where the directory being renamed is missing, which
236+
// needs to be handled here. uses fs.renameSync rather than
237+
// fs.promises.rename as the latter seems to have a bug and would
238+
// let multiple parallel invocations think they "won" the race
239+
if (tarExtractDir !== devDir) {
240+
// per comments in graceful-fs, retry rename for up to 60 seconds
241+
// on Windows due to A/V software locking files for that long
242+
const options = win ? {
243+
numOfAttempts: Infinity,
244+
retry: () => Date.now() - start < 60000
245+
} : undefined
246+
let start
247+
try {
248+
start = Date.now()
249+
await backOff(() => fs.renameSync(devDir, `${tarExtractDir}-old`), options)
250+
start = Date.now()
251+
await backOff(() => fs.renameSync(tarExtractDir, devDir), options)
252+
} catch (err) {
253+
log.error('error while swapping temp tarball extract directory', err)
254+
throw new Error('There was a fatal problem while downloading/extracting the tarball')
255+
}
256+
try {
257+
await util.promisify(rm)(`${tarExtractDir}-old`)
258+
} catch {
259+
log.warn('failed to clean up old version directory')
260+
}
261+
}
262+
} catch (err) {
263+
if (tarExtractDir !== devDir) {
264+
// try to cleanup temp dir
265+
try {
266+
await util.promisify(rm)(tarExtractDir)
267+
} catch {
268+
log.warn('failed to clean up temp tarball extract directory')
269+
}
270+
}
271+
throw err
213272
}
214273

215274
async function downloadShasums () {
@@ -240,7 +299,7 @@ async function install (fs, gyp, argv) {
240299
log.verbose('on Windows; need to download `' + release.name + '.lib`...')
241300
const archs = ['ia32', 'x64', 'arm64']
242301
return archs.map(async (arch) => {
243-
const dir = path.resolve(devDir, arch)
302+
const dir = path.resolve(tarExtractDir, arch)
244303
const targetLibPath = path.resolve(dir, release.name + '.lib')
245304
const { libUrl, libPath } = release[arch]
246305
const name = `${arch} ${release.name}.lib`

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"main": "./lib/node-gyp.js",
2424
"dependencies": {
2525
"env-paths": "^2.2.0",
26+
"exponential-backoff": "^3.1.1",
2627
"glob": "^7.1.4",
2728
"graceful-fs": "^4.2.6",
2829
"make-fetch-happen": "^10.0.3",

0 commit comments

Comments
 (0)