From 6349c3ca3a2339f6b08ee5f31a313e48ea224327 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Wed, 16 Nov 2022 20:18:31 +0000 Subject: [PATCH 01/24] bsd + zstd fallback implementation --- packages/cache/src/internal/constants.ts | 2 + packages/cache/src/internal/tar.ts | 51 +++++++++++++++++++----- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index e61b46f4..0acea2ef 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -24,3 +24,5 @@ export const SocketTimeout = 5000 // The default path of GNUtar on hosted Windows runners export const GnuTarPathOnWindows = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + +export const SystemTarPathOnWindows = `${process.env['SYSTEMDRIVE']}\\Windows\\System32\\tar.exe` \ No newline at end of file diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 42692008..22dd05bf 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -3,10 +3,11 @@ import * as io from '@actions/io' import {existsSync, writeFileSync} from 'fs' import * as path from 'path' import * as utils from './cacheUtils' -import {CompressionMethod} from './constants' +import {CompressionMethod, GnuTarPathOnWindows, 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 { switch (process.platform) { case 'win32': { @@ -14,7 +15,9 @@ async function getTarPath(args: string[]): Promise { const systemTar = `${process.env['windir']}\\System32\\tar.exe` if (gnuTar) { // Use GNUtar as default on windows - args.push('--force-local') + if (args.length > 0) { + args.push('--force-local') + } return gnuTar } else if (existsSync(systemTar)) { return systemTar @@ -25,7 +28,9 @@ 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 - args.push('--delay-directory-restore') + if (args.length > 0) { + args.push('--delay-directory-restore') + } return gnuTar } break @@ -49,18 +54,30 @@ function getWorkingDirectory(): string { } // Common function for extractTar and listTar to get the compression method -function getCompressionProgram(compressionMethod: CompressionMethod): string[] { +async function getCompressionProgram(compressionMethod: CompressionMethod): string[] { // -d: Decompress. // 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 BSD_TAR_ZSTD = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { case CompressionMethod.Zstd: + if (BSD_TAR_ZSTD) { + return [ + '-O', '|','zstd -d --long=30 -o' + ] + } return [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ] case CompressionMethod.ZstdWithoutLong: + if (BSD_TAR_ZSTD) { + return [ + '-O', '|', 'zstd -d --long=30 -o' + ] + } return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: return ['-z'] @@ -72,8 +89,8 @@ export async function listTar( compressionMethod: CompressionMethod ): Promise { const args = [ - ...getCompressionProgram(compressionMethod), '-tf', + ...getCompressionProgram(compressionMethod), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' ] @@ -88,8 +105,8 @@ export async function extractTar( const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) const args = [ - ...getCompressionProgram(compressionMethod), '-xf', + ...getCompressionProgram(compressionMethod), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', @@ -117,14 +134,26 @@ 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. - function getCompressionProgram(): string[] { + async function getCompressionProgram(): string[] { + const tarPath = await getTarPath([]) + const BSD_TAR_ZSTD = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { case CompressionMethod.Zstd: + if (BSD_TAR_ZSTD) { + return [ + '-O', '|', 'zstd -T0 --long=30 -o' + ] + } return [ '--use-compress-program', IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' ] case CompressionMethod.ZstdWithoutLong: + if (BSD_TAR_ZSTD) { + return [ + '-O', '|', 'zstd -T0 -o' + ] + } return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] default: return ['-z'] @@ -132,16 +161,16 @@ export async function createTar( } const args = [ '--posix', - ...getCompressionProgram(), - '-cf', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--exclude', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', - manifestFilename + manifestFilename, + ...getCompressionProgram(), + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + ] await execTar(args, archiveFolder) } From db3517fe3b63c59c4ab7857d6aa7658b15b99eaa Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Wed, 16 Nov 2022 20:21:40 +0000 Subject: [PATCH 02/24] bsd + zstd fallback implementation --- packages/cache/src/internal/tar.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 22dd05bf..f077dc52 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -54,7 +54,7 @@ function getWorkingDirectory(): string { } // Common function for extractTar and listTar to get the compression method -async function getCompressionProgram(compressionMethod: CompressionMethod): string[] { +async function getCompressionProgram(compressionMethod: CompressionMethod): Promise { // -d: Decompress. // 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. @@ -90,7 +90,7 @@ export async function listTar( ): Promise { const args = [ '-tf', - ...getCompressionProgram(compressionMethod), + ...(await getCompressionProgram(compressionMethod)), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' ] @@ -106,7 +106,7 @@ export async function extractTar( await io.mkdirP(workingDirectory) const args = [ '-xf', - ...getCompressionProgram(compressionMethod), + ...(await getCompressionProgram(compressionMethod)), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', @@ -134,7 +134,7 @@ 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(): string[] { + async function getCompressionProgram(): Promise { const tarPath = await getTarPath([]) const BSD_TAR_ZSTD = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { @@ -168,7 +168,7 @@ export async function createTar( workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', manifestFilename, - ...getCompressionProgram(), + ...(await getCompressionProgram()), cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), ] From 27bf8304ea3aef2975760926d9742b6d1eb1bf56 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Wed, 16 Nov 2022 20:50:04 +0000 Subject: [PATCH 03/24] Fix tar operations --- packages/cache/src/internal/tar.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index f077dc52..1a7f38fc 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -64,9 +64,7 @@ async function getCompressionProgram(compressionMethod: CompressionMethod): Prom switch (compressionMethod) { case CompressionMethod.Zstd: if (BSD_TAR_ZSTD) { - return [ - '-O', '|','zstd -d --long=30 -o' - ] + return ['-a'] // auto-detect compression } return [ '--use-compress-program', @@ -74,9 +72,7 @@ async function getCompressionProgram(compressionMethod: CompressionMethod): Prom ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { - return [ - '-O', '|', 'zstd -d --long=30 -o' - ] + return ['a'] // auto-detect compression } return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: @@ -168,6 +164,7 @@ export async function createTar( workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', manifestFilename, + '-cf', ...(await getCompressionProgram()), cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), From e9e146bbf8faa49d1d78bc774d5c65aabb71bdab Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Wed, 16 Nov 2022 21:04:25 +0000 Subject: [PATCH 04/24] Add -v option for testing --- packages/cache/src/internal/tar.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 1a7f38fc..a1785039 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -165,6 +165,7 @@ export async function createTar( '--files-from', manifestFilename, '-cf', + '-v', ...(await getCompressionProgram()), cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), From 5f89653f1b9e6c52c7afbabb923cd08fb6a8210e Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Thu, 17 Nov 2022 06:22:13 +0000 Subject: [PATCH 05/24] Fix order of args for tar --- packages/cache/src/internal/tar.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index a1785039..2cabe3b4 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -68,13 +68,14 @@ async function getCompressionProgram(compressionMethod: CompressionMethod): Prom } return [ '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', + '-tf' ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { return ['a'] // auto-detect compression } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] + return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd', '-tf'] default: return ['-z'] } @@ -85,7 +86,6 @@ export async function listTar( compressionMethod: CompressionMethod ): Promise { const args = [ - '-tf', ...(await getCompressionProgram(compressionMethod)), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' @@ -142,7 +142,8 @@ export async function createTar( } return [ '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30', + '-cf' ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { @@ -150,7 +151,7 @@ export async function createTar( '-O', '|', 'zstd -T0 -o' ] } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] + return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt', '-cf'] default: return ['-z'] } @@ -164,8 +165,6 @@ export async function createTar( workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', manifestFilename, - '-cf', - '-v', ...(await getCompressionProgram()), cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), From ea9856079f362753e8dc557c7ff97d2991a6a2a0 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Thu, 17 Nov 2022 08:53:46 +0000 Subject: [PATCH 06/24] Fix tar tests --- packages/cache/__tests__/tar.test.ts | 19 ++++++++++--------- packages/cache/src/internal/tar.ts | 8 ++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 31f9bd5e..ec214181 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -149,17 +149,17 @@ test('zstd create tar', async () => { `"${tarPath}"`, [ '--posix', - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30', - '-cf', - IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, '--exclude', IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, '-P', '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt' + 'manifest.txt', + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30', + '-cf', + IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -187,16 +187,16 @@ test('gzip create tar', async () => { `"${tarPath}"`, [ '--posix', - '-z', - '-cf', - IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, '--exclude', IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, '-P', '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt' + 'manifest.txt', + '-z', + '-cf', + IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -281,3 +281,4 @@ test('gzip list tar', async () => { {cwd: undefined} ) }) + diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 2cabe3b4..450cde3b 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -69,13 +69,12 @@ async function getCompressionProgram(compressionMethod: CompressionMethod): Prom return [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', - '-tf' ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { return ['a'] // auto-detect compression } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd', '-tf'] + return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: return ['-z'] } @@ -87,6 +86,7 @@ export async function listTar( ): Promise { const args = [ ...(await getCompressionProgram(compressionMethod)), + '-tf', archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' ] @@ -101,8 +101,8 @@ export async function extractTar( const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) const args = [ - '-xf', ...(await getCompressionProgram(compressionMethod)), + '-xf', archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', @@ -153,7 +153,7 @@ export async function createTar( } return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt', '-cf'] default: - return ['-z'] + return ['-z', '-cf'] } } const args = [ From 4fa5b7d133e7c42c32c09fd1b007c49f2a41ef72 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Thu, 17 Nov 2022 09:12:53 +0000 Subject: [PATCH 07/24] Fix lint issues --- packages/cache/__tests__/tar.test.ts | 3 +-- packages/cache/src/internal/constants.ts | 2 +- packages/cache/src/internal/tar.ts | 27 ++++++++++++------------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index ec214181..abf8eecb 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -196,7 +196,7 @@ test('gzip create tar', async () => { 'manifest.txt', '-z', '-cf', - IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, + IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -281,4 +281,3 @@ test('gzip list tar', async () => { {cwd: undefined} ) }) - diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index 0acea2ef..c6d8a490 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -25,4 +25,4 @@ export const SocketTimeout = 5000 // The default path of GNUtar on hosted Windows runners export const GnuTarPathOnWindows = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` -export const SystemTarPathOnWindows = `${process.env['SYSTEMDRIVE']}\\Windows\\System32\\tar.exe` \ No newline at end of file +export const SystemTarPathOnWindows = `${process.env['SYSTEMDRIVE']}\\Windows\\System32\\tar.exe` diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 450cde3b..182bc7bb 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -3,7 +3,7 @@ import * as io from '@actions/io' import {existsSync, writeFileSync} from 'fs' import * as path from 'path' import * as utils from './cacheUtils' -import {CompressionMethod, GnuTarPathOnWindows, SystemTarPathOnWindows} from './constants' +import {CompressionMethod, SystemTarPathOnWindows} from './constants' const IS_WINDOWS = process.platform === 'win32' @@ -16,7 +16,7 @@ async function getTarPath(args: string[]): Promise { if (gnuTar) { // Use GNUtar as default on windows if (args.length > 0) { - args.push('--force-local') + args.push('--force-local') } return gnuTar } else if (existsSync(systemTar)) { @@ -54,7 +54,9 @@ function getWorkingDirectory(): string { } // Common function for extractTar and listTar to get the compression method -async function getCompressionProgram(compressionMethod: CompressionMethod): Promise { +async function getCompressionProgram( + compressionMethod: CompressionMethod +): Promise { // -d: Decompress. // 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. @@ -68,7 +70,7 @@ async function getCompressionProgram(compressionMethod: CompressionMethod): Prom } return [ '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { @@ -136,9 +138,7 @@ export async function createTar( switch (compressionMethod) { case CompressionMethod.Zstd: if (BSD_TAR_ZSTD) { - return [ - '-O', '|', 'zstd -T0 --long=30 -o' - ] + return ['-O', '|', 'zstd -T0 --long=30 -o'] } return [ '--use-compress-program', @@ -147,11 +147,13 @@ export async function createTar( ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_ZSTD) { - return [ - '-O', '|', 'zstd -T0 -o' - ] + return ['-O', '|', 'zstd -T0 -o'] } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt', '-cf'] + return [ + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0' : 'zstdmt', + '-cf' + ] default: return ['-z', '-cf'] } @@ -166,8 +168,7 @@ export async function createTar( '--files-from', manifestFilename, ...(await getCompressionProgram()), - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] await execTar(args, archiveFolder) } From b3bd482c0f13ddeac82b85fb36e393c86dbfd761 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Thu, 17 Nov 2022 09:46:01 +0000 Subject: [PATCH 08/24] Fix windows gnutar test case --- packages/cache/__tests__/tar.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index abf8eecb..3166ad68 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -113,7 +113,7 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { await tar.extractTar(archivePath, CompressionMethod.Gzip) - expect(isGnuMock).toHaveBeenCalledTimes(1) + expect(isGnuMock).toHaveBeenCalledTimes(2) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"tar"`, From f9dfb05bd2c556223750394ee101ff83c5b71da2 Mon Sep 17 00:00:00 2001 From: Lovepreet Singh Date: Thu, 17 Nov 2022 12:13:49 +0000 Subject: [PATCH 09/24] Temporarily remove thhe condition that prevents zstd usage on windows unless with GNUtar --- packages/cache/src/internal/cacheUtils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index 64fef08a..03c52561 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -94,10 +94,10 @@ async function getVersion(app: string): Promise { // Use zstandard if possible to maximize cache performance export async function getCompressionMethod(): Promise { - if (process.platform === 'win32' && !(await getGnuTarPathOnWindows())) { - // Disable zstd due to bug https://github.com/actions/cache/issues/301 - return CompressionMethod.Gzip - } + // if (process.platform === 'win32' && !(await getGnuTarPathOnWindows())) { + // // Disable zstd due to bug https://github.com/actions/cache/issues/301 + // return CompressionMethod.Gzip + // } const versionOutput = await getVersion('zstd') const version = semver.clean(versionOutput) From 54eb9b8055b5027dd2907ce2e8ec17b60222c190 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 21 Nov 2022 12:05:03 +0000 Subject: [PATCH 10/24] Address some comments and correct compression commands --- packages/cache/__tests__/tar.test.ts | 12 +++--- packages/cache/src/internal/cacheUtils.ts | 5 --- packages/cache/src/internal/tar.ts | 47 +++++++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 3166ad68..bdb79cb8 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -149,6 +149,8 @@ test('zstd create tar', async () => { `"${tarPath}"`, [ '--posix', + '-cf', + IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, '--exclude', IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, '-P', @@ -157,9 +159,7 @@ test('zstd create tar', async () => { '--files-from', 'manifest.txt', '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30', - '-cf', - IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), @@ -187,6 +187,8 @@ test('gzip create tar', async () => { `"${tarPath}"`, [ '--posix', + '-cf', + IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, '--exclude', IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, '-P', @@ -194,9 +196,7 @@ test('gzip create tar', async () => { IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', 'manifest.txt', - '-z', - '-cf', - IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip + '-z' ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []), diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index 03c52561..ea1e7de6 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -94,11 +94,6 @@ async function getVersion(app: string): Promise { // Use zstandard if possible to maximize cache performance export async function getCompressionMethod(): Promise { - // if (process.platform === 'win32' && !(await getGnuTarPathOnWindows())) { - // // Disable zstd due to bug https://github.com/actions/cache/issues/301 - // return CompressionMethod.Gzip - // } - const versionOutput = await getVersion('zstd') const version = semver.clean(versionOutput) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 182bc7bb..99c638ec 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -62,10 +62,10 @@ async function getCompressionProgram( // --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 BSD_TAR_ZSTD = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { case CompressionMethod.Zstd: - if (BSD_TAR_ZSTD) { + if (BSD_TAR_WINDOWS) { return ['-a'] // auto-detect compression } return [ @@ -73,7 +73,7 @@ async function getCompressionProgram( IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ] case CompressionMethod.ZstdWithoutLong: - if (BSD_TAR_ZSTD) { + if (BSD_TAR_WINDOWS) { return ['a'] // auto-detect compression } return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] @@ -121,6 +121,9 @@ export async function createTar( // Write source directories to manifest.txt to avoid command length limits const manifestFilename = 'manifest.txt' const cacheFileName = utils.getCacheFileName(compressionMethod) + const tarFile = 'cache.tar' + const tarPath = await getTarPath([]) + const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows writeFileSync( path.join(archiveFolder, manifestFilename), sourceDirectories.join('\n') @@ -133,42 +136,44 @@ export async function createTar( // 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 { - const tarPath = await getTarPath([]) - const BSD_TAR_ZSTD = IS_WINDOWS && tarPath === SystemTarPathOnWindows switch (compressionMethod) { case CompressionMethod.Zstd: - if (BSD_TAR_ZSTD) { - return ['-O', '|', 'zstd -T0 --long=30 -o'] + if (BSD_TAR_WINDOWS) { + return ['&&', 'zstd -T0 --long=30 -o'] } return [ '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30', - '-cf' + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' ] case CompressionMethod.ZstdWithoutLong: - if (BSD_TAR_ZSTD) { - return ['-O', '|', 'zstd -T0 -o'] + if (BSD_TAR_WINDOWS) { + return ['&&', 'zstd -T0 -o'] } - return [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0' : 'zstdmt', - '-cf' - ] + return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] default: - return ['-z', '-cf'] + return ['-z'] } } const args = [ '--posix', + '-cf', + BSD_TAR_WINDOWS + ? tarFile + : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--exclude', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + 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()), - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/') - ] + ...(await getCompressionProgram()) + ].concat( + BSD_TAR_WINDOWS + ? [cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/')] + : [] + ) await execTar(args, archiveFolder) } From 32b95825ba44015ec43f5089391e924e6ef061cd Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 21 Nov 2022 12:19:44 +0000 Subject: [PATCH 11/24] Add windows bsdtar test --- packages/cache/__tests__/tar.test.ts | 44 ++++++++++++++++++++++++++++ packages/cache/src/internal/tar.ts | 2 +- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index bdb79cb8..5d58774e 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -169,6 +169,50 @@ test('zstd create tar', async () => { ) }) +test('zstd create tar with windows BSDtar', async () => { + if (IS_WINDOWS) { + const execMock = jest.spyOn(exec, 'exec') + const isGnuMock = jest + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('')) + + const archiveFolder = getTempDir() + const workspace = process.env['GITHUB_WORKSPACE'] + const sourceDirectories = ['~/.npm/cache', `${workspace}/dist`] + + await fs.promises.mkdir(archiveFolder, {recursive: true}) + + await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) + + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath + + // expect(isGnuMock).toHaveBeenCalledTimes(1) + expect(execMock).toHaveBeenCalledTimes(1) + expect(execMock).toHaveBeenCalledWith( + `"${tarPath}"`, + [ + '--posix', + '-cf', + IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, + '--exclude', + IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, + '-P', + '-C', + IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, + '--files-from', + 'manifest.txt', + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + ] + .concat(IS_WINDOWS ? ['--force-local'] : []) + .concat(IS_MAC ? ['--delay-directory-restore'] : []), + { + cwd: archiveFolder + } + ) + } +}) + test('gzip create tar', async () => { const execMock = jest.spyOn(exec, 'exec') diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 99c638ec..964f6437 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -74,7 +74,7 @@ async function getCompressionProgram( ] case CompressionMethod.ZstdWithoutLong: if (BSD_TAR_WINDOWS) { - return ['a'] // auto-detect compression + return ['-a'] // auto-detect compression } return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: From 32f538163db2fb4a6e13eccad3b4a66a7bc50ab2 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 21 Nov 2022 12:51:07 +0000 Subject: [PATCH 12/24] Fix windows test --- packages/cache/__tests__/tar.test.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 5d58774e..477dd7dc 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -4,7 +4,8 @@ import * as path from 'path' import { CacheFilename, CompressionMethod, - GnuTarPathOnWindows + GnuTarPathOnWindows, + SystemTarPathOnWindows } from '../src/internal/constants' import * as tar from '../src/internal/tar' import * as utils from '../src/internal/cacheUtils' @@ -179,33 +180,33 @@ 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" await fs.promises.mkdir(archiveFolder, {recursive: true}) await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath + const tarPath = SystemTarPathOnWindows - // expect(isGnuMock).toHaveBeenCalledTimes(1) + expect(isGnuMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, [ '--posix', '-cf', - IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, + tarFilename.replace(/\\/g, '/'), '--exclude', - IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, + tarFilename.replace(/\\/g, '/'), '-P', '-C', - IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, + workspace?.replace(/\\/g, '/'), '--files-from', 'manifest.txt', - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' - ] - .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + "&&", + "zstd -T0 --long=30 -o", + CacheFilename.Zstd.replace(/\\/g, '/') + ], { cwd: archiveFolder } From ffde3e4bd50fb0dce86b72c6a56c7249c729d60a Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 22 Nov 2022 07:33:01 +0000 Subject: [PATCH 13/24] Fix test --- packages/cache/__tests__/tar.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 477dd7dc..33781214 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -188,7 +188,7 @@ test('zstd create tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows - expect(isGnuMock).toHaveBeenCalledTimes(1) + expect(isGnuMock).toHaveBeenCalledTimes(2) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, From 2f73afa8435b06b211571703a2cc28c4e3750e92 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 07:39:15 +0000 Subject: [PATCH 14/24] 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) } From 39b7a86b23c0f2db28788407bb33741e26aaa18f Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 07:58:46 +0000 Subject: [PATCH 15/24] Fix old tests --- packages/cache/__tests__/tar.test.ts | 71 +++++++++++++--------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 4b28c7cb..f1792217 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -63,12 +63,14 @@ test('zstd extract tar', async () => { IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', '-C', - IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, - '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace ] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat([ + '--use-compress-program', + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + ]), {cwd: undefined} ) }) @@ -93,11 +95,11 @@ test('gzip extract tar', async () => { IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', '-C', - IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, - '-z' + IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace ] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat(['-z']), {cwd: undefined} ) }) @@ -119,13 +121,13 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { expect(execMock).toHaveBeenCalledWith( `"tar"`, [ - '-z', '-xf', archivePath.replace(/\\/g, '/'), '-P', '-C', workspace?.replace(/\\/g, '/'), - '--force-local' + '--force-local', + '-z' ], {cwd: undefined} ) @@ -158,12 +160,14 @@ test('zstd create tar', async () => { '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt', - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + 'manifest.txt' ] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat([ + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + ]), { cwd: archiveFolder } @@ -245,11 +249,11 @@ test('gzip create tar', async () => { '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt', - '-z' + 'manifest.txt' ] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat(['-z']), { cwd: archiveFolder } @@ -269,15 +273,13 @@ test('zstd list tar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, - [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', - '-tf', - IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, - '-P' - ] + ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat([ + '--use-compress-program', + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + ]), {cwd: undefined} ) }) @@ -295,15 +297,10 @@ test('zstdWithoutLong list tar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, - [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -d' : 'unzstd', - '-tf', - IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, - '-P' - ] + ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat(['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']), {cwd: undefined} ) }) @@ -320,14 +317,10 @@ test('gzip list tar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, - [ - '-z', - '-tf', - IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, - '-P' - ] + ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + .concat(IS_MAC ? ['--delay-directory-restore'] : []) + .concat(['-z']), {cwd: undefined} ) }) From 1f3371766a9666538ac3f0128f7a8db4ad60a7c9 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 10:44:38 +0000 Subject: [PATCH 16/24] Add new tests --- packages/cache/__tests__/tar.test.ts | 72 ++++++++++++++-- packages/cache/src/internal/tar.ts | 118 ++++++++++++++------------- 2 files changed, 128 insertions(+), 62 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index f1792217..36c0a38d 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -18,7 +18,7 @@ jest.mock('@actions/io') const IS_WINDOWS = process.platform === 'win32' const IS_MAC = process.platform === 'darwin' -const defaultTarPath = process.platform === 'darwin' ? 'gtar' : 'tar' +const defaultTarPath = IS_MAC ? 'gtar' : 'tar' function getTempDir(): string { return path.join(__dirname, '_temp', 'tar') @@ -75,6 +75,41 @@ test('zstd extract tar', async () => { ) }) +test('zstd extract tar with windows BSDtar', async () => { + if (IS_WINDOWS) { + const mkdirMock = jest.spyOn(io, 'mkdirP') + const execMock = jest.spyOn(exec, 'exec') + jest + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('')) + + const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` + const workspace = process.env['GITHUB_WORKSPACE'] + const tarPath = SystemTarPathOnWindows + const tarFilename = 'cache.tar' + + await tar.extractTar(archivePath, CompressionMethod.Zstd) + + expect(mkdirMock).toHaveBeenCalledWith(workspace) + expect(execMock).toHaveBeenCalledTimes(1) + expect(execMock).toHaveBeenCalledWith( + 'zstd -d --long=30 -o', + [ + tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&', + `"${tarPath}"`, + '-xf', + tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P', + '-C', + workspace?.replace(/\\/g, '/') + ], + {cwd: undefined} + ) + } +}) + test('gzip extract tar', async () => { const mkdirMock = jest.spyOn(io, 'mkdirP') const execMock = jest.spyOn(exec, 'exec') @@ -107,7 +142,7 @@ test('gzip extract tar', async () => { test('gzip extract GNU tar on windows with GNUtar in path', async () => { if (IS_WINDOWS) { // GNU tar present in path but not at default location - const isGnuMock = jest + jest .spyOn(utils, 'getGnuTarPathOnWindows') .mockReturnValue(Promise.resolve('tar')) const execMock = jest.spyOn(exec, 'exec') @@ -116,7 +151,6 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { await tar.extractTar(archivePath, CompressionMethod.Gzip) - expect(isGnuMock).toHaveBeenCalledTimes(2) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"tar"`, @@ -177,7 +211,7 @@ test('zstd create tar', async () => { test('zstd create tar with windows BSDtar', async () => { if (IS_WINDOWS) { const execMock = jest.spyOn(exec, 'exec') - const isGnuMock = jest + jest .spyOn(utils, 'getGnuTarPathOnWindows') .mockReturnValue(Promise.resolve('')) @@ -196,7 +230,6 @@ test('zstd create tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows - expect(isGnuMock).toHaveBeenCalledTimes(2) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -284,6 +317,35 @@ test('zstd list tar', async () => { ) }) +test('zstd list tar with windows BSDtar', async () => { + if (IS_WINDOWS) { + const execMock = jest.spyOn(exec, 'exec') + jest + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('')) + const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` + + await tar.listTar(archivePath, CompressionMethod.Zstd) + + const tarFilename = 'cache.tar' + const tarPath = SystemTarPathOnWindows + expect(execMock).toHaveBeenCalledTimes(1) + expect(execMock).toHaveBeenCalledWith( + 'zstd -d --long=30 -o', + [ + tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&', + `"${tarPath}"`, + '-tf', + tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '-P' + ], + {cwd: undefined} + ) + } +}) + test('zstdWithoutLong list tar', async () => { const execMock = jest.spyOn(exec, 'exec') diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 8647fdc0..27fcf1ca 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -46,7 +46,9 @@ async function getTarArgs( const tarFile = 'cache.tar' const tarPath = await getTarPath() const workingDirectory = getWorkingDirectory() - const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_ZSTD = + tarPath === SystemTarPathOnWindows && + compressionMethod !== CompressionMethod.Gzip // Method specific args switch (type) { @@ -54,11 +56,11 @@ async function getTarArgs( args.push( '--posix', '-cf', - BSD_TAR_WINDOWS + BSD_TAR_ZSTD ? tarFile : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--exclude', - BSD_TAR_WINDOWS + BSD_TAR_ZSTD ? tarFile : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', @@ -71,7 +73,7 @@ async function getTarArgs( case 'extract': args.push( '-xf', - BSD_TAR_WINDOWS + BSD_TAR_ZSTD ? tarFile : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', @@ -79,11 +81,10 @@ async function getTarArgs( 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 + BSD_TAR_ZSTD ? tarFile : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' @@ -149,31 +150,31 @@ async function getCompressionProgram( const tarPath = await getTarPath() const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' - const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_ZSTD = + tarPath === SystemTarPathOnWindows && + compressionMethod !== CompressionMethod.Gzip switch (compressionMethod) { case CompressionMethod.Zstd: - if (BSD_TAR_WINDOWS) { - return [ - 'zstd -d --long=30 -o', - tarFile, - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' - ] - } - return [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' - ] + return BSD_TAR_ZSTD + ? [ + 'zstd -d --long=30 -o', + tarFile, + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&' + ] + : [ + '--use-compress-program', + IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + ] case CompressionMethod.ZstdWithoutLong: - if (BSD_TAR_WINDOWS) { - return [ - 'zstd -d -o', - tarFile, - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' - ] - } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] + return BSD_TAR_ZSTD + ? [ + 'zstd -d -o', + tarFile, + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + '&&' + ] + : ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] default: return ['-z'] } @@ -184,13 +185,15 @@ export async function listTar( compressionMethod: CompressionMethod ): Promise { const tarPath = await getTarPath() - const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_ZSTD = + tarPath === SystemTarPathOnWindows && + compressionMethod !== CompressionMethod.Gzip 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) { + if (BSD_TAR_ZSTD) { const command = compressionArgs[0] - const args = compressionArgs.slice(1).concat(tarArgs) + const args = compressionArgs.slice(1).concat([tarPath]).concat(tarArgs) await execCommand(command, args) } else { const args = tarArgs.concat(compressionArgs) @@ -205,13 +208,15 @@ export async function extractTar( // Create directory to extract tar into const workingDirectory = getWorkingDirectory() const tarPath = await getTarPath() - const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_ZSTD = + tarPath === SystemTarPathOnWindows && + compressionMethod !== CompressionMethod.Gzip await io.mkdirP(workingDirectory) const tarArgs = await getTarArgs(compressionMethod, 'extract', archivePath) const compressionArgs = await getCompressionProgram(compressionMethod) - if (BSD_TAR_WINDOWS) { + if (BSD_TAR_ZSTD) { const command = compressionArgs[0] - const args = compressionArgs.slice(1).concat(tarArgs) + const args = compressionArgs.slice(1).concat([tarPath]).concat(tarArgs) await execCommand(command, args) } else { const args = tarArgs.concat(compressionArgs) @@ -229,12 +234,13 @@ export async function createTar( const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' const tarPath = await getTarPath() - const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows + const BSD_TAR_ZSTD = + tarPath === SystemTarPathOnWindows && + compressionMethod !== CompressionMethod.Gzip writeFileSync( path.join(archiveFolder, manifestFilename), sourceDirectories.join('\n') ) - const workingDirectory = getWorkingDirectory() // -T#: Compress using # working thread. If # is 0, attempt to detect and use the number of physical CPU cores. // zstdmt is equivalent to 'zstd -T0' @@ -244,28 +250,26 @@ export async function createTar( function getCompressionProgram(): string[] { switch (compressionMethod) { case CompressionMethod.Zstd: - if (BSD_TAR_WINDOWS) { - return [ - '&&', - 'zstd -T0 --long=30 -o', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile - ] - } - return [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' - ] + return BSD_TAR_ZSTD + ? [ + '&&', + 'zstd -T0 --long=30 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] + : [ + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + ] case CompressionMethod.ZstdWithoutLong: - if (BSD_TAR_WINDOWS) { - return [ - '&&', - 'zstd -T0 -o', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile - ] - } - return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] + return BSD_TAR_ZSTD + ? [ + '&&', + 'zstd -T0 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] + : ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] default: return ['-z'] } From 187781e27372d5c8a32e2e199c0abb093b86d704 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 11:40:11 +0000 Subject: [PATCH 17/24] Fix tests --- packages/cache/__tests__/tar.test.ts | 4 ++-- packages/cache/src/internal/tar.ts | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 36c0a38d..5d997758 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -98,7 +98,7 @@ test('zstd extract tar with windows BSDtar', async () => { tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', - `"${tarPath}"`, + `${tarPath}`, '-xf', tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', @@ -336,7 +336,7 @@ test('zstd list tar with windows BSDtar', async () => { tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', - `"${tarPath}"`, + `${tarPath}`, '-tf', tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 27fcf1ca..a38302ec 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -141,7 +141,8 @@ function getWorkingDirectory(): string { // Common function for extractTar and listTar to get the compression method async function getCompressionProgram( - compressionMethod: CompressionMethod + compressionMethod: CompressionMethod, + archivePath: string ): Promise { // -d: Decompress. // unzstd is equivalent to 'zstd -d' @@ -159,7 +160,7 @@ async function getCompressionProgram( ? [ 'zstd -d --long=30 -o', tarFile, - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&' ] : [ @@ -171,7 +172,7 @@ async function getCompressionProgram( ? [ 'zstd -d -o', tarFile, - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&' ] : ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] @@ -188,12 +189,18 @@ export async function listTar( const BSD_TAR_ZSTD = tarPath === SystemTarPathOnWindows && compressionMethod !== CompressionMethod.Gzip - const compressionArgs = await getCompressionProgram(compressionMethod) + const compressionArgs = await getCompressionProgram( + compressionMethod, + archivePath + ) const tarArgs = await getTarArgs(compressionMethod, 'list', archivePath) // TODO: Add a test for BSD tar on windows if (BSD_TAR_ZSTD) { const command = compressionArgs[0] - const args = compressionArgs.slice(1).concat([tarPath]).concat(tarArgs) + const args = compressionArgs + .slice(1) + .concat([tarPath]) + .concat(tarArgs) await execCommand(command, args) } else { const args = tarArgs.concat(compressionArgs) @@ -213,10 +220,16 @@ export async function extractTar( compressionMethod !== CompressionMethod.Gzip await io.mkdirP(workingDirectory) const tarArgs = await getTarArgs(compressionMethod, 'extract', archivePath) - const compressionArgs = await getCompressionProgram(compressionMethod) + const compressionArgs = await getCompressionProgram( + compressionMethod, + archivePath + ) if (BSD_TAR_ZSTD) { const command = compressionArgs[0] - const args = compressionArgs.slice(1).concat([tarPath]).concat(tarArgs) + const args = compressionArgs + .slice(1) + .concat([tarPath]) + .concat(tarArgs) await execCommand(command, args) } else { const args = tarArgs.concat(compressionArgs) From 0822441ee01f9c599f4946878791dbe57b352d79 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 23 Nov 2022 11:51:46 +0000 Subject: [PATCH 18/24] Fix lint test --- packages/cache/src/internal/tar.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index a38302ec..041f2b5e 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -149,7 +149,6 @@ async function getCompressionProgram( // --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 cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' const BSD_TAR_ZSTD = tarPath === SystemTarPathOnWindows && From 0fd856d0a0b514fe05fb361f6d938c0bc563ac72 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 28 Nov 2022 10:24:40 +0000 Subject: [PATCH 19/24] Refactor code --- packages/cache/src/internal/constants.ts | 5 + packages/cache/src/internal/contracts.d.ts | 5 + packages/cache/src/internal/tar.ts | 238 ++++++++++----------- 3 files changed, 118 insertions(+), 130 deletions(-) diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index c6d8a490..7d0eb65d 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -11,6 +11,11 @@ export enum CompressionMethod { Zstd = 'zstd' } +export enum ArchiveToolType { + GNU = 'gnu', + BSD = 'bsd' +} + // The default number of retry attempts. export const DefaultRetryAttempts = 2 diff --git a/packages/cache/src/internal/contracts.d.ts b/packages/cache/src/internal/contracts.d.ts index 1b2a13a1..4aa48791 100644 --- a/packages/cache/src/internal/contracts.d.ts +++ b/packages/cache/src/internal/contracts.d.ts @@ -31,3 +31,8 @@ export interface InternalCacheOptions { compressionMethod?: CompressionMethod cacheSize?: number } + +export interface ArchiveTool { + path: string + type: string +} \ No newline at end of file diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 041f2b5e..00b14fda 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -3,21 +3,26 @@ import * as io from '@actions/io' import {existsSync, writeFileSync} from 'fs' import * as path from 'path' import * as utils from './cacheUtils' -import {CompressionMethod, SystemTarPathOnWindows} from './constants' +import {ArchiveTool} from './contracts' +import { + CompressionMethod, + SystemTarPathOnWindows, + ArchiveToolType +} 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(): Promise { +async function getTarPath(): Promise { switch (process.platform) { case 'win32': { const gnuTar = await utils.getGnuTarPathOnWindows() const systemTar = SystemTarPathOnWindows if (gnuTar) { // Use GNUtar as default on windows - return gnuTar + return {path: gnuTar, type: ArchiveToolType.GNU} } else if (existsSync(systemTar)) { - return systemTar + return {path: systemTar, type: ArchiveToolType.BSD} } break } @@ -25,30 +30,39 @@ async function getTarPath(): 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 - return gnuTar + return {path: gnuTar, type: ArchiveToolType.GNU} + } else { + return { + path: await io.which('tar', true), + type: ArchiveToolType.BSD + } } - break } default: break } - return await io.which('tar', true) + return { + path: await io.which('tar', true), + type: ArchiveToolType.GNU + } } +// Return arguments for tar as per tarPath, compressionMethod, method type and os async function getTarArgs( + tarPath: ArchiveTool, compressionMethod: CompressionMethod, type: string, archivePath = '' ): Promise { - const args = [] + const args = [tarPath.path] const manifestFilename = 'manifest.txt' const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' - const tarPath = await getTarPath() const workingDirectory = getWorkingDirectory() const BSD_TAR_ZSTD = - tarPath === SystemTarPathOnWindows && - compressionMethod !== CompressionMethod.Gzip + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS // Method specific args switch (type) { @@ -93,45 +107,44 @@ async function getTarArgs( } // Platform specific args - switch (process.platform) { - case 'win32': { - const gnuTar = await utils.getGnuTarPathOnWindows() - if (gnuTar) { - // Use GNUtar as default on windows + if (tarPath.type === ArchiveToolType.GNU) { + switch (process.platform) { + case 'win32': 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 + break + case 'darwin': args.push('--delay-directory-restore') - } - break + break } } return args } -async function execTar(args: string[], cwd?: string): Promise { - try { - 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}`) +async function getArgs( + compressionMethod: CompressionMethod, + type: string, + archivePath = '' +): Promise { + const tarPath = await getTarPath() + const tarArgs = await getTarArgs( + tarPath, + compressionMethod, + type, + archivePath + ) + const compressionArgs = + type !== 'create' + ? await getDecompressionProgram(tarPath, compressionMethod, archivePath) + : await getCompressionProgram(tarPath, compressionMethod) + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS + if (BSD_TAR_ZSTD && type !== 'create') { + return [...compressionArgs, ...tarArgs].join(' ') + } else { + return [...tarArgs, ...compressionArgs].join(' ') } } @@ -140,7 +153,8 @@ function getWorkingDirectory(): string { } // Common function for extractTar and listTar to get the compression method -async function getCompressionProgram( +async function getDecompressionProgram( + tarPath: ArchiveTool, compressionMethod: CompressionMethod, archivePath: string ): Promise { @@ -148,11 +162,11 @@ 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 tarFile = 'cache.tar' const BSD_TAR_ZSTD = - tarPath === SystemTarPathOnWindows && - compressionMethod !== CompressionMethod.Gzip + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS switch (compressionMethod) { case CompressionMethod.Zstd: return BSD_TAR_ZSTD @@ -180,31 +194,54 @@ async function getCompressionProgram( } } +// -T#: Compress using # working thread. If # is 0, attempt to detect and use the number of physical CPU cores. +// zstdmt is equivalent to 'zstd -T0' +// --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( + tarPath: ArchiveTool, + compressionMethod: CompressionMethod +): Promise { + const cacheFileName = utils.getCacheFileName(compressionMethod) + const tarFile = 'cache.tar' + const BSD_TAR_ZSTD = + tarPath.type === ArchiveToolType.BSD && + compressionMethod !== CompressionMethod.Gzip && + IS_WINDOWS + switch (compressionMethod) { + case CompressionMethod.Zstd: + return BSD_TAR_ZSTD + ? [ + '&&', + 'zstd -T0 --long=30 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] + : [ + '--use-compress-program', + IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + ] + case CompressionMethod.ZstdWithoutLong: + return BSD_TAR_ZSTD + ? [ + '&&', + 'zstd -T0 -o', + cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + tarFile + ] + : ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] + default: + return ['-z'] + } +} + export async function listTar( archivePath: string, compressionMethod: CompressionMethod ): Promise { - const tarPath = await getTarPath() - const BSD_TAR_ZSTD = - tarPath === SystemTarPathOnWindows && - compressionMethod !== CompressionMethod.Gzip - const compressionArgs = await getCompressionProgram( - compressionMethod, - archivePath - ) - const tarArgs = await getTarArgs(compressionMethod, 'list', archivePath) - // TODO: Add a test for BSD tar on windows - if (BSD_TAR_ZSTD) { - const command = compressionArgs[0] - const args = compressionArgs - .slice(1) - .concat([tarPath]) - .concat(tarArgs) - await execCommand(command, args) - } else { - const args = tarArgs.concat(compressionArgs) - await execTar(args) - } + const args = await getArgs(compressionMethod, 'list', archivePath) + exec(args) } export async function extractTar( @@ -213,27 +250,9 @@ export async function extractTar( ): Promise { // Create directory to extract tar into const workingDirectory = getWorkingDirectory() - const tarPath = await getTarPath() - const BSD_TAR_ZSTD = - tarPath === SystemTarPathOnWindows && - compressionMethod !== CompressionMethod.Gzip await io.mkdirP(workingDirectory) - const tarArgs = await getTarArgs(compressionMethod, 'extract', archivePath) - const compressionArgs = await getCompressionProgram( - compressionMethod, - archivePath - ) - if (BSD_TAR_ZSTD) { - const command = compressionArgs[0] - const args = compressionArgs - .slice(1) - .concat([tarPath]) - .concat(tarArgs) - await execCommand(command, args) - } else { - const args = tarArgs.concat(compressionArgs) - await execTar(args) - } + const args = await getArgs(compressionMethod, 'extract', archivePath) + exec(args) } export async function createTar( @@ -243,51 +262,10 @@ export async function createTar( ): Promise { // Write source directories to manifest.txt to avoid command length limits const manifestFilename = 'manifest.txt' - const cacheFileName = utils.getCacheFileName(compressionMethod) - const tarFile = 'cache.tar' - const tarPath = await getTarPath() - const BSD_TAR_ZSTD = - tarPath === SystemTarPathOnWindows && - compressionMethod !== CompressionMethod.Gzip writeFileSync( path.join(archiveFolder, manifestFilename), sourceDirectories.join('\n') ) - - // -T#: Compress using # working thread. If # is 0, attempt to detect and use the number of physical CPU cores. - // zstdmt is equivalent to 'zstd -T0' - // --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. - function getCompressionProgram(): string[] { - switch (compressionMethod) { - case CompressionMethod.Zstd: - return BSD_TAR_ZSTD - ? [ - '&&', - 'zstd -T0 --long=30 -o', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile - ] - : [ - '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' - ] - case CompressionMethod.ZstdWithoutLong: - return BSD_TAR_ZSTD - ? [ - '&&', - 'zstd -T0 -o', - cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile - ] - : ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] - default: - return ['-z'] - } - } - const tarArgs = await getTarArgs(compressionMethod, 'create') - const compressionArgs = getCompressionProgram() - const args = tarArgs.concat(compressionArgs) - await execTar(args, archiveFolder) + const args = await getArgs(compressionMethod, 'create') + await exec(args) } From 34f0143be2d12d6299b67df7c6903695f3de84c3 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 29 Nov 2022 10:35:22 +0000 Subject: [PATCH 20/24] Address review comments --- packages/cache/__tests__/tar.test.ts | 79 ++++++++++++------------ packages/cache/src/internal/constants.ts | 5 ++ packages/cache/src/internal/tar.ts | 17 +++-- 3 files changed, 54 insertions(+), 47 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 5d997758..e868363f 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -5,7 +5,9 @@ import { CacheFilename, CompressionMethod, GnuTarPathOnWindows, - SystemTarPathOnWindows + ManifestFilename, + SystemTarPathOnWindows, + TarFilename } from '../src/internal/constants' import * as tar from '../src/internal/tar' import * as utils from '../src/internal/cacheUtils' @@ -57,8 +59,8 @@ test('zstd extract tar', async () => { expect(mkdirMock).toHaveBeenCalledWith(workspace) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, [ + `"${tarPath}"`, '-xf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', @@ -70,7 +72,8 @@ test('zstd extract tar', async () => { .concat([ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' - ]), + ]) + .join(' '), {cwd: undefined} ) }) @@ -86,25 +89,24 @@ test('zstd extract tar with windows BSDtar', async () => { const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] const tarPath = SystemTarPathOnWindows - const tarFilename = 'cache.tar' await tar.extractTar(archivePath, CompressionMethod.Zstd) expect(mkdirMock).toHaveBeenCalledWith(workspace) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - 'zstd -d --long=30 -o', [ - tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + 'zstd -d --long=30 -o', + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', `${tarPath}`, '-xf', - tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', workspace?.replace(/\\/g, '/') - ], + ].join(' '), {cwd: undefined} ) } @@ -124,8 +126,8 @@ test('gzip extract tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, [ + `"${tarPath}"`, '-xf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P', @@ -134,7 +136,8 @@ test('gzip extract tar', async () => { ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) - .concat(['-z']), + .concat(['-z']) + .join(' '), {cwd: undefined} ) }) @@ -153,8 +156,8 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"tar"`, [ + `"tar"`, '-xf', archivePath.replace(/\\/g, '/'), '-P', @@ -162,7 +165,7 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { workspace?.replace(/\\/g, '/'), '--force-local', '-z' - ], + ].join(' '), {cwd: undefined} ) } @@ -183,8 +186,8 @@ test('zstd create tar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, [ + `"${tarPath}"`, '--posix', '-cf', IS_WINDOWS ? CacheFilename.Zstd.replace(/\\/g, '/') : CacheFilename.Zstd, @@ -194,14 +197,15 @@ test('zstd create tar', async () => { '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt' + ManifestFilename ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' - ]), + ]) + .join(' '), { cwd: archiveFolder } @@ -218,7 +222,6 @@ 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' await fs.promises.mkdir(archiveFolder, {recursive: true}) @@ -232,23 +235,23 @@ test('zstd create tar with windows BSDtar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, [ + `"${tarPath}"`, '--posix', '-cf', - tarFilename.replace(/\\/g, '/'), + TarFilename.replace(/\\/g, '/'), '--exclude', - tarFilename.replace(/\\/g, '/'), + TarFilename.replace(/\\/g, '/'), '-P', '-C', workspace?.replace(/\\/g, '/'), '--files-from', - 'manifest.txt', + ManifestFilename, '&&', 'zstd -T0 --long=30 -o', CacheFilename.Zstd.replace(/\\/g, '/'), - tarFilename.replace(/\\/g, '/') - ], + TarFilename.replace(/\\/g, '/') + ].join(' '), { cwd: archiveFolder } @@ -282,11 +285,12 @@ test('gzip create tar', async () => { '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', - 'manifest.txt' + ManifestFilename ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) - .concat(['-z']), + .concat(['-z']) + .join(' '), { cwd: archiveFolder } @@ -305,14 +309,14 @@ test('zstd list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, - ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' - ]), + ]) + .join(' '), {cwd: undefined} ) }) @@ -327,20 +331,19 @@ test('zstd list tar with windows BSDtar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) - const tarFilename = 'cache.tar' const tarPath = SystemTarPathOnWindows expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - 'zstd -d --long=30 -o', [ - tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + 'zstd -d --long=30 -o', + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', `${tarPath}`, '-tf', - tarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), + TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' - ], + ].join(' '), {cwd: undefined} ) } @@ -358,11 +361,11 @@ test('zstdWithoutLong list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, - ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) - .concat(['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']), + .concat(['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']) + .join(' '), {cwd: undefined} ) }) @@ -378,11 +381,11 @@ test('gzip list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, - ['-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) - .concat(['-z']), + .concat(['-z']) + .join(' '), {cwd: undefined} ) }) diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index 7d0eb65d..b2cddf96 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -30,4 +30,9 @@ export const SocketTimeout = 5000 // The default path of GNUtar on hosted Windows runners export const GnuTarPathOnWindows = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` +// The default path of BSDtar on hosted Windows runners export const SystemTarPathOnWindows = `${process.env['SYSTEMDRIVE']}\\Windows\\System32\\tar.exe` + +export const TarFilename = 'cache.tar' + +export const ManifestFilename = 'manifest.txt' \ No newline at end of file diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 00b14fda..7549fb14 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -7,7 +7,9 @@ import {ArchiveTool} from './contracts' import { CompressionMethod, SystemTarPathOnWindows, - ArchiveToolType + ArchiveToolType, + TarFilename, + ManifestFilename } from './constants' const IS_WINDOWS = process.platform === 'win32' @@ -55,7 +57,6 @@ async function getTarArgs( archivePath = '' ): Promise { const args = [tarPath.path] - const manifestFilename = 'manifest.txt' const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' const workingDirectory = getWorkingDirectory() @@ -81,7 +82,7 @@ async function getTarArgs( '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '--files-from', - manifestFilename + ManifestFilename ) break case 'extract': @@ -162,7 +163,6 @@ async function getDecompressionProgram( // 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 tarFile = 'cache.tar' const BSD_TAR_ZSTD = tarPath.type === ArchiveToolType.BSD && compressionMethod !== CompressionMethod.Gzip && @@ -172,7 +172,7 @@ async function getDecompressionProgram( return BSD_TAR_ZSTD ? [ 'zstd -d --long=30 -o', - tarFile, + TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&' ] @@ -184,7 +184,7 @@ async function getDecompressionProgram( return BSD_TAR_ZSTD ? [ 'zstd -d -o', - tarFile, + TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&' ] @@ -204,7 +204,6 @@ async function getCompressionProgram( compressionMethod: CompressionMethod ): Promise { const cacheFileName = utils.getCacheFileName(compressionMethod) - const tarFile = 'cache.tar' const BSD_TAR_ZSTD = tarPath.type === ArchiveToolType.BSD && compressionMethod !== CompressionMethod.Gzip && @@ -216,7 +215,7 @@ async function getCompressionProgram( '&&', 'zstd -T0 --long=30 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile + TarFilename ] : [ '--use-compress-program', @@ -228,7 +227,7 @@ async function getCompressionProgram( '&&', 'zstd -T0 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - tarFile + TarFilename ] : ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] default: From 2e5a5174604f49b48c910fb50602fdea8e32a2d4 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 29 Nov 2022 12:16:17 +0000 Subject: [PATCH 21/24] Fix test --- packages/cache/__tests__/tar.test.ts | 50 +++++++++++++++++----------- packages/cache/src/internal/tar.ts | 7 ++-- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index e868363f..2143ad89 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -73,8 +73,7 @@ test('zstd extract tar', async () => { '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ]) - .join(' '), - {cwd: undefined} + .join(' ') ) }) @@ -106,8 +105,7 @@ test('zstd extract tar with windows BSDtar', async () => { '-P', '-C', workspace?.replace(/\\/g, '/') - ].join(' '), - {cwd: undefined} + ].join(' ') ) } }) @@ -137,8 +135,7 @@ test('gzip extract tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' '), - {cwd: undefined} + .join(' ') ) }) @@ -165,8 +162,7 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { workspace?.replace(/\\/g, '/'), '--force-local', '-z' - ].join(' '), - {cwd: undefined} + ].join(' ') ) } }) @@ -206,6 +202,7 @@ test('zstd create tar', async () => { IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' ]) .join(' '), + undefined, // args { cwd: archiveFolder } @@ -252,6 +249,7 @@ test('zstd create tar with windows BSDtar', async () => { CacheFilename.Zstd.replace(/\\/g, '/'), TarFilename.replace(/\\/g, '/') ].join(' '), + undefined, // args { cwd: archiveFolder } @@ -274,8 +272,8 @@ test('gzip create tar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, [ + `"${tarPath}"`, '--posix', '-cf', IS_WINDOWS ? CacheFilename.Gzip.replace(/\\/g, '/') : CacheFilename.Gzip, @@ -291,6 +289,7 @@ test('gzip create tar', async () => { .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) .join(' '), + undefined, // args { cwd: archiveFolder } @@ -309,15 +308,19 @@ test('zstd list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [ + `"${tarPath}"`, + '-tf', + IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, + '-P' + ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' ]) - .join(' '), - {cwd: undefined} + .join(' ') ) }) @@ -343,8 +346,7 @@ test('zstd list tar with windows BSDtar', async () => { '-tf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' - ].join(' '), - {cwd: undefined} + ].join(' ') ) } }) @@ -361,12 +363,16 @@ test('zstdWithoutLong list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [ + `"${tarPath}"`, + '-tf', + IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, + '-P' + ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']) - .join(' '), - {cwd: undefined} + .join(' ') ) }) @@ -381,11 +387,15 @@ test('gzip list tar', async () => { const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - [`"${tarPath}"`, '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P'] + [ + `"${tarPath}"`, + '-tf', + IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, + '-P' + ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' '), - {cwd: undefined} + .join(' ') ) }) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 7549fb14..f9f91d15 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -56,7 +56,7 @@ async function getTarArgs( type: string, archivePath = '' ): Promise { - const args = [tarPath.path] + const args = [`"${tarPath.path}"`] const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' const workingDirectory = getWorkingDirectory() @@ -260,11 +260,10 @@ export async function createTar( compressionMethod: CompressionMethod ): Promise { // Write source directories to manifest.txt to avoid command length limits - const manifestFilename = 'manifest.txt' writeFileSync( - path.join(archiveFolder, manifestFilename), + path.join(archiveFolder, ManifestFilename), sourceDirectories.join('\n') ) const args = await getArgs(compressionMethod, 'create') - await exec(args) + await exec(args, undefined, {cwd: archiveFolder}) } From 424ae62ee752742c2b7cff5fdab46aba4701b545 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 30 Nov 2022 06:05:34 +0000 Subject: [PATCH 22/24] Fix tar test --- packages/cache/__tests__/tar.test.ts | 4 ++-- packages/cache/src/internal/constants.ts | 2 +- packages/cache/src/internal/contracts.d.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 2143ad89..a669ccea 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -99,7 +99,7 @@ test('zstd extract tar with windows BSDtar', async () => { TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', - `${tarPath}`, + `"${tarPath}"`, '-xf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', @@ -342,7 +342,7 @@ test('zstd list tar with windows BSDtar', async () => { TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&', - `${tarPath}`, + `"${tarPath}"`, '-tf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index b2cddf96..4dbff574 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -35,4 +35,4 @@ export const SystemTarPathOnWindows = `${process.env['SYSTEMDRIVE']}\\Windows\\S export const TarFilename = 'cache.tar' -export const ManifestFilename = 'manifest.txt' \ No newline at end of file +export const ManifestFilename = 'manifest.txt' diff --git a/packages/cache/src/internal/contracts.d.ts b/packages/cache/src/internal/contracts.d.ts index 4aa48791..d215e26d 100644 --- a/packages/cache/src/internal/contracts.d.ts +++ b/packages/cache/src/internal/contracts.d.ts @@ -35,4 +35,4 @@ export interface InternalCacheOptions { export interface ArchiveTool { path: string type: string -} \ No newline at end of file +} From afbc5c0a9e6a290c6b2c574193af2c9f56da4413 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 30 Nov 2022 06:16:19 +0000 Subject: [PATCH 23/24] Add await to async function calls --- packages/cache/src/internal/tar.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index f9f91d15..db05a9e5 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -240,7 +240,11 @@ export async function listTar( compressionMethod: CompressionMethod ): Promise { const args = await getArgs(compressionMethod, 'list', archivePath) - exec(args) + try { + await exec(args) + } catch (error) { + throw new Error(`Tar failed with error: ${error?.message}`) + } } export async function extractTar( @@ -251,7 +255,11 @@ export async function extractTar( const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) const args = await getArgs(compressionMethod, 'extract', archivePath) - exec(args) + try { + await exec(args) + } catch (error) { + throw new Error(`Tar failed with error: ${error?.message}`) + } } export async function createTar( @@ -265,5 +273,9 @@ export async function createTar( sourceDirectories.join('\n') ) const args = await getArgs(compressionMethod, 'create') - await exec(args, undefined, {cwd: archiveFolder}) + try { + await exec(args, undefined, {cwd: archiveFolder}) + } catch (error) { + throw new Error(`Tar failed with error: ${error?.message}`) + } } From 85958314526502ff3572b86f6b237fd406cae6e3 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Wed, 30 Nov 2022 10:11:07 +0000 Subject: [PATCH 24/24] Fix test --- packages/cache/__tests__/tar.test.ts | 8 ++++---- packages/cache/src/internal/tar.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index a669ccea..c68d0be8 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -71,7 +71,7 @@ test('zstd extract tar', async () => { .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) .join(' ') ) @@ -199,7 +199,7 @@ test('zstd create tar', async () => { .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + IS_WINDOWS ? '"zstd -T0 --long=30"' : 'zstdmt --long=30' ]) .join(' '), undefined, // args @@ -318,7 +318,7 @@ test('zstd list tar', async () => { .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat([ '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) .join(' ') ) @@ -371,7 +371,7 @@ test('zstdWithoutLong list tar', async () => { ] .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) - .concat(['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']) + .concat(['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd']) .join(' ') ) }) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index db05a9e5..ae55d321 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -178,7 +178,7 @@ async function getDecompressionProgram( ] : [ '--use-compress-program', - IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30' + IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ] case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD @@ -188,7 +188,7 @@ async function getDecompressionProgram( archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '&&' ] - : ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd'] + : ['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd'] default: return ['-z'] } @@ -219,7 +219,7 @@ async function getCompressionProgram( ] : [ '--use-compress-program', - IS_WINDOWS ? 'zstd -T0 --long=30' : 'zstdmt --long=30' + IS_WINDOWS ? '"zstd -T0 --long=30"' : 'zstdmt --long=30' ] case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD @@ -229,7 +229,7 @@ async function getCompressionProgram( cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename ] - : ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt'] + : ['--use-compress-program', IS_WINDOWS ? '"zstd -T0"' : 'zstdmt'] default: return ['-z'] }