Skip to content

Commit

Permalink
Bugfix: Fix issue with interactive unzip on Linux (#807)
Browse files Browse the repository at this point in the history 
* Add new powershell commands for windows unzip

* Test fails to overwrite file

* Add new windows commands for unzip

* Add Test for failing case for both pwsh and powershell

* Modify test to confirm overwrite behavior for xar

* Delete ._test.txt

* Add fallback case for older windows systems

* Remove try

* Run Tests on windows-2016

* Update tar tests to handle existing files

* Lint

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update from PR feedback
  • Loading branch information
@luketomlinson
luketomlinson committed May 21, 2021
1 parent 6e33c78 commit a1b068e
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 66 deletions.
175 changes: 124 additions & 51 deletions packages/tool-cache/__tests__/tool-cache.test.ts
View file
Expand Up @@ -239,6 +239,10 @@ describe('@actions/tool-cache', function() {
const _7zFile: string = path.join(tempDir, 'test.7z')
await io.cp(path.join(__dirname, 'data', 'test.7z'), _7zFile)

const destDir = path.join(tempDir, 'destination')
await io.mkdirP(destDir)
fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')

// extract/cache
const extPath: string = await tc.extract7z(_7zFile)
await tc.cacheDir(extPath, 'my-7z-contents', '1.1.0')
Expand All @@ -247,6 +251,9 @@ describe('@actions/tool-cache', function() {
expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
'file.txt contents'
)
expect(
fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
).toBeTruthy()
Expand Down Expand Up @@ -343,6 +350,22 @@ describe('@actions/tool-cache', function() {
await io.rmRF(tempDir)
}
})
it.each(['pwsh', 'powershell'])(
'unzip properly fails with bad path (%s)',
async powershellTool => {
const originalPath = process.env['PATH']
try {
if (powershellTool === 'powershell' && IS_WINDOWS) {
//remove pwsh from PATH temporarily to test fallback case
process.env['PATH'] = removePWSHFromPath(process.env['PATH'])
}

await expect(tc.extractZip('badPath')).rejects.toThrow()
} finally {
process.env['PATH'] = originalPath
}
}
)
} else if (IS_MAC) {
it('extract .xar', async () => {
const tempDir = path.join(tempPath, 'test-install.xar')
Expand All @@ -356,14 +379,21 @@ describe('@actions/tool-cache', function() {
cwd: sourcePath
})

const destDir = path.join(tempDir, 'destination')
await io.mkdirP(destDir)
fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')

// extract/cache
const extPath: string = await tc.extractXar(targetPath, undefined, '-x')
const extPath: string = await tc.extractXar(targetPath, destDir, ['-x'])
await tc.cacheDir(extPath, 'my-xar-contents', '1.1.0')
const toolPath: string = tc.find('my-xar-contents', '1.1.0')

expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
'file.txt contents'
)
expect(
fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
).toBeTruthy()
Expand Down Expand Up @@ -458,14 +488,23 @@ describe('@actions/tool-cache', function() {
const _tgzFile: string = path.join(tempDir, 'test.tar.gz')
await io.cp(path.join(__dirname, 'data', 'test.tar.gz'), _tgzFile)

//Create file to overwrite
const destDir = path.join(tempDir, 'extract-dest')
await io.rmRF(destDir)
await io.mkdirP(destDir)
fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')

// extract/cache
const extPath: string = await tc.extractTar(_tgzFile)
const extPath: string = await tc.extractTar(_tgzFile, destDir)
await tc.cacheDir(extPath, 'my-tgz-contents', '1.1.0')
const toolPath: string = tc.find('my-tgz-contents', '1.1.0')

expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
'file.txt contents'
)
expect(
fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
).toBeTruthy()
Expand Down Expand Up @@ -496,6 +535,9 @@ describe('@actions/tool-cache', function() {
expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
'file.txt contents'
)
expect(
fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
).toBeTruthy()
Expand All @@ -516,8 +558,14 @@ describe('@actions/tool-cache', function() {
const _txzFile: string = path.join(tempDir, 'test.tar.xz')
await io.cp(path.join(__dirname, 'data', 'test.tar.xz'), _txzFile)

//Create file to overwrite
const destDir = path.join(tempDir, 'extract-dest')
await io.rmRF(destDir)
await io.mkdirP(destDir)
fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')

// extract/cache
const extPath: string = await tc.extractTar(_txzFile, undefined, 'x')
const extPath: string = await tc.extractTar(_txzFile, destDir, 'x')
await tc.cacheDir(extPath, 'my-txz-contents', '1.1.0')
const toolPath: string = tc.find('my-txz-contents', '1.1.0')

Expand All @@ -530,58 +578,70 @@ describe('@actions/tool-cache', function() {
).toBe('foo/hello: world')
})

it('installs a zip and finds it', async () => {
const tempDir = path.join(__dirname, 'test-install-zip')
try {
await io.mkdirP(tempDir)
it.each(['pwsh', 'powershell'])(
'installs a zip and finds it (%s)',
async powershellTool => {
const tempDir = path.join(__dirname, 'test-install-zip')
const originalPath = process.env['PATH']
try {
await io.mkdirP(tempDir)

// stage the layout for a zip file:
// file.txt
// folder/nested-file.txt
const stagingDir = path.join(tempDir, 'zip-staging')
await io.mkdirP(path.join(stagingDir, 'folder'))
fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')
// stage the layout for a zip file:
// file.txt
// folder/nested-file.txt
const stagingDir = path.join(tempDir, 'zip-staging')
await io.mkdirP(path.join(stagingDir, 'folder'))
fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')

// create the zip
const zipFile = path.join(tempDir, 'test.zip')
await io.rmRF(zipFile)
if (IS_WINDOWS) {
const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes
const escapedZipFile = zipFile.replace(/'/g, "''")
const powershellPath =
(await io.which('pwsh', false)) ||
(await io.which('powershell', true))
const args = [
'-NoLogo',
'-Sta',
'-NoProfile',
'-NonInteractive',
'-ExecutionPolicy',
'Unrestricted',
'-Command',
`$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')`
]
await exec.exec(`"${powershellPath}"`, args)
} else {
const zipPath: string = await io.which('zip', true)
await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], {
cwd: stagingDir
})
}

// create the zip
const zipFile = path.join(tempDir, 'test.zip')
await io.rmRF(zipFile)
if (IS_WINDOWS) {
const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes
const escapedZipFile = zipFile.replace(/'/g, "''")
const powershellPath =
(await io.which('pwsh', false)) ||
(await io.which('powershell', true))
const args = [
'-NoLogo',
'-Sta',
'-NoProfile',
'-NonInteractive',
'-ExecutionPolicy',
'Unrestricted',
'-Command',
`$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')`
]
await exec.exec(`"${powershellPath}"`, args)
} else {
const zipPath: string = await io.which('zip', true)
await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], {cwd: stagingDir})
}
if (powershellTool === 'powershell' && IS_WINDOWS) {
//remove pwsh from PATH temporarily to test fallback case
process.env['PATH'] = removePWSHFromPath(process.env['PATH'])
}

const extPath: string = await tc.extractZip(zipFile)
await tc.cacheDir(extPath, 'foo', '1.1.0')
const toolPath: string = tc.find('foo', '1.1.0')
const extPath: string = await tc.extractZip(zipFile)
await tc.cacheDir(extPath, 'foo', '1.1.0')
const toolPath: string = tc.find('foo', '1.1.0')

expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(
fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
).toBeTruthy()
} finally {
await io.rmRF(tempDir)
expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(
fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
).toBeTruthy()
} finally {
await io.rmRF(tempDir)
process.env['PATH'] = originalPath
}
}
})
)

it('installs a zip and extracts it to specified directory', async function() {
const tempDir = path.join(__dirname, 'test-install-zip')
Expand All @@ -593,7 +653,7 @@ describe('@actions/tool-cache', function() {
// folder/nested-file.txt
const stagingDir = path.join(tempDir, 'zip-staging')
await io.mkdirP(path.join(stagingDir, 'folder'))
fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
fs.writeFileSync(path.join(stagingDir, 'file.txt'), 'originalText')
fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')

// create the zip
Expand Down Expand Up @@ -625,12 +685,16 @@ describe('@actions/tool-cache', function() {
await io.rmRF(destDir)
await io.mkdirP(destDir)
try {
fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
const extPath: string = await tc.extractZip(zipFile, destDir)
await tc.cacheDir(extPath, 'foo', '1.1.0')
const toolPath: string = tc.find('foo', '1.1.0')
expect(fs.existsSync(toolPath)).toBeTruthy()
expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
'originalText'
)
expect(
fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
).toBeTruthy()
Expand Down Expand Up @@ -839,3 +903,12 @@ function setGlobal<T>(key: string, value: T | undefined): void {
g[key] = value
}
}

function removePWSHFromPath(pathEnv: string | undefined): string {
return (pathEnv || '')
.split(';')
.filter(segment => {
return !segment.startsWith(`C:\\Program Files\\PowerShell`)
})
.join(';')
}
66 changes: 51 additions & 15 deletions packages/tool-cache/src/tool-cache.ts
View file
Expand Up @@ -272,6 +272,7 @@ export async function extractTar(
if (isGnuTar) {
// Suppress warnings when using GNU tar to extract archives created by BSD tar
args.push('--warning=no-unknown-keyword')
args.push('--overwrite')
}

args.push('-C', destArg, '-f', fileArg)
Expand Down Expand Up @@ -344,21 +345,55 @@ async function extractZipWin(file: string, dest: string): Promise<void> {
// build the powershell command
const escapedFile = file.replace(/'/g, "''").replace(/"|\n|\r/g, '') // double-up single quotes, remove double quotes and newlines
const escapedDest = dest.replace(/'/g, "''").replace(/"|\n|\r/g, '')
const command = `$ErrorActionPreference = 'Stop' ; try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ; [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}')`

// run powershell
const powershellPath = await io.which('powershell', true)
const args = [
'-NoLogo',
'-Sta',
'-NoProfile',
'-NonInteractive',
'-ExecutionPolicy',
'Unrestricted',
'-Command',
command
]
await exec(`"${powershellPath}"`, args)
const pwshPath = await io.which('pwsh', false)

//To match the file overwrite behavior on nix systems, we use the overwrite = true flag for ExtractToDirectory
//and the -Force flag for Expand-Archive as a fallback
if (pwshPath) {
//attempt to use pwsh with ExtractToDirectory, if this fails attempt Expand-Archive
const pwshCommand = [
`$ErrorActionPreference = 'Stop' ;`,
`try { Add-Type -AssemblyName System.IO.Compression.ZipFile } catch { } ;`,
`try { [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`,
`catch { if (($_.Exception.GetType().FullName -eq 'System.Management.Automation.MethodException') -or ($_.Exception.GetType().FullName -eq 'System.Management.Automation.RuntimeException') ){ Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force } else { throw $_ } } ;`
].join(' ')

const args = [
'-NoLogo',
'-NoProfile',
'-NonInteractive',
'-ExecutionPolicy',
'Unrestricted',
'-Command',
pwshCommand
]

core.debug(`Using pwsh at path: ${pwshPath}`)
await exec(`"${pwshPath}"`, args)
} else {
const powershellCommand = [
`$ErrorActionPreference = 'Stop' ;`,
`try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ;`,
`if ((Get-Command -Name Expand-Archive -Module Microsoft.PowerShell.Archive -ErrorAction Ignore)) { Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force }`,
`else {[System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`
].join(' ')

const args = [
'-NoLogo',
'-Sta',
'-NoProfile',
'-NonInteractive',
'-ExecutionPolicy',
'Unrestricted',
'-Command',
powershellCommand
]

const powershellPath = await io.which('powershell', true)
core.debug(`Using powershell at path: ${powershellPath}`)

await exec(`"${powershellPath}"`, args)
}
}

async function extractZipNix(file: string, dest: string): Promise<void> {
Expand All @@ -367,6 +402,7 @@ async function extractZipNix(file: string, dest: string): Promise<void> {
if (!core.isDebug()) {
args.unshift('-q')
}
args.unshift('-o') //overwrite with -o, otherwise a prompt is shown which freezes the run
await exec(`"${unzipPath}"`, args, {cwd: dest})
}

Expand Down

0 comments on commit a1b068e

Please sign in to comment.