Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix two issues related to using zstd compression #469

Merged
merged 5 commits into from May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/cache/RELEASES.md
Expand Up @@ -2,4 +2,7 @@

### 0.1.0

- Initial release
- Initial release

### 0.2.0
- Fixes two issues around using zstd compression algorithm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Fixes two issues around using zstd compression algorithm
- Fixes issues with the zstd compression algorithm on Windows and Ubuntu 16.04 [#469](https://github.com/actions/toolkit/pull/469)

We might want to link the PRs in the RELEASES notes to help with discoverability

15 changes: 5 additions & 10 deletions packages/cache/__tests__/tar.test.ts
Expand Up @@ -38,13 +38,11 @@ test('zstd extract tar', async () => {
? `${process.env['windir']}\\fakepath\\cache.tar`
: 'cache.tar'
const workspace = process.env['GITHUB_WORKSPACE']
const tarPath = 'tar'

await tar.extractTar(archivePath, CompressionMethod.Zstd)

expect(mkdirMock).toHaveBeenCalledWith(workspace)
const tarPath = IS_WINDOWS
? `${process.env['windir']}\\System32\\tar.exe`
: 'tar'
expect(execMock).toHaveBeenCalledTimes(1)
expect(execMock).toHaveBeenCalledWith(
`"${tarPath}"`,
Expand All @@ -56,7 +54,7 @@ test('zstd extract tar', async () => {
'-P',
'-C',
IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace
],
].concat(IS_WINDOWS ? ['--force-local'] : []),
{cwd: undefined}
)
})
Expand Down Expand Up @@ -95,7 +93,7 @@ test('gzip extract GNU tar on windows', async () => {
jest.spyOn(fs, 'existsSync').mockReturnValueOnce(false)

const isGnuMock = jest
.spyOn(utils, 'useGnuTar')
.spyOn(utils, 'isGnuTarInstalled')
.mockReturnValue(Promise.resolve(true))
const execMock = jest.spyOn(exec, 'exec')
const archivePath = `${process.env['windir']}\\fakepath\\cache.tar`
Expand Down Expand Up @@ -127,15 +125,12 @@ test('zstd create tar', async () => {
const archiveFolder = getTempDir()
const workspace = process.env['GITHUB_WORKSPACE']
const sourceDirectories = ['~/.npm/cache', `${workspace}/dist`]
const tarPath = 'tar'

await fs.promises.mkdir(archiveFolder, {recursive: true})

await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd)

const tarPath = IS_WINDOWS
? `${process.env['windir']}\\System32\\tar.exe`
: 'tar'

expect(execMock).toHaveBeenCalledTimes(1)
expect(execMock).toHaveBeenCalledWith(
`"${tarPath}"`,
Expand All @@ -149,7 +144,7 @@ test('zstd create tar', async () => {
IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace,
'--files-from',
'manifest.txt'
],
].concat(IS_WINDOWS ? ['--force-local'] : []),
{
cwd: archiveFolder
}
Expand Down
191 changes: 101 additions & 90 deletions packages/cache/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/cache/package.json
@@ -1,6 +1,6 @@
{
"name": "@actions/cache",
"version": "0.1.0",
"version": "0.2.0",
"preview": true,
"description": "Actions cache lib",
"keywords": [
Expand Down Expand Up @@ -41,10 +41,12 @@
"@actions/glob": "^0.1.0",
"@actions/http-client": "^1.0.8",
"@actions/io": "^1.0.1",
"semver": "^6.1.0",
"uuid": "^3.3.3"
},
"devDependencies": {
"typescript": "^3.8.3",
"@types/semver": "^6.0.0",
"@types/uuid": "^3.4.5"
}
}
4 changes: 3 additions & 1 deletion packages/cache/src/internal/cacheHttpClient.ts
Expand Up @@ -96,7 +96,9 @@ export function getCacheVersion(
compressionMethod?: CompressionMethod
): string {
const components = paths.concat(
compressionMethod === CompressionMethod.Zstd ? [compressionMethod] : []
!compressionMethod || compressionMethod === CompressionMethod.Gzip
? []
: [compressionMethod]
)

// Add salt to cache version to support breaking changes in cache entry
Expand Down
29 changes: 22 additions & 7 deletions packages/cache/src/internal/cacheUtils.ts
Expand Up @@ -4,6 +4,7 @@ import * as glob from '@actions/glob'
import * as io from '@actions/io'
import * as fs from 'fs'
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'
Expand Down Expand Up @@ -82,19 +83,33 @@ async function getVersion(app: string): Promise<string> {

// Use zstandard if possible to maximize cache performance
export async function getCompressionMethod(): Promise<CompressionMethod> {
if (process.platform === 'win32' && !isGnuTarInstalled()) {
aiqiaoy marked this conversation as resolved.
Show resolved Hide resolved
// Disable zstd due to bug https://github.com/actions/cache/issues/301
return CompressionMethod.Gzip
}

const versionOutput = await getVersion('zstd')
return versionOutput.toLowerCase().includes('zstd command line interface')
? CompressionMethod.Zstd
: CompressionMethod.Gzip
const version = semver.clean(versionOutput)

if (!versionOutput.toLowerCase().includes('zstd command line interface')) {
// zstd is not installed
return CompressionMethod.Gzip
} else if (!version || semver.lt(version, 'v1.3.2')) {
// zstd is installed but using a version earlier than v1.3.2
// v1.3.2 is required to use the `--long` options in zstd
return CompressionMethod.ZstdWithoutLong
} else {
return CompressionMethod.Zstd
}
}

export function getCacheFileName(compressionMethod: CompressionMethod): string {
return compressionMethod === CompressionMethod.Zstd
? CacheFilename.Zstd
: CacheFilename.Gzip
return compressionMethod === CompressionMethod.Gzip
? CacheFilename.Gzip
: CacheFilename.Zstd
}

export async function useGnuTar(): Promise<boolean> {
export async function isGnuTarInstalled(): Promise<boolean> {
const versionOutput = await getVersion('tar')
return versionOutput.toLowerCase().includes('gnu tar')
}
3 changes: 3 additions & 0 deletions packages/cache/src/internal/constants.ts
Expand Up @@ -5,6 +5,9 @@ export enum CacheFilename {

export enum CompressionMethod {
Gzip = 'gzip',
// Long range mode was added to zstd in v1.3.2.
// This enum is for earlier version of zstd that does not have --long support
ZstdWithoutLong = 'zstd-without-long',
Zstd = 'zstd'
}

Expand Down