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

feat: bump Known Good Release when downloading new version #364

Merged
merged 4 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ recommended as a security practice. Permitted values for the package manager are
## Known Good Releases

When running Corepack within projects that don't list a supported package
manager, it will default to a set of Known Good Releases. In a way, you can
compare this to Node.js, where each version ships with a specific version of
npm.
manager, it will default to a set of Known Good Releases.

If there is no Known Good Release for the requested package manager, Corepack
looks up the npm registry for the latest available version and cache it for
future use.

The Known Good Releases can be updated system-wide using `corepack install -g`.
When Corepack downloads a new version of a given package manager on the same
major line as the Known Good Release, it auto-updates it by default.

## Offline Workflow

Expand Down Expand Up @@ -221,7 +221,8 @@ same major line. Should you need to upgrade to a new major, use an explicit

- `COREPACK_DEFAULT_TO_LATEST` can be set to `0` in order to instruct Corepack
not to lookup on the remote registry for the latest version of the selected
package manager.
package manager, and to not update the Last Known Good version when it
downloads a new version of the same major line.

- `COREPACK_ENABLE_NETWORK` can be set to `0` to prevent Corepack from accessing
the network (in which case you'll be responsible for hydrating the package
Expand Down
119 changes: 81 additions & 38 deletions sources/Engine.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {UsageError} from 'clipanion';
import type {FileHandle} from 'fs/promises';
import fs from 'fs';
import path from 'path';
import process from 'process';
Expand All @@ -7,13 +8,57 @@ import semver from 'semver';
import defaultConfig from '../config.json';

import * as corepackUtils from './corepackUtils';
import * as debugUtils from './debugUtils';
import * as folderUtils from './folderUtils';
import type {NodeError} from './nodeUtils';
import * as semverUtils from './semverUtils';
import {Config, Descriptor, Locator} from './types';
import {SupportedPackageManagers, SupportedPackageManagerSet} from './types';

export type PreparedPackageManagerInfo = Awaited<ReturnType<Engine[`ensurePackageManager`]>>;

export function getLastKnownGoodFile(flag = `r`) {
return fs.promises.open(path.join(folderUtils.getInstallFolder(), `lastKnownGood.json`), flag);
}

export async function getJSONFileContent(fh: FileHandle) {
let lastKnownGood: unknown;
try {
lastKnownGood = JSON.parse(await fh.readFile(`utf8`));
} catch {
// Ignore errors; too bad
return undefined;
}

return lastKnownGood;
}

async function overwriteJSONFileContent(fh: FileHandle, content: unknown) {
await fh.truncate(0);
await fh.write(`${JSON.stringify(content, null, 2)}\n`, 0);
}

export function getLastKnownGoodFromFileContent(lastKnownGood: unknown, packageManager: string) {
if (typeof lastKnownGood === `object` && lastKnownGood !== null &&
Object.hasOwn(lastKnownGood, packageManager)) {
const override = (lastKnownGood as any)[packageManager];
if (typeof override === `string`) {
return override;
}
}
return undefined;
}

export async function activatePackageManagerFromFileHandle(lastKnownGoodFile: FileHandle, lastKnownGood: unknown, locator: Locator) {
if (typeof lastKnownGood !== `object` || lastKnownGood === null)
lastKnownGood = {};

(lastKnownGood as Record<string, string>)[locator.name] = locator.reference;

debugUtils.log(`Setting ${locator.name}@${locator.reference} as Last Known Good version`);
await overwriteJSONFileContent(lastKnownGoodFile, lastKnownGood);
}

export class Engine {
constructor(public config: Config = defaultConfig as Config) {
}
Expand Down Expand Up @@ -77,51 +122,53 @@ export class Engine {
if (typeof definition === `undefined`)
throw new UsageError(`This package manager (${packageManager}) isn't supported by this corepack build`);

let lastKnownGood: unknown;
try {
lastKnownGood = JSON.parse(await fs.promises.readFile(this.getLastKnownGoodFile(), `utf8`));
} catch {
// Ignore errors; too bad
}

if (typeof lastKnownGood === `object` && lastKnownGood !== null &&
Object.hasOwn(lastKnownGood, packageManager)) {
const override = (lastKnownGood as any)[packageManager];
if (typeof override === `string`) {
return override;
let emptyFile = false;
const lastKnownGoodFile = await getLastKnownGoodFile(`r+`).catch(err => {
if ((err as NodeError)?.code === `ENOENT`) {
emptyFile = true;
return getLastKnownGoodFile(`w`);
}
}

if (process.env.COREPACK_DEFAULT_TO_LATEST === `0`)
return definition.default;
throw err;
});
try {
const lastKnownGood = emptyFile || await getJSONFileContent(lastKnownGoodFile);
const lastKnownGoodForThisPackageManager = getLastKnownGoodFromFileContent(lastKnownGood, packageManager);
if (lastKnownGoodForThisPackageManager)
return lastKnownGoodForThisPackageManager;

const reference = await corepackUtils.fetchLatestStableVersion(definition.fetchLatestFrom);
if (process.env.COREPACK_DEFAULT_TO_LATEST === `0`)
return definition.default;

await this.activatePackageManager({
name: packageManager,
reference,
});
const reference = await corepackUtils.fetchLatestStableVersion(definition.fetchLatestFrom);

await activatePackageManagerFromFileHandle(lastKnownGoodFile, lastKnownGood, {
name: packageManager,
reference,
});

return reference;
return reference;
} finally {
await lastKnownGoodFile.close();
}
}

async activatePackageManager(locator: Locator) {
const lastKnownGoodFile = this.getLastKnownGoodFile();
let emptyFile = false;
const lastKnownGoodFile = await getLastKnownGoodFile(`r+`).catch(err => {
if ((err as NodeError)?.code === `ENOENT`) {
emptyFile = true;
return getLastKnownGoodFile(`w`);
}

let lastKnownGood;
throw err;
});
try {
lastKnownGood = JSON.parse(await fs.promises.readFile(lastKnownGoodFile, `utf8`));
} catch {
// Ignore errors; too bad
await activatePackageManagerFromFileHandle(lastKnownGoodFile, emptyFile || await getJSONFileContent(lastKnownGoodFile), locator);
await lastKnownGoodFile.close();
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
} finally {
await lastKnownGoodFile.close();
}

if (typeof lastKnownGood !== `object` || lastKnownGood === null)
lastKnownGood = {};

lastKnownGood[locator.name] = locator.reference;

await fs.promises.mkdir(path.dirname(lastKnownGoodFile), {recursive: true});
await fs.promises.writeFile(lastKnownGoodFile, `${JSON.stringify(lastKnownGood, null, 2)}\n`);
}

async ensurePackageManager(locator: Locator) {
Expand Down Expand Up @@ -194,8 +241,4 @@ export class Engine {

return {name: finalDescriptor.name, reference: highestVersion[0]};
}

private getLastKnownGoodFile() {
return path.join(folderUtils.getInstallFolder(), `lastKnownGood.json`);
}
}
30 changes: 28 additions & 2 deletions sources/corepackUtils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {createHash} from 'crypto';
import {once} from 'events';
import {FileHandle} from 'fs/promises';
import fs from 'fs';
import type {Dir} from 'fs';
import Module from 'module';
import path from 'path';
import semver from 'semver';

import * as engine from './Engine';
import * as debugUtils from './debugUtils';
import * as folderUtils from './folderUtils';
import * as fsUtils from './fsUtils';
Expand Down Expand Up @@ -103,8 +105,8 @@ export async function findInstalledVersion(installTarget: string, descriptor: De
}

export async function installVersion(installTarget: string, locator: Locator, {spec}: {spec: PackageManagerSpec}) {
const {default: tar} = await import(`tar`);
const {version, build} = semver.parse(locator.reference)!;
const locatorReference = semver.parse(locator.reference)!;
const {version, build} = locatorReference;

const installFolder = path.join(installTarget, locator.name, version);
const corepackFile = path.join(installFolder, `.corepack`);
Expand Down Expand Up @@ -146,6 +148,7 @@ export async function installVersion(installTarget: string, locator: Locator, {s

let sendTo: any;
if (ext === `.tgz`) {
const {default: tar} = await import(`tar`);
sendTo = tar.x({strip: 1, cwd: tmpFolder});
} else if (ext === `.js`) {
outputFile = path.join(tmpFolder, path.posix.basename(parsedUrl.pathname));
Expand Down Expand Up @@ -193,6 +196,29 @@ export async function installVersion(installTarget: string, locator: Locator, {s
}
}

if (process.env.COREPACK_DEFAULT_TO_LATEST !== `0`) {
let lastKnownGoodFile: FileHandle;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Not a fan of working with file handles; it makes the code harder to follow for something that's relatively trivial perf-wise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were reading and parsing the same JSON twice, so at least that should improve the perf ever so slightly. But I think it's the only way to avoid race conditions in case other processes are touching the same file.

try {
lastKnownGoodFile = await engine.getLastKnownGoodFile(`r+`);
const lastKnownGood = await engine.getJSONFileContent(lastKnownGoodFile);
const defaultVersion = engine.getLastKnownGoodFromFileContent(lastKnownGood, locator.name);
if (defaultVersion) {
const currentDefault = semver.parse(defaultVersion)!;
if (currentDefault.major === locatorReference.major && semver.lt(currentDefault, locatorReference)) {
await engine.activatePackageManagerFromFileHandle(lastKnownGoodFile, lastKnownGood, locator);
}
}
} catch (err) {
// ENOENT would mean there are no lastKnownGoodFile, in which case we can ignore.
if ((err as nodeUtils.NodeError)?.code !== `ENOENT`) {
throw err;
}
} finally {
// @ts-expect-error used before assigned
await lastKnownGoodFile?.close();
}
}

debugUtils.log(`Install finished`);

return {
Expand Down
46 changes: 46 additions & 0 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,52 @@ for (const [name, version] of testedPackageManagers) {
});
}

it(`should update the Known Good Release only when the major matches`, async () => {
await xfs.writeJsonPromise(ppath.join(corepackHome, `lastKnownGood.json`), {
yarn: `1.0.0`,
});

process.env.COREPACK_DEFAULT_TO_LATEST = `1`;

await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
packageManager: `[email protected]+sha224.0d6eecaf4d82ec12566fdd97143794d0f0c317e0d652bd4d1b305430`,
});

await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
stdout: `1.22.4\n`,
});

await xfs.removePromise(ppath.join(cwd, `package.json` as Filename));

await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
stdout: `1.22.4\n`,
});

await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
packageManager: `[email protected]`,
});

await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
stdout: `2.2.2\n`,
});

await xfs.removePromise(ppath.join(cwd, `package.json` as Filename));

await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
stdout: `1.22.4\n`,
});
});
});

it(`should ignore the packageManager field when found within a node_modules vendor`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.mkdirPromise(ppath.join(cwd, `node_modules/foo` as PortablePath), {recursive: true});
Expand Down
Binary file modified tests/nock/AL__3okpCdfjA6kGuG2rFQ-1.dat
Binary file not shown.
Binary file added tests/nock/_ssVB5fpNumqL8RMl4TqHw-1.dat
Binary file not shown.
Binary file added tests/nock/_ssVB5fpNumqL8RMl4TqHw-2.dat
Binary file not shown.
Binary file added tests/nock/_ssVB5fpNumqL8RMl4TqHw-3.dat
Binary file not shown.
Binary file added tests/nock/_ssVB5fpNumqL8RMl4TqHw-4.dat
Binary file not shown.
Loading