From 3854a40aee6b66135b974c5a63118c69b6c0d2d0 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 13 Dec 2019 17:24:37 -0500 Subject: [PATCH] Use BSD tar on windows (#126) * Use BSD tar on windows * Linting * Fallback to which tar if no system tar * Fix formatting * Bump prettier and typescript --- __tests__/restore.test.ts | 72 +++++++------------------------------ __tests__/save.test.ts | 74 ++++++++------------------------------- __tests__/tar.test.ts | 58 ++++++++++++++++++++++++++++++ package-lock.json | 12 +++---- package.json | 4 +-- src/restore.ts | 25 ++----------- src/save.ts | 22 ++---------- src/tar.ts | 47 +++++++++++++++++++++++++ 8 files changed, 143 insertions(+), 171 deletions(-) create mode 100644 __tests__/tar.test.ts create mode 100644 src/tar.ts diff --git a/__tests__/restore.test.ts b/__tests__/restore.test.ts index 78b00ec..15e0ba5 100644 --- a/__tests__/restore.test.ts +++ b/__tests__/restore.test.ts @@ -1,18 +1,16 @@ import * as core from "@actions/core"; -import * as exec from "@actions/exec"; -import * as io from "@actions/io"; import * as path from "path"; import * as cacheHttpClient from "../src/cacheHttpClient"; import { Events, Inputs } from "../src/constants"; import { ArtifactCacheEntry } from "../src/contracts"; import run from "../src/restore"; +import * as tar from "../src/tar"; import * as actionUtils from "../src/utils/actionUtils"; import * as testUtils from "../src/utils/testUtils"; -jest.mock("@actions/exec"); -jest.mock("@actions/io"); -jest.mock("../src/utils/actionUtils"); jest.mock("../src/cacheHttpClient"); +jest.mock("../src/tar"); +jest.mock("../src/utils/actionUtils"); beforeAll(() => { jest.spyOn(actionUtils, "resolvePath").mockImplementation(filePath => { @@ -35,10 +33,6 @@ beforeAll(() => { const actualUtils = jest.requireActual("../src/utils/actionUtils"); return actualUtils.getSupportedEvents(); }); - - jest.spyOn(io, "which").mockImplementation(tool => { - return Promise.resolve(tool); - }); }); beforeEach(() => { @@ -245,8 +239,7 @@ test("restore with cache found", async () => { .spyOn(actionUtils, "getArchiveFileSize") .mockReturnValue(fileSize); - const mkdirMock = jest.spyOn(io, "mkdirP"); - const execMock = jest.spyOn(exec, "exec"); + const extractTarMock = jest.spyOn(tar, "extractTar"); const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput"); await run(); @@ -257,22 +250,9 @@ test("restore with cache found", async () => { expect(createTempDirectoryMock).toHaveBeenCalledTimes(1); expect(downloadCacheMock).toHaveBeenCalledWith(cacheEntry, archivePath); expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath); - expect(mkdirMock).toHaveBeenCalledWith(cachePath); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-xz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/") - ] - : ["-xz", "-f", archivePath, "-C", cachePath]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(extractTarMock).toHaveBeenCalledTimes(1); + expect(extractTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1); expect(setCacheHitOutputMock).toHaveBeenCalledWith(true); @@ -323,8 +303,7 @@ test("restore with a pull request event and cache found", async () => { .spyOn(actionUtils, "getArchiveFileSize") .mockReturnValue(fileSize); - const mkdirMock = jest.spyOn(io, "mkdirP"); - const execMock = jest.spyOn(exec, "exec"); + const extractTarMock = jest.spyOn(tar, "extractTar"); const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput"); await run(); @@ -336,22 +315,9 @@ test("restore with a pull request event and cache found", async () => { expect(downloadCacheMock).toHaveBeenCalledWith(cacheEntry, archivePath); expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath); expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`); - expect(mkdirMock).toHaveBeenCalledWith(cachePath); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-xz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/") - ] - : ["-xz", "-f", archivePath, "-C", cachePath]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(extractTarMock).toHaveBeenCalledTimes(1); + expect(extractTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1); expect(setCacheHitOutputMock).toHaveBeenCalledWith(true); @@ -402,8 +368,7 @@ test("restore with cache found for restore key", async () => { .spyOn(actionUtils, "getArchiveFileSize") .mockReturnValue(fileSize); - const mkdirMock = jest.spyOn(io, "mkdirP"); - const execMock = jest.spyOn(exec, "exec"); + const extractTarMock = jest.spyOn(tar, "extractTar"); const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput"); await run(); @@ -415,22 +380,9 @@ test("restore with cache found for restore key", async () => { expect(downloadCacheMock).toHaveBeenCalledWith(cacheEntry, archivePath); expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath); expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`); - expect(mkdirMock).toHaveBeenCalledWith(cachePath); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-xz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/") - ] - : ["-xz", "-f", archivePath, "-C", cachePath]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(extractTarMock).toHaveBeenCalledTimes(1); + expect(extractTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1); expect(setCacheHitOutputMock).toHaveBeenCalledWith(false); diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts index 89657c4..ba76cda 100644 --- a/__tests__/save.test.ts +++ b/__tests__/save.test.ts @@ -1,19 +1,17 @@ import * as core from "@actions/core"; -import * as exec from "@actions/exec"; -import * as io from "@actions/io"; import * as path from "path"; import * as cacheHttpClient from "../src/cacheHttpClient"; import { Events, Inputs } from "../src/constants"; import { ArtifactCacheEntry } from "../src/contracts"; import run from "../src/save"; +import * as tar from "../src/tar"; import * as actionUtils from "../src/utils/actionUtils"; import * as testUtils from "../src/utils/testUtils"; jest.mock("@actions/core"); -jest.mock("@actions/exec"); -jest.mock("@actions/io"); -jest.mock("../src/utils/actionUtils"); jest.mock("../src/cacheHttpClient"); +jest.mock("../src/tar"); +jest.mock("../src/utils/actionUtils"); beforeAll(() => { jest.spyOn(core, "getInput").mockImplementation((name, options) => { @@ -49,10 +47,6 @@ beforeAll(() => { jest.spyOn(actionUtils, "createTempDirectory").mockImplementation(() => { return Promise.resolve("/foo/bar"); }); - - jest.spyOn(io, "which").mockImplementation(tool => { - return Promise.resolve(tool); - }); }); beforeEach(() => { @@ -128,7 +122,7 @@ test("save with exact match returns early", async () => { return primaryKey; }); - const execMock = jest.spyOn(exec, "exec"); + const createTarMock = jest.spyOn(tar, "createTar"); await run(); @@ -136,7 +130,7 @@ test("save with exact match returns early", async () => { `Cache hit occurred on the primary key ${primaryKey}, not saving cache.` ); - expect(execMock).toHaveBeenCalledTimes(0); + expect(createTarMock).toHaveBeenCalledTimes(0); expect(failedMock).toHaveBeenCalledTimes(0); }); @@ -198,7 +192,7 @@ test("save with large cache outputs warning", async () => { const cachePath = path.resolve(inputPath); testUtils.setInput(Inputs.Path, inputPath); - const execMock = jest.spyOn(exec, "exec"); + const createTarMock = jest.spyOn(tar, "createTar"); const cacheSize = 1024 * 1024 * 1024; //~1GB, over the 400MB limit jest.spyOn(actionUtils, "getArchiveFileSize").mockImplementationOnce(() => { @@ -209,21 +203,8 @@ test("save with large cache outputs warning", async () => { const archivePath = path.join("/foo/bar", "cache.tgz"); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-cz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/"), - "." - ] - : ["-cz", "-f", archivePath, "-C", cachePath, "."]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(createTarMock).toHaveBeenCalledTimes(1); + expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(logWarningMock).toHaveBeenCalledTimes(1); expect(logWarningMock).toHaveBeenCalledWith( @@ -259,7 +240,7 @@ test("save with server error outputs warning", async () => { const cachePath = path.resolve(inputPath); testUtils.setInput(Inputs.Path, inputPath); - const execMock = jest.spyOn(exec, "exec"); + const createTarMock = jest.spyOn(tar, "createTar"); const saveCacheMock = jest .spyOn(cacheHttpClient, "saveCache") @@ -271,21 +252,8 @@ test("save with server error outputs warning", async () => { const archivePath = path.join("/foo/bar", "cache.tgz"); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-cz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/"), - "." - ] - : ["-cz", "-f", archivePath, "-C", cachePath, "."]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(createTarMock).toHaveBeenCalledTimes(1); + expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(saveCacheMock).toHaveBeenCalledTimes(1); expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath); @@ -321,29 +289,15 @@ test("save with valid inputs uploads a cache", async () => { const cachePath = path.resolve(inputPath); testUtils.setInput(Inputs.Path, inputPath); - const execMock = jest.spyOn(exec, "exec"); - + const createTarMock = jest.spyOn(tar, "createTar"); const saveCacheMock = jest.spyOn(cacheHttpClient, "saveCache"); await run(); const archivePath = path.join("/foo/bar", "cache.tgz"); - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-cz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/"), - "." - ] - : ["-cz", "-f", archivePath, "-C", cachePath, "."]; - - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + expect(createTarMock).toHaveBeenCalledTimes(1); + expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePath); expect(saveCacheMock).toHaveBeenCalledTimes(1); expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath); diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts new file mode 100644 index 0000000..55ff4c7 --- /dev/null +++ b/__tests__/tar.test.ts @@ -0,0 +1,58 @@ +import * as exec from "@actions/exec"; +import * as io from "@actions/io"; +import * as tar from "../src/tar"; + +jest.mock("@actions/exec"); +jest.mock("@actions/io"); + +beforeAll(() => { + jest.spyOn(io, "which").mockImplementation(tool => { + return Promise.resolve(tool); + }); +}); + +test("extract tar", async () => { + const mkdirMock = jest.spyOn(io, "mkdirP"); + const execMock = jest.spyOn(exec, "exec"); + + const archivePath = "cache.tar"; + const targetDirectory = "~/.npm/cache"; + await tar.extractTar(archivePath, targetDirectory); + + expect(mkdirMock).toHaveBeenCalledWith(targetDirectory); + + const IS_WINDOWS = process.platform === "win32"; + const tarPath = IS_WINDOWS + ? `${process.env["windir"]}\\System32\\tar.exe` + : "tar"; + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"${tarPath}"`, [ + "-xz", + "-f", + archivePath, + "-C", + targetDirectory + ]); +}); + +test("create tar", async () => { + const execMock = jest.spyOn(exec, "exec"); + + const archivePath = "cache.tar"; + const sourceDirectory = "~/.npm/cache"; + await tar.createTar(archivePath, sourceDirectory); + + const IS_WINDOWS = process.platform === "win32"; + const tarPath = IS_WINDOWS + ? `${process.env["windir"]}\\System32\\tar.exe` + : "tar"; + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"${tarPath}"`, [ + "-cz", + "-f", + archivePath, + "-C", + sourceDirectory, + "." + ]); +}); diff --git a/package-lock.json b/package-lock.json index 2e8413e..986b08b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4859,9 +4859,9 @@ "dev": true }, "prettier": { - "version": "1.18.2", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-1.18.2.tgz", - "integrity": "sha512-OeHeMc0JhFE9idD4ZdtNibzY0+TPHSpSSb9h8FqtP+YnoZZ1sl8Vc9b1sasjfymH3SonAF4QcA2+mzHPhMvIiw==", + "version": "1.19.1", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-1.19.1.tgz", + "integrity": "sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew==", "dev": true }, "prettier-linter-helpers": { @@ -5983,9 +5983,9 @@ } }, "typescript": { - "version": "3.6.4", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.6.4.tgz", - "integrity": "sha512-unoCll1+l+YK4i4F8f22TaNVPRHcD9PA3yCuZ8g5e0qGqlVlJ/8FSateOLLSagn+Yg5+ZwuPkL8LFUc0Jcvksg==", + "version": "3.7.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.3.tgz", + "integrity": "sha512-Mcr/Qk7hXqFBXMN7p7Lusj1ktCBydylfQM/FZCk5glCNQJrCUKPkMHdo9R0MTFWsC/4kPFvDS0fDPvukfCkFsw==", "dev": true }, "uglify-js": { diff --git a/package.json b/package.json index 42fbdbe..facc0af 100644 --- a/package.json +++ b/package.json @@ -46,8 +46,8 @@ "jest": "^24.8.0", "jest-circus": "^24.7.1", "nock": "^11.7.0", - "prettier": "1.18.2", + "prettier": "^1.19.1", "ts-jest": "^24.0.2", - "typescript": "^3.6.4" + "typescript": "^3.7.3" } } diff --git a/src/restore.ts b/src/restore.ts index 15570cd..599dbd7 100644 --- a/src/restore.ts +++ b/src/restore.ts @@ -1,9 +1,8 @@ import * as core from "@actions/core"; -import { exec } from "@actions/exec"; -import * as io from "@actions/io"; import * as path from "path"; import * as cacheHttpClient from "./cacheHttpClient"; import { Events, Inputs, State } from "./constants"; +import { extractTar } from "./tar"; import * as utils from "./utils/actionUtils"; async function run(): Promise { @@ -87,27 +86,7 @@ async function run(): Promise { )} MB (${archiveFileSize} B)` ); - // Create directory to extract tar into - await io.mkdirP(cachePath); - - // http://man7.org/linux/man-pages/man1/tar.1.html - // tar [-options] [files or directories which to add into archive] - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-xz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/") - ] - : ["-xz", "-f", archivePath, "-C", cachePath]; - - const tarPath = await io.which("tar", true); - core.debug(`Tar Path: ${tarPath}`); - - await exec(`"${tarPath}"`, args); + await extractTar(archivePath, cachePath); const isExactKeyMatch = utils.isExactKeyMatch( primaryKey, diff --git a/src/save.ts b/src/save.ts index 21f32d3..56198a7 100644 --- a/src/save.ts +++ b/src/save.ts @@ -1,9 +1,8 @@ import * as core from "@actions/core"; -import { exec } from "@actions/exec"; -import * as io from "@actions/io"; import * as path from "path"; import * as cacheHttpClient from "./cacheHttpClient"; import { Events, Inputs, State } from "./constants"; +import { createTar } from "./tar"; import * as utils from "./utils/actionUtils"; async function run(): Promise { @@ -46,24 +45,7 @@ async function run(): Promise { ); core.debug(`Archive Path: ${archivePath}`); - // http://man7.org/linux/man-pages/man1/tar.1.html - // tar [-options] [files or directories which to add into archive] - const IS_WINDOWS = process.platform === "win32"; - const args = IS_WINDOWS - ? [ - "-cz", - "--force-local", - "-f", - archivePath.replace(/\\/g, "/"), - "-C", - cachePath.replace(/\\/g, "/"), - "." - ] - : ["-cz", "-f", archivePath, "-C", cachePath, "."]; - - const tarPath = await io.which("tar", true); - core.debug(`Tar Path: ${tarPath}`); - await exec(`"${tarPath}"`, args); + await createTar(archivePath, cachePath); const fileSizeLimit = 400 * 1024 * 1024; // 400MB const archiveFileSize = utils.getArchiveFileSize(archivePath); diff --git a/src/tar.ts b/src/tar.ts new file mode 100644 index 0000000..1f572d1 --- /dev/null +++ b/src/tar.ts @@ -0,0 +1,47 @@ +import { exec } from "@actions/exec"; +import * as io from "@actions/io"; +import { existsSync } from "fs"; + +async function getTarPath(): Promise { + // Explicitly use BSD Tar on Windows + const IS_WINDOWS = process.platform === "win32"; + if (IS_WINDOWS) { + const systemTar = `${process.env["windir"]}\\System32\\tar.exe`; + if (existsSync(systemTar)) { + return systemTar; + } + } + return await io.which("tar", true); +} + +async function execTar(args: string[]): Promise { + try { + await exec(`"${await getTarPath()}"`, args); + } catch (error) { + const IS_WINDOWS = process.platform === "win32"; + if (IS_WINDOWS) { + throw new Error( + `Tar failed with error: ${error?.message}. Ensure BSD tar is installed and on the PATH.` + ); + } + throw new Error(`Tar failed with error: ${error?.message}`); + } +} + +export async function extractTar( + archivePath: string, + targetDirectory: string +): Promise { + // Create directory to extract tar into + await io.mkdirP(targetDirectory); + const args = ["-xz", "-f", archivePath, "-C", targetDirectory]; + await execTar(args); +} + +export async function createTar( + archivePath: string, + sourceDirectory: string +): Promise { + const args = ["-cz", "-f", archivePath, "-C", sourceDirectory, "."]; + await execTar(args); +}