From 2f73afa8435b06b211571703a2cc28c4e3750e92 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 07:39:15 +0000 Subject: [PATCH] Separate args --- packages/cache/__tests__/tar.test.ts | 25 ++-- packages/cache/src/internal/tar.ts | 204 ++++++++++++++++++++------- 2 files changed, 166 insertions(+), 63 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 33781214..4b28c7cb 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -59,13 +59,13 @@ test('zstd extract tar', async () => { expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', '-xf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', '-C', - IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace + IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, + '--use-compress-program', + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -89,12 +89,12 @@ test('gzip extract tar', async () => { expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, [ - '-z', '-xf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', '-C', - IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace + IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, + '-z' ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -180,11 +180,15 @@ test('zstd create tar with windows BSDtar', async () => { const archiveFolder = getTempDir() const workspace = process.env['GITHUB_WORKSPACE'] const sourceDirectories = ['~/.npm/cache', `${workspace}/dist`] - const tarFilename = "cache.tar" + const tarFilename = 'cache.tar' await fs.promises.mkdir(archiveFolder, {recursive: true}) - await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) + await tar.createTar( + archiveFolder, + sourceDirectories, + CompressionMethod.Zstd + ) const tarPath = SystemTarPathOnWindows @@ -203,9 +207,10 @@ test('zstd create tar with windows BSDtar', async () => { workspace?.replace(/\\/g, '/'), '--files-from', 'manifest.txt', - "&&", - "zstd -T0 --long=30 -o", - CacheFilename.Zstd.replace(/\\/g, '/') + '&&', + 'zstd -T0 --long=30 -o', + CacheFilename.Zstd.replace(/\\/g, '/'), + tarFilename.replace(/\\/g, '/') ], { cwd: archiveFolder diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 964f6437..8647fdc0 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -8,16 +8,13 @@ import {CompressionMethod, SystemTarPathOnWindows} from './constants' const IS_WINDOWS = process.platform === 'win32' // Function also mutates the args array. For non-mutation call with passing an empty array. -async function getTarPath(args: string[]): Promise { +async function getTarPath(): Promise { switch (process.platform) { case 'win32': { const gnuTar = await utils.getGnuTarPathOnWindows() - const systemTar = `${process.env['windir']}\\System32\\tar.exe` + const systemTar = SystemTarPathOnWindows if (gnuTar) { // Use GNUtar as default on windows - if (args.length > 0) { - args.push('--force-local') - } return gnuTar } else if (existsSync(systemTar)) { return systemTar @@ -28,9 +25,6 @@ async function getTarPath(args: string[]): Promise { const gnuTar = await io.which('gtar', false) if (gnuTar) { // fix permission denied errors when extracting BSD tar archive with GNU tar - https://github.com/actions/cache/issues/527 - if (args.length > 0) { - args.push('--delay-directory-restore') - } return gnuTar } break @@ -41,9 +35,100 @@ async function getTarPath(args: string[]): Promise { return await io.which('tar', true) } +async function getTarArgs( + compressionMethod: CompressionMethod, + type: string, + archivePath = '' +): Promise { + const args = [] + const manifestFilename = 'manifest.txt' + const cacheFileName = utils.getCacheFileName(compressionMethod) + const tarFile = 'cache.tar' + const tarPath = await getTarPath() + const workingDirectory = getWorkingDirectory() + const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + + // Method specific args + switch (type) { + case 'create': + args.push( + '--posix', + '-cf', + BSD_TAR_WINDOWS + ? tarFile + : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '--exclude', + BSD_TAR_WINDOWS + ? tarFile + : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P', + '-C', + workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '--files-from', + manifestFilename + ) + break + case 'extract': + args.push( + '-xf', + BSD_TAR_WINDOWS + ? tarFile + : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P', + '-C', + workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ) + break + // TODO: Correct the below code especially archivePath for BSD_TAR_WINDOWS + case 'list': + args.push( + '-tf', + BSD_TAR_WINDOWS + ? tarFile + : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P' + ) + break + } + + // Platform specific args + switch (process.platform) { + case 'win32': { + const gnuTar = await utils.getGnuTarPathOnWindows() + if (gnuTar) { + // Use GNUtar as default on windows + args.push('--force-local') + } + break + } + case 'darwin': { + const gnuTar = await io.which('gtar', false) + if (gnuTar) { + // fix permission denied errors when extracting BSD tar archive with GNU tar - https://github.com/actions/cache/issues/527 + args.push('--delay-directory-restore') + } + break + } + } + + return args +} + async function execTar(args: string[], cwd?: string): Promise { try { - await exec(`"${await getTarPath(args)}"`, args, {cwd}) + await exec(`"${await getTarPath()}"`, args, {cwd}) + } catch (error) { + throw new Error(`Tar failed with error: ${error?.message}`) + } +} + +async function execCommand( + command: string, + args: string[], + cwd?: string +): Promise { + try { + await exec(command, args, {cwd}) } catch (error) { throw new Error(`Tar failed with error: ${error?.message}`) } @@ -61,12 +146,19 @@ async function getCompressionProgram( // unzstd is equivalent to 'zstd -d' // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit. // Using 30 here because we also support 32-bit self-hosted runners. - const tarPath = await getTarPath([]) + const tarPath = await getTarPath() + const cacheFileName = utils.getCacheFileName(compressionMethod) + const tarFile = 'cache.tar' const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { case CompressionMethod.Zstd: if (BSD_TAR_WINDOWS) { - return ['-a'] // auto-detect compression + return [ + 'zstd -d --long=30 -o', + tarFile, + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&' + ] } return [ '--use-compress-program', @@ -74,7 +166,12 @@ async function getCompressionProgram( ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_WINDOWS) { - return ['-a'] // auto-detect compression + return [ + 'zstd -d -o', + tarFile, + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&' + ] } return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: @@ -86,13 +183,19 @@ export async function listTar( archivePath: string, compressionMethod: CompressionMethod ): Promise { - const args = [ - ...(await getCompressionProgram(compressionMethod)), - '-tf', - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P' - ] - await execTar(args) + const tarPath = await getTarPath() + const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const compressionArgs = await getCompressionProgram(compressionMethod) + const tarArgs = await getTarArgs(compressionMethod, 'list', archivePath) + // TODO: Add a test for BSD tar on windows + if (BSD_TAR_WINDOWS) { + const command = compressionArgs[0] + const args = compressionArgs.slice(1).concat(tarArgs) + await execCommand(command, args) + } else { + const args = tarArgs.concat(compressionArgs) + await execTar(args) + } } export async function extractTar( @@ -101,16 +204,19 @@ export async function extractTar( ): Promise { // Create directory to extract tar into const workingDirectory = getWorkingDirectory() + const tarPath = await getTarPath() + const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows await io.mkdirP(workingDirectory) - const args = [ - ...(await getCompressionProgram(compressionMethod)), - '-xf', - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '-C', - workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') - ] - await execTar(args) + const tarArgs = await getTarArgs(compressionMethod, 'extract', archivePath) + const compressionArgs = await getCompressionProgram(compressionMethod) + if (BSD_TAR_WINDOWS) { + const command = compressionArgs[0] + const args = compressionArgs.slice(1).concat(tarArgs) + await execCommand(command, args) + } else { + const args = tarArgs.concat(compressionArgs) + await execTar(args) + } } export async function createTar( @@ -122,7 +228,7 @@ export async function createTar( const manifestFilename = 'manifest.txt' const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' - const tarPath = await getTarPath([]) + const tarPath = await getTarPath() const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows writeFileSync( path.join(archiveFolder, manifestFilename), @@ -135,11 +241,16 @@ export async function createTar( // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit. // Using 30 here because we also support 32-bit self-hosted runners. // Long range mode is added to zstd in v1.3.2 release, so we will not use --long in older version of zstd. - async function getCompressionProgram(): Promise { + function getCompressionProgram(): string[] { switch (compressionMethod) { case CompressionMethod.Zstd: if (BSD_TAR_WINDOWS) { - return ['&&', 'zstd -T0 --long=30 -o'] + return [ + '&&', + 'zstd -T0 --long=30 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] } return [ '--use-compress-program', @@ -147,33 +258,20 @@ export async function createTar( ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_WINDOWS) { - return ['&&', 'zstd -T0 -o'] + return [ + '&&', + 'zstd -T0 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] } return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] default: return ['-z'] } } - const args = [ - '--posix', - '-cf', - BSD_TAR_WINDOWS - ? tarFile - : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '--exclude', - BSD_TAR_WINDOWS - ? tarFile - : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '-C', - workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '--files-from', - manifestFilename, - ...(await getCompressionProgram()) - ].concat( - BSD_TAR_WINDOWS - ? [cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/')] - : [] - ) + const tarArgs = await getTarArgs(compressionMethod, 'create') + const compressionArgs = getCompressionProgram() + const args = tarArgs.concat(compressionArgs) await execTar(args, archiveFolder) }