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.

Sign up for GitHub

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

Jump to bottom

MATLAB Cache fixes #107

Merged
merged 5 commits into from
Apr 11, 2024
Merged

MATLAB Cache fixes #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a81a7be
Cache fix (#106)
davidbuzinski Apr 10, 2024
0f3e3a2
Update matlab.ts
davidbuzinski Apr 10, 2024
9dc6a9f
Update matlab.ts
davidbuzinski Apr 10, 2024
6a7b6fc
fix cruft problems on windows and unchange var names
davidbuzinski Apr 11, 2024
1a1d13b
Update src/matlab.ts
davidbuzinski Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/cache-save.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export async function cacheMATLAB() {
return;
}

await cache.saveCache([matlabPath, supportPackagesPath], primaryKey);
core.info(`Cache saved with the key: ${primaryKey}`);
try {
await cache.saveCache([matlabPath, supportPackagesPath], primaryKey);
core.info(`Cache saved with the key: ${primaryKey}`);
} catch (e) {
core.warning(`Failed to save MATLAB to cache: ${e}`);
}
}
2 changes: 1 addition & 1 deletion src/install.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export async function install(platform: string, architecture: string, release: s
);

await core.group("Setting up MATLAB", async () => {
let [destination, alreadyExists]: [string, boolean] = await matlab.makeToolcacheDir(releaseInfo, platform);
let [destination, alreadyExists]: [string, boolean] = await matlab.getToolcacheDir(platform, releaseInfo);
let cacheHit = false;

if (useCache) {
Expand Down
8 changes: 4 additions & 4 deletions src/install.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ afterEach(() => {
describe("install procedure", () => {
let matlabInstallSystemDependenciesMock: jest.Mock;
let matlabGetReleaseInfoMock: jest.Mock;
let matlabMakeToolcacheDirMock: jest.Mock;
let matlabGetToolcacheDirMock: jest.Mock;
let matlabSetupBatchMock: jest.Mock;
let mpmSetupMock: jest.Mock;
let mpmInstallMock: jest.Mock;
Expand All @@ -42,7 +42,7 @@ describe("install procedure", () => {
beforeEach(() => {
matlabInstallSystemDependenciesMock = matlab.installSystemDependencies as jest.Mock;
matlabGetReleaseInfoMock = matlab.getReleaseInfo as jest.Mock;
matlabMakeToolcacheDirMock = matlab.makeToolcacheDir as jest.Mock;
matlabGetToolcacheDirMock = matlab.getToolcacheDir as jest.Mock;
matlabSetupBatchMock = matlab.setupBatch as jest.Mock;
mpmSetupMock = mpm.setup as jest.Mock;
mpmInstallMock = mpm.install as jest.Mock;
Expand All @@ -56,7 +56,7 @@ describe("install procedure", () => {
return func();
});
matlabGetReleaseInfoMock.mockResolvedValue(releaseInfo);
matlabMakeToolcacheDirMock.mockResolvedValue(["/opt/hostedtoolcache/MATLAB/9.13.0/x64", false]);
matlabGetToolcacheDirMock.mockResolvedValue(["/opt/hostedtoolcache/MATLAB/9.13.0/x64", false]);
});

it("ideally works", async () => {
Expand All @@ -70,7 +70,7 @@ describe("install procedure", () => {
});

it("NoOp on existing install", async () => {
matlabMakeToolcacheDirMock.mockResolvedValue(["/opt/hostedtoolcache/MATLAB/9.13.0/x64", true]);
matlabGetToolcacheDirMock.mockResolvedValue(["/opt/hostedtoolcache/MATLAB/9.13.0/x64", true]);
await expect(doInstall()).resolves.toBeUndefined();
expect(mpmInstallMock).toHaveBeenCalledTimes(0);
expect(addPathMock).toHaveBeenCalledTimes(1);
Expand Down
74 changes: 44 additions & 30 deletions src/matlab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,33 @@ export interface Release {
isPrerelease: boolean;
}

export async function makeToolcacheDir(release: Release, platform: string): Promise<[string, boolean]> {
export async function getToolcacheDir(platform: string, release: Release): Promise<[string, boolean]> {
let toolpath: string = tc.find("MATLAB", release.version);
let alreadyExists = false;
if (toolpath) {
core.info(`Found MATLAB ${release.name} in cache at ${toolpath}.`);
alreadyExists = true;
} else {
if (platform === "win32") {
toolpath = await windowsHostedToolpath(release).catch(async () => {
return await defaultToolpath(release, platform);
});
} else {
toolpath = await defaultToolpath(release, platform);
}
toolpath = await makeToolcacheDir(platform, release);
}
if (platform == "darwin") {
toolpath = toolpath + "/MATLAB.app";
}
return [toolpath, alreadyExists]
}

async function windowsHostedToolpath(release: Release): Promise<string> {
async function makeToolcacheDir(platform: string, release: Release): Promise<string> {
let toolcacheDir: string;
if (platform === "win32") {
toolcacheDir = await makeWindowsHostedToolpath(release)
.catch(async () => await makeDefaultToolpath(release));
} else {
toolcacheDir = await makeDefaultToolpath(release);
}
return toolcacheDir;
}

async function makeWindowsHostedToolpath(release: Release): Promise<string> {
// bail early if not on a github hosted runner
if (process.env['RUNNER_ENVIRONMENT'] !== 'github-hosted' && process.env['AGENT_ISSELFHOSTED'] === '1') {
return Promise.reject();
Expand All @@ -55,30 +63,36 @@ async function windowsHostedToolpath(release: Release): Promise<string> {
const actualToolCacheRoot = defaultToolCacheRoot.replace("C:", "D:").replace("c:", "d:");
process.env['RUNNER_TOOL_CACHE'] = actualToolCacheRoot;

// create install directory and link it to the toolcache directory
fs.writeFileSync(".keep", "");
let actualToolCacheDir = await tc.cacheFile(".keep", ".keep", "MATLAB", release.version);
io.rmRF(".keep");
let defaultToolCacheDir = actualToolCacheDir.replace(actualToolCacheRoot, defaultToolCacheRoot);
fs.mkdirSync(path.dirname(defaultToolCacheDir), {recursive: true});
fs.symlinkSync(actualToolCacheDir, defaultToolCacheDir, 'junction');

// required for github actions to make the cacheDir persistent
const actualToolCacheCompleteFile = `${actualToolCacheDir}.complete`;
const defaultToolCacheCompleteFile = `${defaultToolCacheDir}.complete`;
fs.symlinkSync(actualToolCacheCompleteFile, defaultToolCacheCompleteFile, 'file');

process.env['RUNNER_TOOL_CACHE'] = defaultToolCacheRoot;
return actualToolCacheDir;
try {
// create install directory and link it to the toolcache directory
fs.writeFileSync(".keep", "");
let actualToolCacheDir = await tc.cacheFile(".keep", ".keep", "MATLAB", release.version);
await io.rmRF(".keep");
let defaultToolCacheDir = actualToolCacheDir.replace(actualToolCacheRoot, defaultToolCacheRoot);

// remove cruft from incomplete installs
await io.rmRF(defaultToolCacheDir);

// link to actual tool cache directory
fs.mkdirSync(path.dirname(defaultToolCacheDir), {recursive: true});
fs.symlinkSync(actualToolCacheDir, defaultToolCacheDir, 'junction');

// .complete file is required for github actions to make the cacheDir persistent
const actualToolCacheCompleteFile = `${actualToolCacheDir}.complete`;
const defaultToolCacheCompleteFile = `${defaultToolCacheDir}.complete`;
await io.rmRF(defaultToolCacheCompleteFile);
fs.symlinkSync(actualToolCacheCompleteFile, defaultToolCacheCompleteFile, 'file');

return actualToolCacheDir;
} finally {
process.env['RUNNER_TOOL_CACHE'] = defaultToolCacheRoot;
Copy link
Member

@mcafaro mcafaro Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why is this env variable changed above? I don't see it being used after it is change.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess it's used by tc.cacheFile behind the scenes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used inside GitHub's tc.cacheFile code (and some of the other tool-cache functions) to locate the tool cache directory on the runner. We set it to the same location on D: to create/ link the install directory then we restore it to it's default C: location at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I guess it's used by tc.cacheFile behind the scenes?

Yeah exactly

}
}

async function defaultToolpath(release: Release, platform: string): Promise<string> {
async function makeDefaultToolpath(release: Release): Promise<string> {
fs.writeFileSync(".keep", "");
let toolpath = await tc.cacheFile(".keep", ".keep", "MATLAB", release.version);
io.rmRF(".keep");
if (platform == "darwin") {
toolpath = toolpath + "/MATLAB.app";
}
await io.rmRF(".keep");
return toolpath
}

Expand Down Expand Up @@ -199,7 +213,7 @@ export async function installSystemDependencies(platform: string, architecture:

async function installAppleSiliconJdk() {
const jdkPath = path.join(process.env["RUNNER_TEMP"] ?? "", "jdk.pkg");
io.rmRF(jdkPath);
await io.rmRF(jdkPath);
const jdk = await tc.downloadTool(properties.appleSiliconJdkUrl, jdkPath);
const exitCode = await exec.exec(`sudo installer -pkg "${jdk}" -target /`);
if (exitCode !== 0) {
Expand Down
16 changes: 8 additions & 8 deletions src/matlab.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,20 @@ describe("matlab tests", () => {

it("returns toolpath if in toolcache", async () => {
findMock.mockReturnValue("/opt/hostedtoolcache/matlab/r2022b");
await expect(matlab.makeToolcacheDir(release, platform)).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b", true]);
await expect(matlab.getToolcacheDir(platform, release)).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b", true]);
expect(infoMock).toHaveBeenCalledTimes(1);
});

it("creates cache and returns default path for linux", async () => {
findMock.mockReturnValue("");
cacheFileMock.mockReturnValue("/opt/hostedtoolcache/matlab/r2022b");
await expect(matlab.makeToolcacheDir(release, platform)).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b", false]);
await expect(matlab.getToolcacheDir(platform, release)).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b", false]);
});

it("creates cache and returns default path for mac", async () => {
findMock.mockReturnValue("");
cacheFileMock.mockReturnValue("/opt/hostedtoolcache/matlab/r2022b");
await expect(matlab.makeToolcacheDir(release, "darwin")).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b/MATLAB.app", false]);
await expect(matlab.getToolcacheDir("darwin", release)).resolves.toMatchObject(["/opt/hostedtoolcache/matlab/r2022b/MATLAB.app", false]);
});

describe("windows performance workaround", () => {
Expand Down Expand Up @@ -94,7 +94,7 @@ describe("matlab tests", () => {
let mkdirSyncSpy = jest.spyOn(fs, "mkdirSync").mockImplementation(() => "");
let symlinkSyncSpy = jest.spyOn(fs, "symlinkSync").mockImplementation(() => {});

await expect(matlab.makeToolcacheDir(release, "win32")).resolves.toMatchObject([expectedToolcacheDir, false]);
await expect(matlab.getToolcacheDir("win32", release)).resolves.toMatchObject([expectedToolcacheDir, false]);
expect(existsSyncSpy).toHaveBeenCalledTimes(2);
expect(mkdirSyncSpy).toHaveBeenCalledTimes(1);
expect(symlinkSyncSpy).toHaveBeenCalledTimes(2);
Expand All @@ -104,26 +104,26 @@ describe("matlab tests", () => {
let expectedToolcacheDir = "C:\\hostedtoolcache\\windows\\matlab\\r2022b";
process.env["AGENT_ISSELFHOSTED"] = "1";
process.env["RUNNER_ENVIRONMENT"] = "self-hosted";
await expect(matlab.makeToolcacheDir(release, "win32")).resolves.toMatchObject([expectedToolcacheDir, false]);
await expect(matlab.getToolcacheDir("win32", release)).resolves.toMatchObject([expectedToolcacheDir, false]);
});

it("uses default toolcache directory toolcache directory is not defined", async () => {
let expectedToolcacheDir = "C:\\hostedtoolcache\\windows\\matlab\\r2022b";
process.env["RUNNER_TOOL_CACHE"] = '';
cacheFileMock.mockReturnValue(expectedToolcacheDir);
await expect(matlab.makeToolcacheDir(release, "win32")).resolves.toMatchObject([expectedToolcacheDir, false]);
await expect(matlab.getToolcacheDir("win32", release)).resolves.toMatchObject([expectedToolcacheDir, false]);
});

it("uses default toolcache directory if d: drive doesn't exist", async () => {
jest.spyOn(fs, "existsSync").mockReturnValue(false);
let expectedToolcacheDir = "C:\\hostedtoolcache\\windows\\matlab\\r2022b";
await expect(matlab.makeToolcacheDir(release, "win32")).resolves.toMatchObject([expectedToolcacheDir, false]);
await expect(matlab.getToolcacheDir("win32", release)).resolves.toMatchObject([expectedToolcacheDir, false]);
});

it("uses default toolcache directory if c: drive doesn't exist", async () => {
jest.spyOn(fs, "existsSync").mockReturnValueOnce(true).mockReturnValue(false);
let expectedToolcacheDir = "C:\\hostedtoolcache\\windows\\matlab\\r2022b";
await expect(matlab.makeToolcacheDir(release, "win32")).resolves.toMatchObject([expectedToolcacheDir, false]);
await expect(matlab.getToolcacheDir("win32", release)).resolves.toMatchObject([expectedToolcacheDir, false]);

});
});
Expand Down
Loading