From 0e707aeabc9c3a6d1e679ff0c19ddf6cd4bd1b54 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 15 Nov 2022 08:41:04 +0000 Subject: [PATCH] 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 }