From 86102e88e9688475d5cc9f2a7120742cdc723ebc Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Thu, 17 Nov 2022 12:26:27 +0530 Subject: [PATCH] Revert "Add GNUtar as default in windows" --- packages/cache/__tests__/tar.test.ts | 56 +++++++++-------------- packages/cache/src/internal/cacheUtils.ts | 15 ++---- packages/cache/src/internal/constants.ts | 3 -- packages/cache/src/internal/tar.ts | 28 ++++++++---- 4 files changed, 44 insertions(+), 58 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 31f9bd5e..e4233bc9 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -1,11 +1,7 @@ import * as exec from '@actions/exec' import * as io from '@actions/io' import * as path from 'path' -import { - CacheFilename, - CompressionMethod, - GnuTarPathOnWindows -} from '../src/internal/constants' +import {CacheFilename, CompressionMethod} 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 @@ -32,10 +28,6 @@ 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()) @@ -49,14 +41,13 @@ test('zstd extract tar', async () => { ? `${process.env['windir']}\\fakepath\\cache.tar` : 'cache.tar' const workspace = process.env['GITHUB_WORKSPACE'] - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath await tar.extractTar(archivePath, CompressionMethod.Zstd) expect(mkdirMock).toHaveBeenCalledWith(workspace) expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, + `"${defaultTarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', @@ -83,7 +74,9 @@ test('gzip extract tar', async () => { await tar.extractTar(archivePath, CompressionMethod.Gzip) expect(mkdirMock).toHaveBeenCalledWith(workspace) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath + const tarPath = IS_WINDOWS + ? `${process.env['windir']}\\System32\\tar.exe` + : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -94,19 +87,18 @@ test('gzip extract tar', async () => { '-P', '-C', IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace - ] - .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + ].concat(IS_MAC ? ['--delay-directory-restore'] : []), {cwd: undefined} ) }) -test('gzip extract GNU tar on windows with GNUtar in path', async () => { +test('gzip extract GNU tar on windows', async () => { if (IS_WINDOWS) { - // GNU tar present in path but not at default location + jest.spyOn(fs, 'existsSync').mockReturnValueOnce(false) + const isGnuMock = jest - .spyOn(utils, 'getGnuTarPathOnWindows') - .mockReturnValue(Promise.resolve('tar')) + .spyOn(utils, 'isGnuTarInstalled') + .mockReturnValue(Promise.resolve(true)) const execMock = jest.spyOn(exec, 'exec') const archivePath = `${process.env['windir']}\\fakepath\\cache.tar` const workspace = process.env['GITHUB_WORKSPACE'] @@ -142,11 +134,9 @@ test('zstd create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath - expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, + `"${defaultTarPath}"`, [ '--posix', '--use-compress-program', @@ -180,7 +170,9 @@ test('gzip create tar', async () => { await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath + const tarPath = IS_WINDOWS + ? `${process.env['windir']}\\System32\\tar.exe` + : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( @@ -197,9 +189,7 @@ test('gzip create tar', async () => { IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace, '--files-from', 'manifest.txt' - ] - .concat(IS_WINDOWS ? ['--force-local'] : []) - .concat(IS_MAC ? ['--delay-directory-restore'] : []), + ].concat(IS_MAC ? ['--delay-directory-restore'] : []), { cwd: archiveFolder } @@ -215,10 +205,9 @@ test('zstd list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, + `"${defaultTarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30', @@ -241,10 +230,9 @@ test('zstdWithoutLong list tar', async () => { await tar.listTar(archivePath, CompressionMethod.ZstdWithoutLong) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( - `"${tarPath}"`, + `"${defaultTarPath}"`, [ '--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd', @@ -266,7 +254,9 @@ test('gzip list tar', async () => { await tar.listTar(archivePath, CompressionMethod.Gzip) - const tarPath = IS_WINDOWS ? GnuTarPathOnWindows : defaultTarPath + const tarPath = IS_WINDOWS + ? `${process.env['windir']}\\System32\\tar.exe` + : defaultTarPath expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( `"${tarPath}"`, @@ -275,9 +265,7 @@ test('gzip list tar', async () => { '-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'] : []), {cwd: undefined} ) }) diff --git a/packages/cache/src/internal/cacheUtils.ts b/packages/cache/src/internal/cacheUtils.ts index 64fef08a..c2ace526 100644 --- a/packages/cache/src/internal/cacheUtils.ts +++ b/packages/cache/src/internal/cacheUtils.ts @@ -7,11 +7,7 @@ import * as path from 'path' import * as semver from 'semver' import * as util from 'util' import {v4 as uuidV4} from 'uuid' -import { - CacheFilename, - CompressionMethod, - GnuTarPathOnWindows -} from './constants' +import {CacheFilename, CompressionMethod} from './constants' // From https://github.com/actions/toolkit/blob/main/packages/tool-cache/src/tool-cache.ts#L23 export async function createTempDirectory(): Promise { @@ -94,7 +90,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 getGnuTarPathOnWindows())) { + if (process.platform === 'win32' && !(await isGnuTarInstalled())) { // Disable zstd due to bug https://github.com/actions/cache/issues/301 return CompressionMethod.Gzip } @@ -120,12 +116,9 @@ export function getCacheFileName(compressionMethod: CompressionMethod): string { : CacheFilename.Zstd } -export async function getGnuTarPathOnWindows(): Promise { - if (fs.existsSync(GnuTarPathOnWindows)) { - return GnuTarPathOnWindows - } +export async function isGnuTarInstalled(): Promise { const versionOutput = await getVersion('tar') - return versionOutput.toLowerCase().includes('gnu tar') ? io.which('tar') : '' + return versionOutput.toLowerCase().includes('gnu 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 e61b46f4..2f78d326 100644 --- a/packages/cache/src/internal/constants.ts +++ b/packages/cache/src/internal/constants.ts @@ -21,6 +21,3 @@ 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 = `${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 42692008..2e28ca1a 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -7,17 +7,21 @@ import {CompressionMethod} from './constants' const IS_WINDOWS = process.platform === 'win32' -async function getTarPath(args: string[]): Promise { +async function getTarPath( + args: string[], + compressionMethod: CompressionMethod +): Promise { switch (process.platform) { case 'win32': { - const gnuTar = await utils.getGnuTarPathOnWindows() const systemTar = `${process.env['windir']}\\System32\\tar.exe` - if (gnuTar) { - // Use GNUtar as default on windows + 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') - return gnuTar } else if (existsSync(systemTar)) { return systemTar + } else if (await utils.isGnuTarInstalled()) { + args.push('--force-local') } break } @@ -36,9 +40,13 @@ async function getTarPath(args: string[]): Promise { return await io.which('tar', true) } -async function execTar(args: string[], cwd?: string): Promise { +async function execTar( + args: string[], + compressionMethod: CompressionMethod, + cwd?: string +): Promise { try { - await exec(`"${await getTarPath(args)}"`, args, {cwd}) + await exec(`"${await getTarPath(args, compressionMethod)}"`, args, {cwd}) } catch (error) { throw new Error(`Tar failed with error: ${error?.message}`) } @@ -77,7 +85,7 @@ export async function listTar( archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P' ] - await execTar(args) + await execTar(args, compressionMethod) } export async function extractTar( @@ -95,7 +103,7 @@ export async function extractTar( '-C', workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] - await execTar(args) + await execTar(args, compressionMethod) } export async function createTar( @@ -143,5 +151,5 @@ export async function createTar( '--files-from', manifestFilename ] - await execTar(args, archiveFolder) + await execTar(args, compressionMethod, archiveFolder) }