From 7352daed7837304805de32c1bdb83561fa01c93c Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Sat, 7 Dec 2019 23:00:48 -0500 Subject: [PATCH] Use BSD tar on windows --- __tests__/restore.test.ts | 72 +++++++------------------------------ __tests__/save.test.ts | 74 ++++++++------------------------------- __tests__/tar.test.ts | 60 +++++++++++++++++++++++++++++++ src/restore.ts | 25 ++----------- src/save.ts | 22 ++---------- src/tar.ts | 27 ++++++++++++++ 6 files changed, 117 insertions(+), 163 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..0f7bd18 --- /dev/null +++ b/__tests__/tar.test.ts @@ -0,0 +1,60 @@ +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(() => { + process.env["windir"] = "C:"; + + jest.spyOn(io, "which").mockImplementation(tool => { + return Promise.resolve(tool); + }); +}); + +afterAll(() => { + delete process.env["windir"]; +}); + +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 ? "C:\\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 ? "C:\\System32\\tar.exe" : "tar"; + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"${tarPath}"`, [ + "-cz", + "-f", + archivePath, + "-C", + sourceDirectory, + "." + ]); +}); 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..26039b8 --- /dev/null +++ b/src/tar.ts @@ -0,0 +1,27 @@ +import { exec } from "@actions/exec"; +import * as io from "@actions/io"; + +export async function extractTar(archivePath: string, targetDirectory: string) { + // Create directory to extract tar into + await io.mkdirP(targetDirectory); + + // http://man7.org/linux/man-pages/man1/tar.1.html + // tar [-options] [files or directories which to add into archive] + const args = ["-xz", "-f", archivePath, "-C", targetDirectory]; + await exec(`"${await getTarPath()}"`, args); +} + +export async function createTar(archivePath: string, sourceDirectory: string) { + // http://man7.org/linux/man-pages/man1/tar.1.html + // tar [-options] [files or directories which to add into archive] + const args = ["-cz", "-f", archivePath, "-C", sourceDirectory, "."]; + await exec(`"${await getTarPath()}"`, args); +} + +async function getTarPath(): Promise { + // Explicitly use BSD Tar on Windows + const IS_WINDOWS = process.platform === "win32"; + return IS_WINDOWS + ? `${process.env["windir"]}\\System32\\tar.exe` + : await io.which("tar", true); +}