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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Fix issue with interactive unzip on Linux #807

Merged
merged 16 commits into from May 21, 2021
Merged

Bugfix: Fix issue with interactive unzip on Linux #807

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0911ed2
Add new powershell commands for windows unzip
luketomlinson May 12, 2021
ce08409
Test fails to overwrite file
luketomlinson May 12, 2021
008211e
Add new windows commands for unzip
luketomlinson May 12, 2021
495c529
Add Test for failing case for both pwsh and powershell
luketomlinson May 13, 2021
b31bbe5
Modify test to confirm overwrite behavior for xar
luketomlinson May 14, 2021
8684339
Delete ._test.txt
luketomlinson May 14, 2021
62eb9f8
Add fallback case for older windows systems
luketomlinson May 18, 2021
72970bf
Remove try
luketomlinson May 18, 2021
e5db96a
Run Tests on windows-2016
luketomlinson May 19, 2021
0195ec2
Update tar tests to handle existing files
luketomlinson May 19, 2021
533ca19
Lint
luketomlinson May 19, 2021
a00d1ad
Update tool-cache.test.ts
luketomlinson May 19, 2021
3b56477
Update tool-cache.test.ts
luketomlinson May 19, 2021
58ac2ac
Update tool-cache.test.ts
luketomlinson May 19, 2021
b88c5cf
Update tool-cache.test.ts
luketomlinson May 19, 2021
aebc03c
Update from PR feedback
luketomlinson May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
175 changes: 124 additions & 51 deletions packages/tool-cache/__tests__/tool-cache.test.ts
View file
Open in desktop
Expand Up @@ -243,6 +243,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 @@ -251,6 +255,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 @@ -347,6 +354,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 @@ -360,14 +383,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 @@ -462,14 +492,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 @@ -500,6 +539,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 @@ -520,8 +562,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 @@ -534,58 +582,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 @@ -597,7 +657,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 @@ -629,12 +689,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 @@ -843,3 +907,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(';')
}
65 changes: 50 additions & 15 deletions packages/tool-cache/src/tool-cache.ts
View file
Open in desktop
Expand Up @@ -344,21 +344,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)
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved

//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 { } ;`,
ericsciple marked this conversation as resolved.
Show resolved Hide resolved
`try { [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`,
`catch { if ($_.Exception.GetType().FullName -eq 'System.Management.Automation.MethodException'){ Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force } else { throw $_ } } ;`
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
].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 { try { Add-Type -AssemblyName System.IO.Compression.ZipFile } catch { } ; [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
].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 +401,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
ericsciple marked this conversation as resolved.
Show resolved Hide resolved
await exec(`"${unzipPath}"`, args, {cwd: dest})
}

Expand Down