From 9366237c904b6ff20682f2204e8f9d9cac467ddb Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 7 Nov 2022 13:39:07 +0000 Subject: [PATCH 01/14] Add gnuTar as default in windows --- packages/cache/src/internal/tar.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 2e28ca1a..3b351414 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -14,7 +14,11 @@ async function getTarPath( switch (process.platform) { case 'win32': { const systemTar = `${process.env['windir']}\\System32\\tar.exe` - if (compressionMethod !== CompressionMethod.Gzip) { + const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` + if (compressionMethod !== CompressionMethod.Gzip && existsSync(gnuTar)) { + args.push('--force-local') + return gnuTar + } else if (compressionMethod !== CompressionMethod.Gzip) { // We only use zstandard compression on windows when gnu tar is installed due to // a bug with compressing large files with bsdtar + zstd args.push('--force-local') From 0c58e4113e697b8be21607b6a70f4de3dc9106c3 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 7 Nov 2022 13:43:54 +0000 Subject: [PATCH 02/14] Fix gnutar check on windows --- packages/cache/src/internal/cacheUtils.ts | 3 ++- packages/cache/src/internal/tar.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index c2ace526..4dd3a614 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -117,8 +117,9 @@ export function getCacheFileName(compressionMethod: CompressionMethod): string { } export async function isGnuTarInstalled(): Promise { + const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` const versionOutput = await getVersion('tar') - return versionOutput.toLowerCase().includes('gnu tar') + return versionOutput.toLowerCase().includes('gnu tar') || fs.existsSync(gnuTar) } export function assertDefined(name: string, value?: T): T { diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 3b351414..9d5add77 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -15,7 +15,7 @@ async function getTarPath( case 'win32': { const systemTar = `${process.env['windir']}\\System32\\tar.exe` const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` - if (compressionMethod !== CompressionMethod.Gzip && existsSync(gnuTar)) { + if (existsSync(gnuTar)) { args.push('--force-local') return gnuTar } else if (compressionMethod !== CompressionMethod.Gzip) { From 62a66a8ce9ca7e66b1a421b160a0499ae19e0a9e Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 8 Nov 2022 06:43:28 +0000 Subject: [PATCH 03/14] Fix tests --- packages/cache/__tests__/tar.test.ts | 4 ++-- packages/cache/src/internal/cacheUtils.ts | 4 +++- packages/cache/src/internal/tar.ts | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index e4233bc9..31af266e 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -92,9 +92,9 @@ test('gzip extract tar', async () => { ) }) -test('gzip extract GNU tar on windows', async () => { +test('gzip extract GNU tar on windows with GNUtar in path', async () => { if (IS_WINDOWS) { - jest.spyOn(fs, 'existsSync').mockReturnValueOnce(false) + jest.spyOn(fs, 'existsSync').mockReturnValue(false) const isGnuMock = jest .spyOn(utils, 'isGnuTarInstalled') diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index 4dd3a614..e6c09eec 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -119,7 +119,9 @@ export function getCacheFileName(compressionMethod: CompressionMethod): string { export async function isGnuTarInstalled(): Promise { const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` const versionOutput = await getVersion('tar') - return versionOutput.toLowerCase().includes('gnu tar') || fs.existsSync(gnuTar) + return ( + versionOutput.toLowerCase().includes('gnu tar') || fs.existsSync(gnuTar) + ) } export function assertDefined(name: string, value?: T): T { diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 9d5add77..b650f4d0 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -16,6 +16,7 @@ async function getTarPath( const systemTar = `${process.env['windir']}\\System32\\tar.exe` const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` if (existsSync(gnuTar)) { + // Making GNUtar + zstd as default on windows args.push('--force-local') return gnuTar } else if (compressionMethod !== CompressionMethod.Gzip) { From e0aadb573c67b49e4b8d290385253c40d69b6582 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 8 Nov 2022 08:29:31 +0000 Subject: [PATCH 04/14] Fix GNUtar path --- packages/cache/src/internal/cacheUtils.ts | 2 +- packages/cache/src/internal/tar.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index e6c09eec..da4b8d81 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -117,7 +117,7 @@ export function getCacheFileName(compressionMethod: CompressionMethod): string { } export async function isGnuTarInstalled(): Promise { - const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` + const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` const versionOutput = await getVersion('tar') return ( versionOutput.toLowerCase().includes('gnu tar') || fs.existsSync(gnuTar) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index b650f4d0..9e140fe0 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -14,7 +14,7 @@ async function getTarPath( switch (process.platform) { case 'win32': { const systemTar = `${process.env['windir']}\\System32\\tar.exe` - const gnuTar = `${process.env['windir']}\\Program Files\\Git\\usr\\bin\\tar.exe` + const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` if (existsSync(gnuTar)) { // Making GNUtar + zstd as default on windows args.push('--force-local') From 436cf8d6ea434a91181ebbeecb77344fdc6a6b81 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 8 Nov 2022 08:47:35 +0000 Subject: [PATCH 05/14] Fix tests --- packages/cache/__tests__/tar.test.ts | 43 +++++++++++++++++------ packages/cache/src/internal/cacheUtils.ts | 2 +- packages/cache/src/internal/tar.ts | 9 ++--- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 31af266e..6983d5f7 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -28,6 +28,10 @@ beforeAll(async () => { await jest.requireActual('@actions/io').rmRF(getTempDir()) }) +beforeEach(async () => { + jest.restoreAllMocks() +}) + afterAll(async () => { delete process.env['GITHUB_WORKSPACE'] await jest.requireActual('@actions/io').rmRF(getTempDir()) @@ -41,13 +45,16 @@ test('zstd extract tar', async () => { ? `${process.env['windir']}\\fakepath\\cache.tar` : 'cache.tar' const workspace = process.env['GITHUB_WORKSPACE'] + const tarPath = IS_WINDOWS + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + : defaultTarPath await tar.extractTar(archivePath, CompressionMethod.Zstd) expect(mkdirMock).toHaveBeenCalledWith(workspace) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${defaultTarPath}"`, + `"${tarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', @@ -75,7 +82,7 @@ test('gzip extract tar', async () => { expect(mkdirMock).toHaveBeenCalledWith(workspace) const tarPath = IS_WINDOWS - ? `${process.env['windir']}\\System32\\tar.exe` + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -87,7 +94,9 @@ test('gzip extract tar', async () => { '-P', '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace - ].concat(IS_MAC ? ['--delay-directory-restore'] : []), + ] + .concat(IS_WINDOWS ? ['--force-local'] : []) + .concat(IS_MAC ? ['--delay-directory-restore'] : []), {cwd: undefined} ) }) @@ -134,9 +143,13 @@ test('zstd create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) + const tarPath = IS_WINDOWS + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + : defaultTarPath + expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${defaultTarPath}"`, + `"${tarPath}"`, [ '--posix', '--use-compress-program', @@ -171,7 +184,7 @@ test('gzip create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Gzip) const tarPath = IS_WINDOWS - ? `${process.env['windir']}\\System32\\tar.exe` + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) @@ -189,7 +202,9 @@ test('gzip create tar', async () => { IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', 'manifest.txt' - ].concat(IS_MAC ? ['--delay-directory-restore'] : []), + ] + .concat(IS_WINDOWS ? ['--force-local'] : []) + .concat(IS_MAC ? ['--delay-directory-restore'] : []), { cwd: archiveFolder } @@ -205,9 +220,12 @@ test('zstd list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) + const tarPath = IS_WINDOWS + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${defaultTarPath}"`, + `"${tarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', @@ -230,9 +248,12 @@ test('zstdWithoutLong list tar', async () => { await tar.listTar(archivePath, CompressionMethod.ZstdWithoutLong) + const tarPath = IS_WINDOWS + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${defaultTarPath}"`, + `"${tarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd', @@ -255,7 +276,7 @@ test('gzip list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Gzip) const tarPath = IS_WINDOWS - ? `${process.env['windir']}\\System32\\tar.exe` + ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -265,7 +286,9 @@ test('gzip list tar', async () => { '-tf', IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath, '-P' - ].concat(IS_MAC ? ['--delay-directory-restore'] : []), + ] + .concat(IS_WINDOWS ? ['--force-local'] : []) + .concat(IS_MAC ? ['--delay-directory-restore'] : []), {cwd: undefined} ) }) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index da4b8d81..30538e24 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -67,7 +67,7 @@ export async function unlinkFile(filePath: fs.PathLike): Promise { return util.promisify(fs.unlink)(filePath) } -async function getVersion(app: string): Promise { +export async function getVersion(app: string): Promise { core.debug(`Checking ${app} --version`) let versionOutput = '' try { diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 9e140fe0..1941afd1 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -16,17 +16,18 @@ async function getTarPath( const systemTar = `${process.env['windir']}\\System32\\tar.exe` const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` if (existsSync(gnuTar)) { - // Making GNUtar + zstd as default on windows + // Use GNUtar as default on windows args.push('--force-local') return gnuTar - } else if (compressionMethod !== CompressionMethod.Gzip) { + } else if ( + compressionMethod !== CompressionMethod.Gzip || + (await utils.isGnuTarInstalled()) + ) { // We only use zstandard compression on windows when gnu tar is installed due to // a bug with compressing large files with bsdtar + zstd args.push('--force-local') } else if (existsSync(systemTar)) { return systemTar - } else if (await utils.isGnuTarInstalled()) { - args.push('--force-local') } break } From 4b348086a9879d63ab98241f20231d985a4248a1 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 05:24:09 +0000 Subject: [PATCH 06/14] Add logs for cache version on miss --- packages/cache/src/internal/cacheHttpClient.ts | 18 +++++++++++++++++- packages/cache/src/internal/contracts.d.ts | 6 ++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index c66d1a73..922c2750 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -17,7 +17,8 @@ import { CommitCacheRequest, ReserveCacheRequest, ReserveCacheResponse, - ITypedResponseWithError + ITypedResponseWithError, + ArtifactCacheList } from './contracts' import {downloadCacheHttpClient, downloadCacheStorageSDK} from './downloadUtils' import { @@ -113,6 +114,21 @@ export async function getCacheEntry( const cacheResult = response.result const cacheDownloadUrl = cacheResult?.archiveLocation if (!cacheDownloadUrl) { + // List cache for primary key only if cache miss occurs + const resource = `caches?key=${encodeURIComponent(keys[0])}` + const response = await httpClient.getJson(getCacheApiUrl(resource)) + if(response.statusCode === 204) { + const cacheListResult = response.result + const totalCount = cacheListResult?.totalCount + if(totalCount && totalCount > 0) { + core.info(`Cache miss occurred on the cache key '${keys[0]}' and version '${version} but there is ${totalCount} existing version of the cache for this key. More info on versioning can be found here: https://github.com/actions/cache#cache-version`) + core.debug(`Other versions are as follows:`) + cacheListResult?.artifactCaches?.forEach(cacheEntry => { + core.debug(`Cache Key: ${cacheEntry?.cacheKey}, Cache Version: ${cacheEntry?.cacheVersion}, Cache Scope: ${cacheEntry?.scope}, Cache Created: ${cacheEntry?.creationTime}`) + }) + } + } + throw new Error('Cache not found.') } core.setSecret(cacheDownloadUrl) diff --git a/packages/cache/src/internal/contracts.d.ts b/packages/cache/src/internal/contracts.d.ts index 1b2a13a1..b5f53bdc 100644 --- a/packages/cache/src/internal/contracts.d.ts +++ b/packages/cache/src/internal/contracts.d.ts @@ -9,10 +9,16 @@ export interface ITypedResponseWithError extends TypedResponse { export interface ArtifactCacheEntry { cacheKey?: string scope?: string + cacheVersion?: string creationTime?: string archiveLocation?: string } +export interface ArtifactCacheList { + totalCount: number + artifactCaches?: ArtifactCacheEntry[] +} + export interface CommitCacheRequest { size: number } From 81cd5a5c2e456ae492770c447b5262344a5d8816 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 06:33:53 +0000 Subject: [PATCH 07/14] Revert "Add logs for cache version on miss" This reverts commit 4b348086a9879d63ab98241f20231d985a4248a1. --- packages/cache/src/internal/cacheHttpClient.ts | 18 +----------------- packages/cache/src/internal/contracts.d.ts | 6 ------ 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index 922c2750..c66d1a73 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -17,8 +17,7 @@ import { CommitCacheRequest, ReserveCacheRequest, ReserveCacheResponse, - ITypedResponseWithError, - ArtifactCacheList + ITypedResponseWithError } from './contracts' import {downloadCacheHttpClient, downloadCacheStorageSDK} from './downloadUtils' import { @@ -114,21 +113,6 @@ export async function getCacheEntry( const cacheResult = response.result const cacheDownloadUrl = cacheResult?.archiveLocation if (!cacheDownloadUrl) { - // List cache for primary key only if cache miss occurs - const resource = `caches?key=${encodeURIComponent(keys[0])}` - const response = await httpClient.getJson(getCacheApiUrl(resource)) - if(response.statusCode === 204) { - const cacheListResult = response.result - const totalCount = cacheListResult?.totalCount - if(totalCount && totalCount > 0) { - core.info(`Cache miss occurred on the cache key '${keys[0]}' and version '${version} but there is ${totalCount} existing version of the cache for this key. More info on versioning can be found here: https://github.com/actions/cache#cache-version`) - core.debug(`Other versions are as follows:`) - cacheListResult?.artifactCaches?.forEach(cacheEntry => { - core.debug(`Cache Key: ${cacheEntry?.cacheKey}, Cache Version: ${cacheEntry?.cacheVersion}, Cache Scope: ${cacheEntry?.scope}, Cache Created: ${cacheEntry?.creationTime}`) - }) - } - } - throw new Error('Cache not found.') } core.setSecret(cacheDownloadUrl) diff --git a/packages/cache/src/internal/contracts.d.ts b/packages/cache/src/internal/contracts.d.ts index b5f53bdc..1b2a13a1 100644 --- a/packages/cache/src/internal/contracts.d.ts +++ b/packages/cache/src/internal/contracts.d.ts @@ -9,16 +9,10 @@ export interface ITypedResponseWithError extends TypedResponse { export interface ArtifactCacheEntry { cacheKey?: string scope?: string - cacheVersion?: string creationTime?: string archiveLocation?: string } -export interface ArtifactCacheList { - totalCount: number - artifactCaches?: ArtifactCacheEntry[] -} - export interface CommitCacheRequest { size: number } From 7181b913f50eb47fe572fbe2518b4ac95a0d51d4 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 06:34:18 +0000 Subject: [PATCH 08/14] Remove export --- packages/cache/src/internal/cacheUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index 30538e24..da4b8d81 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -67,7 +67,7 @@ export async function unlinkFile(filePath: fs.PathLike): Promise { return util.promisify(fs.unlink)(filePath) } -export async function getVersion(app: string): Promise { +async function getVersion(app: string): Promise { core.debug(`Checking ${app} --version`) let versionOutput = '' try { From 55484166d81419b46daee58cf6f55e50851d88ae Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 08:12:15 +0000 Subject: [PATCH 09/14] Address review comments --- packages/cache/__tests__/tar.test.ts | 15 ++++++++------- packages/cache/src/internal/tar.ts | 11 ++++++----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 6983d5f7..6579cb3e 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -12,6 +12,7 @@ jest.mock('@actions/io') const IS_WINDOWS = process.platform === 'win32' const IS_MAC = process.platform === 'darwin' +const windowsGnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` const defaultTarPath = process.platform === 'darwin' ? 'gtar' : 'tar' @@ -46,7 +47,7 @@ test('zstd extract tar', async () => { : 'cache.tar' const workspace = process.env['GITHUB_WORKSPACE'] const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath await tar.extractTar(archivePath, CompressionMethod.Zstd) @@ -82,7 +83,7 @@ test('gzip extract tar', async () => { expect(mkdirMock).toHaveBeenCalledWith(workspace) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -144,7 +145,7 @@ test('zstd create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) @@ -184,7 +185,7 @@ test('gzip create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Gzip) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) @@ -221,7 +222,7 @@ test('zstd list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -249,7 +250,7 @@ test('zstdWithoutLong list tar', async () => { await tar.listTar(archivePath, CompressionMethod.ZstdWithoutLong) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -276,7 +277,7 @@ test('gzip list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Gzip) const tarPath = IS_WINDOWS - ? `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 1941afd1..fab9a925 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -11,14 +11,15 @@ async function getTarPath( args: string[], compressionMethod: CompressionMethod ): Promise { + var tarPath = await io.which('tar', true) switch (process.platform) { case 'win32': { - const systemTar = `${process.env['windir']}\\System32\\tar.exe` const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + const systemTar = `${process.env['windir']}\\System32\\tar.exe` if (existsSync(gnuTar)) { // Use GNUtar as default on windows args.push('--force-local') - return gnuTar + tarPath = gnuTar } else if ( compressionMethod !== CompressionMethod.Gzip || (await utils.isGnuTarInstalled()) @@ -27,7 +28,7 @@ async function getTarPath( // a bug with compressing large files with bsdtar + zstd args.push('--force-local') } else if (existsSync(systemTar)) { - return systemTar + tarPath = systemTar } break } @@ -36,14 +37,14 @@ async function getTarPath( 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') - return gnuTar + tarPath = gnuTar } break } default: break } - return await io.which('tar', true) + return tarPath } async function execTar( From 1ddac5e02f4e3f33925d5dbe8b619e7a4c246ce4 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 10:11:46 +0000 Subject: [PATCH 10/14] Fix tests --- packages/cache/__tests__/tar.test.ts | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 6579cb3e..bbd542a6 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -46,9 +46,7 @@ test('zstd extract tar', async () => { ? `${process.env['windir']}\\fakepath\\cache.tar` : 'cache.tar' const workspace = process.env['GITHUB_WORKSPACE'] - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath await tar.extractTar(archivePath, CompressionMethod.Zstd) @@ -82,9 +80,7 @@ test('gzip extract tar', async () => { await tar.extractTar(archivePath, CompressionMethod.Gzip) expect(mkdirMock).toHaveBeenCalledWith(workspace) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -144,9 +140,7 @@ test('zstd create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -184,9 +178,7 @@ test('gzip create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -221,9 +213,7 @@ test('zstd list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -249,9 +239,7 @@ test('zstdWithoutLong list tar', async () => { await tar.listTar(archivePath, CompressionMethod.ZstdWithoutLong) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -276,9 +264,7 @@ test('gzip list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS - ? windowsGnuTar - : defaultTarPath + const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, From 7441dc5e59f08f0b27626cb5be1c32062413c990 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 14 Nov 2022 10:25:17 +0000 Subject: [PATCH 11/14] Fix tests --- packages/cache/src/internal/tar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index fab9a925..8f548bd1 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -11,7 +11,7 @@ async function getTarPath( args: string[], compressionMethod: CompressionMethod ): Promise { - var tarPath = await io.which('tar', true) + let tarPath = await io.which('tar', true) switch (process.platform) { case 'win32': { const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` From 0e707aeabc9c3a6d1e679ff0c19ddf6cd4bd1b54 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 15 Nov 2022 08:41:04 +0000 Subject: [PATCH 12/14] Address comments --- packages/cache/__tests__/tar.test.ts | 28 ++++++++++++----------- packages/cache/src/internal/cacheUtils.ts | 18 +++++++++------ packages/cache/src/internal/constants.ts | 3 +++ packages/cache/src/internal/tar.ts | 11 ++------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index bbd542a6..61338bc3 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -1,7 +1,11 @@ import * as exec from '@actions/exec' import * as io from '@actions/io' import * as path from 'path' -import {CacheFilename, CompressionMethod} from '../src/internal/constants' +import { + CacheFilename, + CompressionMethod, + GnuTarPathOnWindows +} from '../src/internal/constants' import * as tar from '../src/internal/tar' import * as utils from '../src/internal/cacheUtils' // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -12,7 +16,6 @@ jest.mock('@actions/io') const IS_WINDOWS = process.platform === 'win32' const IS_MAC = process.platform === 'darwin' -const windowsGnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` const defaultTarPath = process.platform === 'darwin' ? 'gtar' : 'tar' @@ -46,7 +49,7 @@ test('zstd extract tar', async () => { ? `${process.env['windir']}\\fakepath\\cache.tar` : 'cache.tar' const workspace = process.env['GITHUB_WORKSPACE'] - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath await tar.extractTar(archivePath, CompressionMethod.Zstd) @@ -80,7 +83,7 @@ test('gzip extract tar', async () => { await tar.extractTar(archivePath, CompressionMethod.Gzip) expect(mkdirMock).toHaveBeenCalledWith(workspace) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -100,11 +103,10 @@ test('gzip extract tar', async () => { test('gzip extract GNU tar on windows with GNUtar in path', async () => { if (IS_WINDOWS) { - jest.spyOn(fs, 'existsSync').mockReturnValue(false) - + // GNU tar present in path but not at default location const isGnuMock = jest - .spyOn(utils, 'isGnuTarInstalled') - .mockReturnValue(Promise.resolve(true)) + .spyOn(utils, 'getGnuTarPathOnWindows') + .mockReturnValue(Promise.resolve('C:\\Program Files\\gnutar\\tar.exe')) const execMock = jest.spyOn(exec, 'exec') const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] @@ -140,7 +142,7 @@ test('zstd create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -178,7 +180,7 @@ test('gzip create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -213,7 +215,7 @@ test('zstd list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -239,7 +241,7 @@ test('zstdWithoutLong list tar', async () => { await tar.listTar(archivePath, CompressionMethod.ZstdWithoutLong) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -264,7 +266,7 @@ test('gzip list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS ? windowsGnuTar : defaultTarPath + const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index da4b8d81..64fef08a 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -7,7 +7,11 @@ import * as path from 'path' import * as semver from 'semver' import * as util from 'util' import {v4 as uuidV4} from 'uuid' -import {CacheFilename, CompressionMethod} from './constants' +import { + CacheFilename, + CompressionMethod, + GnuTarPathOnWindows +} from './constants' // From https://github.com/actions/toolkit/blob/main/packages/tool-cache/src/tool-cache.ts#L23 export async function createTempDirectory(): Promise { @@ -90,7 +94,7 @@ async function getVersion(app: string): Promise { // Use zstandard if possible to maximize cache performance export async function getCompressionMethod(): Promise { - if (process.platform === 'win32' && !(await isGnuTarInstalled())) { + if (process.platform === 'win32' && !(await getGnuTarPathOnWindows())) { // Disable zstd due to bug https://github.com/actions/cache/issues/301 return CompressionMethod.Gzip } @@ -116,12 +120,12 @@ export function getCacheFileName(compressionMethod: CompressionMethod): string { : CacheFilename.Zstd } -export async function isGnuTarInstalled(): Promise { - const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` +export async function getGnuTarPathOnWindows(): Promise { + if (fs.existsSync(GnuTarPathOnWindows)) { + return GnuTarPathOnWindows + } const versionOutput = await getVersion('tar') - return ( - versionOutput.toLowerCase().includes('gnu tar') || fs.existsSync(gnuTar) - ) + return versionOutput.toLowerCase().includes('gnu tar') ? io.which('tar') : '' } export function assertDefined(name: string, value?: T): T { diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index 2f78d326..cc58a0b6 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -21,3 +21,6 @@ export const DefaultRetryDelay = 5000 // over the socket during this period, the socket is destroyed and the download // is aborted. export const SocketTimeout = 5000 + +// The default path of GNUtar on hosted Windows runners +export const GnuTarPathOnWindows = 'C:\\Program Files\\Git\\usr\\bin\\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 8f548bd1..db2f635a 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -14,19 +14,12 @@ async function getTarPath( let tarPath = await io.which('tar', true) switch (process.platform) { case 'win32': { - const gnuTar = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` + const gnuTar = await utils.getGnuTarPathOnWindows() const systemTar = `${process.env['windir']}\\System32\\tar.exe` - if (existsSync(gnuTar)) { + if (gnuTar) { // Use GNUtar as default on windows args.push('--force-local') tarPath = gnuTar - } else if ( - compressionMethod !== CompressionMethod.Gzip || - (await utils.isGnuTarInstalled()) - ) { - // We only use zstandard compression on windows when gnu tar is installed due to - // a bug with compressing large files with bsdtar + zstd - args.push('--force-local') } else if (existsSync(systemTar)) { tarPath = systemTar } From 5a0405df4e20ef2d43a1b9b4412a788892a2c91e Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 15 Nov 2022 08:48:29 +0000 Subject: [PATCH 13/14] Fix tests --- packages/cache/__tests__/tar.test.ts | 2 +- packages/cache/src/internal/constants.ts | 2 +- packages/cache/src/internal/tar.ts | 5 +---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 61338bc3..31f9bd5e 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -106,7 +106,7 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { // GNU tar present in path but not at default location const isGnuMock = jest .spyOn(utils, 'getGnuTarPathOnWindows') - .mockReturnValue(Promise.resolve('C:\\Program Files\\gnutar\\tar.exe')) + .mockReturnValue(Promise.resolve('tar')) const execMock = jest.spyOn(exec, 'exec') const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index cc58a0b6..f4da6e6d 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -23,4 +23,4 @@ export const DefaultRetryDelay = 5000 export const SocketTimeout = 5000 // The default path of GNUtar on hosted Windows runners -export const GnuTarPathOnWindows = 'C:\\Program Files\\Git\\usr\\bin\\tar.exe' \ No newline at end of file +export const GnuTarPathOnWindows = 'C:\\Program Files\\Git\\usr\\bin\\tar.exe' diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index db2f635a..b1921613 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -7,10 +7,7 @@ import {CompressionMethod} from './constants' const IS_WINDOWS = process.platform === 'win32' -async function getTarPath( - args: string[], - compressionMethod: CompressionMethod -): Promise { +async function getTarPath(args: string[]): Promise { let tarPath = await io.which('tar', true) switch (process.platform) { case 'win32': { From 9443e26349c030a9ace11099ab35ac6c6c501e89 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 15 Nov 2022 10:18:46 +0000 Subject: [PATCH 14/14] Address review comments --- packages/cache/src/internal/constants.ts | 2 +- packages/cache/src/internal/tar.ts | 23 +++++++++-------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/cache/src/internal/constants.ts b/packages/cache/src/internal/constants.ts index f4da6e6d..e61b46f4 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -23,4 +23,4 @@ export const DefaultRetryDelay = 5000 export const SocketTimeout = 5000 // The default path of GNUtar on hosted Windows runners -export const GnuTarPathOnWindows = 'C:\\Program Files\\Git\\usr\\bin\\tar.exe' +export const GnuTarPathOnWindows = `${process.env['PROGRAMFILES']}\\Git\\usr\\bin\\tar.exe` diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index b1921613..42692008 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -8,7 +8,6 @@ import {CompressionMethod} from './constants' const IS_WINDOWS = process.platform === 'win32' async function getTarPath(args: string[]): Promise { - let tarPath = await io.which('tar', true) switch (process.platform) { case 'win32': { const gnuTar = await utils.getGnuTarPathOnWindows() @@ -16,9 +15,9 @@ async function getTarPath(args: string[]): Promise { if (gnuTar) { // Use GNUtar as default on windows args.push('--force-local') - tarPath = gnuTar + return gnuTar } else if (existsSync(systemTar)) { - tarPath = systemTar + return systemTar } break } @@ -27,23 +26,19 @@ async function getTarPath(args: string[]): Promise { 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') - tarPath = gnuTar + return gnuTar } break } default: break } - return tarPath + return await io.which('tar', true) } -async function execTar( - args: string[], - compressionMethod: CompressionMethod, - cwd?: string -): Promise { +async function execTar(args: string[], cwd?: string): Promise { try { - await exec(`"${await getTarPath(args, compressionMethod)}"`, args, {cwd}) + await exec(`"${await getTarPath(args)}"`, args, {cwd}) } catch (error) { throw new Error(`Tar failed with error: ${error?.message}`) } @@ -82,7 +77,7 @@ export async function listTar( archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' ] - await execTar(args, compressionMethod) + await execTar(args) } export async function extractTar( @@ -100,7 +95,7 @@ export async function extractTar( '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] - await execTar(args, compressionMethod) + await execTar(args) } export async function createTar( @@ -148,5 +143,5 @@ export async function createTar( '--files-from', manifestFilename ] - await execTar(args, compressionMethod, archiveFolder) + await execTar(args, archiveFolder) }