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

Detect cached folders from multiple directories #735

Merged
merged 21 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
review updates second stage
  • Loading branch information
dsame committed Jun 14, 2023
commit 3ef527264ac68164b301e0b46fb15d0472df423a
2 changes: 1 addition & 1 deletion __tests__/cache-restore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('cache-restore', () => {
await restoreCache(packageManager, '');
expect(hashFilesSpy).toHaveBeenCalled();
expect(infoSpy).toHaveBeenCalledWith(
`Cache restored from key: node-cache-${platform}-${packageManager}-v2-${fileHash}`
`Cache restored from key: node-cache-${platform}-${packageManager}-${fileHash}`
);
expect(infoSpy).not.toHaveBeenCalledWith(
`${packageManager} cache is not found`
Expand Down
27 changes: 10 additions & 17 deletions __tests__/cache-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import {
PackageManagerInfo,
isCacheFeatureAvailable,
supportedPackageManagers,
getCommandOutput,
memoizedCacheDependencies
getCommandOutput
} from '../src/cache-utils';
import fs from 'fs';
import * as cacheUtils from '../src/cache-utils';
Expand Down Expand Up @@ -104,10 +103,6 @@ describe('cache-utils', () => {
(pattern: string): Promise<Globber> =>
MockGlobber.create(['/foo', '/bar'])
);

Object.keys(memoizedCacheDependencies).forEach(
key => delete memoizedCacheDependencies[key]
);
});

afterEach(() => {
Expand Down Expand Up @@ -175,25 +170,23 @@ describe('cache-utils', () => {
);

it.each([
[supportedPackageManagers.npm, ''],
[supportedPackageManagers.npm, '/dir/file.lock'],
[supportedPackageManagers.npm, '/**/file.lock'],
[supportedPackageManagers.pnpm, ''],
[supportedPackageManagers.pnpm, '/dir/file.lock'],
[supportedPackageManagers.pnpm, '/**/file.lock'],
[supportedPackageManagers.yarn, ''],
[supportedPackageManagers.yarn, '/dir/file.lock'],
[supportedPackageManagers.yarn, '/**/file.lock']
])(
'getCacheDirectoriesPaths should throw in case of having not directories',
'getCacheDirectoriesPaths should nothrow in case of having not directories',
async (packageManagerInfo, cacheDependency) => {
lstatSpy.mockImplementation(arg => ({
isDirectory: () => false
}));

await expect(
cacheUtils.getCacheDirectories(packageManagerInfo, cacheDependency)
).rejects.toThrow(); //'Could not get cache folder path for /dir');
await cacheUtils.getCacheDirectories(
packageManagerInfo,
cacheDependency
);
expect(warningSpy).toHaveBeenCalledTimes(1);
expect(warningSpy).toHaveBeenCalledWith(
`No existing directories found containing cache-dependency-path="${cacheDependency}"`
);
}
);

Expand Down
25 changes: 4 additions & 21 deletions dist/cache-save/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60434,7 +60434,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.memoizedCacheDependencies = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
const core = __importStar(__nccwpck_require__(2186));
const exec = __importStar(__nccwpck_require__(1514));
const cache = __importStar(__nccwpck_require__(7799));
Expand Down Expand Up @@ -60503,11 +60503,6 @@ const getPackageManagerInfo = (packageManager) => __awaiter(void 0, void 0, void
}
});
exports.getPackageManagerInfo = getPackageManagerInfo;
/**
* glob expanding memoized because it involves potentially very deep
* traversing through the directories tree
*/
exports.memoizedCacheDependencies = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
Expand All @@ -60516,27 +60511,15 @@ exports.memoizedCacheDependencies = {};
* @return list of directories and possible
*/
const getProjectDirectoriesFromCacheDependencyPath = (cacheDependencyPath) => __awaiter(void 0, void 0, void 0, function* () {
let cacheDependenciesPaths;
// memoize unglobbed paths to avoid traversing FS
const memoized = exports.memoizedCacheDependencies[cacheDependencyPath];
if (memoized) {
cacheDependenciesPaths = memoized;
}
else {
const globber = yield glob.create(cacheDependencyPath);
cacheDependenciesPaths = yield globber.glob();
exports.memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
}
const globber = yield glob.create(cacheDependencyPath);
const cacheDependenciesPaths = yield globber.glob();
const existingDirectories = cacheDependenciesPaths
.map(path_1.default.dirname)
.filter(util_1.unique())
.filter(fs_1.default.existsSync)
.filter(directory => fs_1.default.lstatSync(directory).isDirectory());
// if user explicitly pointed out some file, but it does not exist it is definitely
// not he wanted, thus we should throw an error not trying to workaround with unexpected
// result to the whole build
if (!existingDirectories.length)
throw Error('No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"');
core.warning(`No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`);
return existingDirectories;
});
/**
Expand Down
27 changes: 5 additions & 22 deletions dist/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71153,7 +71153,7 @@ const restoreCache = (packageManager, cacheDependencyPath) => __awaiter(void 0,
if (!fileHash) {
throw new Error('Some specified paths were not resolved, unable to cache dependencies.');
}
const primaryKey = `node-cache-${platform}-${packageManager}-v2-${fileHash}`;
const primaryKey = `node-cache-${platform}-${packageManager}-${fileHash}`;
core.debug(`primary key is ${primaryKey}`);
core.saveState(constants_1.State.CachePrimaryKey, primaryKey);
const cacheKey = yield cache.restoreCache(cachePaths, primaryKey);
Expand Down Expand Up @@ -71217,7 +71217,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.memoizedCacheDependencies = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
const core = __importStar(__nccwpck_require__(2186));
const exec = __importStar(__nccwpck_require__(1514));
const cache = __importStar(__nccwpck_require__(7799));
Expand Down Expand Up @@ -71286,11 +71286,6 @@ const getPackageManagerInfo = (packageManager) => __awaiter(void 0, void 0, void
}
});
exports.getPackageManagerInfo = getPackageManagerInfo;
/**
* glob expanding memoized because it involves potentially very deep
* traversing through the directories tree
*/
exports.memoizedCacheDependencies = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
Expand All @@ -71299,27 +71294,15 @@ exports.memoizedCacheDependencies = {};
* @return list of directories and possible
*/
const getProjectDirectoriesFromCacheDependencyPath = (cacheDependencyPath) => __awaiter(void 0, void 0, void 0, function* () {
let cacheDependenciesPaths;
// memoize unglobbed paths to avoid traversing FS
const memoized = exports.memoizedCacheDependencies[cacheDependencyPath];
if (memoized) {
cacheDependenciesPaths = memoized;
}
else {
const globber = yield glob.create(cacheDependencyPath);
cacheDependenciesPaths = yield globber.glob();
exports.memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
}
const globber = yield glob.create(cacheDependencyPath);
const cacheDependenciesPaths = yield globber.glob();
const existingDirectories = cacheDependenciesPaths
.map(path_1.default.dirname)
.filter(util_1.unique())
.filter(fs_1.default.existsSync)
.filter(directory => fs_1.default.lstatSync(directory).isDirectory());
// if user explicitly pointed out some file, but it does not exist it is definitely
// not he wanted, thus we should throw an error not trying to workaround with unexpected
// result to the whole build
if (!existingDirectories.length)
throw Error('No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"');
core.warning(`No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`);
return existingDirectories;
});
/**
Expand Down
2 changes: 1 addition & 1 deletion src/cache-restore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const restoreCache = async (
);
}

const primaryKey = `node-cache-${platform}-${packageManager}-v2-${fileHash}`;
const primaryKey = `node-cache-${platform}-${packageManager}-${fileHash}`;
core.debug(`primary key is ${primaryKey}`);

core.saveState(State.CachePrimaryKey, primaryKey);
Expand Down
25 changes: 4 additions & 21 deletions src/cache-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,6 @@ export const getPackageManagerInfo = async (packageManager: string) => {
}
};

/**
* glob expanding memoized because it involves potentially very deep
* traversing through the directories tree
*/
export const memoizedCacheDependencies: Record<string, string[]> = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
Expand All @@ -125,30 +120,18 @@ export const memoizedCacheDependencies: Record<string, string[]> = {};
const getProjectDirectoriesFromCacheDependencyPath = async (
cacheDependencyPath: string
): Promise<string[]> => {
let cacheDependenciesPaths: string[];

// memoize unglobbed paths to avoid traversing FS
const memoized = memoizedCacheDependencies[cacheDependencyPath];
if (memoized) {
cacheDependenciesPaths = memoized;
} else {
const globber = await glob.create(cacheDependencyPath);
cacheDependenciesPaths = await globber.glob();
memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
}
const globber = await glob.create(cacheDependencyPath);
const cacheDependenciesPaths = await globber.glob();

const existingDirectories: string[] = cacheDependenciesPaths
.map(path.dirname)
.filter(unique())
.filter(fs.existsSync)
.filter(directory => fs.lstatSync(directory).isDirectory());

// if user explicitly pointed out some file, but it does not exist it is definitely
// not he wanted, thus we should throw an error not trying to workaround with unexpected
// result to the whole build
if (!existingDirectories.length)
throw Error(
'No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"'
core.warning(
`No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`
);

return existingDirectories;
Expand Down