Skip to content

Commit

Permalink
fix(core): fix migrate race conditions (#9794)
Browse files Browse the repository at this point in the history
  • Loading branch information
@FrozenPandaz
FrozenPandaz committed Apr 12, 2022
1 parent 9396db0 commit c047151
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 117 deletions.
25 changes: 25 additions & 0 deletions packages/nx/package.json
View file
Expand Up @@ -60,5 +60,30 @@
"minimatch": "3.0.4",
"enquirer": "~2.3.6",
"tslib": "^2.3.0"
},
"ng-update": {
"requirements": {},
"packageGroup": {
"@nrwl/workspace": "*",
"@nrwl/angular": "*",
"@nrwl/cypress": "*",
"@nrwl/devkit": "*",
"@nrwl/eslint-plugin-nx": "*",
"@nrwl/express": "*",
"@nrwl/jest": "*",
"@nrwl/linter": "*",
"@nrwl/nest": "*",
"@nrwl/next": "*",
"@nrwl/node": "*",
"@nrwl/nx-plugin": "*",
"@nrwl/react": "*",
"@nrwl/storybook": "*",
"@nrwl/web": "*",
"@nrwl/js": "*",
"@nrwl/cli": "*",
"@nrwl/nx-cloud": "latest",
"@nrwl/react-native": "*",
"@nrwl/detox": "*"
}
}
}
181 changes: 117 additions & 64 deletions packages/nx/src/command-line/migrate.ts
View file
@@ -1,7 +1,7 @@
import { exec, execSync } from 'child_process';
import { remove } from 'fs-extra';
import { dirname, join } from 'path';
import { gt, lte } from 'semver';
import { gt, lt, lte } from 'semver';
import { promisify } from 'util';
import {
MigrationsJson,
Expand All @@ -16,7 +16,11 @@ import {
writeJsonFile,
} from '../utils/fileutils';
import { logger } from '../utils/logger';
import { NxMigrationsConfiguration, PackageJson } from '../utils/package-json';
import {
NxMigrationsConfiguration,
PackageGroup,
PackageJson,
} from '../utils/package-json';
import {
createTempNpmDirectory,
getPackageManagerCommand,
Expand Down Expand Up @@ -91,11 +95,10 @@ export class Migrator {
}

async updatePackageJson(targetPackage: string, targetVersion: string) {
const packageJson = await this._updatePackageJson(
targetPackage,
{ version: targetVersion, addToPackageJson: false },
{}
);
const packageJson = await this._updatePackageJson(targetPackage, {
version: targetVersion,
addToPackageJson: false,
});

const migrations = await this._createMigrateJson(packageJson);
return { packageJson, migrations };
Expand Down Expand Up @@ -132,10 +135,11 @@ export class Migrator {
return migrations.flat();
}

private collectedVersions: Record<string, string> = {};

private async _updatePackageJson(
targetPackage: string,
target: PackageJsonUpdateForPackage,
collectedVersions: Record<string, PackageJsonUpdateForPackage>
target: PackageJsonUpdateForPackage
): Promise<Record<string, PackageJsonUpdateForPackage>> {
let targetVersion = target.version;
if (this.to[targetPackage]) {
Expand All @@ -155,6 +159,7 @@ export class Migrator {
try {
migrationsJson = await this.fetch(targetPackage, targetVersion);
targetVersion = migrationsJson.version;
this.collectedVersions[targetPackage] = targetVersion;
} catch (e) {
if (e?.message?.includes('No matching version')) {
throw new Error(
Expand All @@ -173,20 +178,16 @@ export class Migrator {

const childPackageMigrations = await Promise.all(
Object.keys(packages)
.filter((packageName) => {
return (
!collectedVersions[packageName] ||
.filter(
(packageName) =>
!this.collectedVersions[packageName] ||
this.gt(
packages[packageName].version,
collectedVersions[packageName].version
this.collectedVersions[packageName]
)
);
})
)
.map((packageName) =>
this._updatePackageJson(packageName, packages[packageName], {
...collectedVersions,
[targetPackage]: target,
})
this._updatePackageJson(packageName, packages[packageName])
)
);

Expand Down Expand Up @@ -222,11 +223,24 @@ export class Migrator {
// this should be used to know what version to include
// we should use from everywhere we use versions

// Support Migrating to older versions of Nx
// Use the packageGroup of the latest version of Nx instead of the one from the target version which could be older.
if (
packageName === '@nrwl/workspace' &&
lt(targetVersion, '14.0.0-beta.0')
) {
migration.packageGroup =
require('../../package.json')['ng-update'].packageGroup;
}

if (migration.packageGroup) {
migration.packageJsonUpdates ??= {};
migration.packageJsonUpdates[`${targetVersion}-defaultPackages`] = {

const packageGroup = normalizePackageGroup(migration.packageGroup);

migration.packageJsonUpdates[targetVersion + '--PackageGroup'] = {
version: targetVersion,
packages: migration.packageGroup.reduce((acc, packageConfig) => {
packages: packageGroup.reduce((acc, packageConfig) => {
const { package: pkg, version } =
typeof packageConfig === 'string'
? { package: packageConfig, version: targetVersion }
Expand Down Expand Up @@ -292,6 +306,18 @@ export class Migrator {
}
}

function normalizePackageGroup(
packageGroup: PackageGroup
): (string | { package: string; version: string })[] {
if (!Array.isArray(packageGroup)) {
return Object.entries(packageGroup).map(([pkg, version]) => ({
package: pkg,
version,
}));
}
return packageGroup;
}

function normalizeVersionWithTagCheck(version: string) {
if (version === 'latest' || version === 'next') return version;
return normalizeVersion(version);
Expand Down Expand Up @@ -427,58 +453,74 @@ function versions(root: string, from: Record<string, string>) {

// testing-fetch-start
function createFetcher() {
const cache: Record<string, ResolvedMigrationConfiguration> = {};
const migrationsCache: Record<
string,
Promise<ResolvedMigrationConfiguration>
> = {};
const resolvedVersionCache: Record<string, Promise<string>> = {};

function fetchMigrations(
packageName,
packageVersion,
setCache: (packageName: string, packageVersion: string) => void
): Promise<ResolvedMigrationConfiguration> {
const cacheKey = packageName + '-' + packageVersion;
return Promise.resolve(resolvedVersionCache[cacheKey])
.then((cachedResolvedVersion) => {
if (cachedResolvedVersion) {
return cachedResolvedVersion;
}

resolvedVersionCache[cacheKey] = resolvePackageVersionUsingRegistry(
packageName,
packageVersion
);
return resolvedVersionCache[cacheKey];
})
.then((resolvedVersion) => {
if (
resolvedVersion !== packageVersion &&
migrationsCache[`${packageName}-${resolvedVersion}`]
) {
return migrationsCache[`${packageName}-${resolvedVersion}`];
}
setCache(packageName, resolvedVersion);
return getPackageMigrationsUsingRegistry(packageName, resolvedVersion);
})
.catch(() => {
logger.info(`Fetching ${packageName}@${packageVersion}`);

return async function nxMigrateFetcher(
return getPackageMigrationsUsingInstall(packageName, packageVersion);
});
}

return function nxMigrateFetcher(
packageName: string,
packageVersion: string
): Promise<ResolvedMigrationConfiguration> {
if (cache[`${packageName}-${packageVersion}`]) {
return cache[`${packageName}-${packageVersion}`];
if (migrationsCache[`${packageName}-${packageVersion}`]) {
return migrationsCache[`${packageName}-${packageVersion}`];
}

let resolvedVersion: string = packageVersion;
let resolvedMigrationConfiguration: ResolvedMigrationConfiguration;

try {
resolvedVersion = await resolvePackageVersionUsingRegistry(
packageName,
packageVersion
);

if (cache[`${packageName}-${resolvedVersion}`]) {
return cache[`${packageName}-${resolvedVersion}`];
}

logger.info(`Fetching ${packageName}@${packageVersion}`);

resolvedMigrationConfiguration = await getPackageMigrationsUsingRegistry(
packageName,
resolvedVersion
);
} catch {
logger.info(`Fetching ${packageName}@${packageVersion}`);

resolvedMigrationConfiguration = await getPackageMigrationsUsingInstall(
packageName,
packageVersion
);

resolvedVersion = resolvedMigrationConfiguration.version;
let migrations: Promise<ResolvedMigrationConfiguration>;
function setCache(packageName: string, packageVersion: string) {
migrationsCache[packageName + '-' + packageVersion] = migrations;
}
migrations = fetchMigrations(packageName, packageVersion, setCache).then(
(result) => {
if (result.schematics) {
result.generators = result.schematics;
delete result.schematics;
}
resolvedVersion = result.version;
return result;
}
);

resolvedMigrationConfiguration = {
...resolvedMigrationConfiguration,
generators:
resolvedMigrationConfiguration.generators ??
resolvedMigrationConfiguration.schematics,
};

cache[`${packageName}-${packageVersion}`] = cache[
`${packageName}-${resolvedVersion}`
] = resolvedMigrationConfiguration;
setCache(packageName, packageVersion);

return resolvedMigrationConfiguration;
return migrations;
};
}
// testing-fetch-end
Expand All @@ -494,13 +536,21 @@ async function getPackageMigrationsUsingRegistry(
packageVersion
);

if (!migrationsConfig) {
return {
version: packageVersion,
};
}

if (!migrationsConfig.migrations) {
return {
version: packageVersion,
packageGroup: migrationsConfig.packageGroup,
};
}

logger.info(`Fetching ${packageName}@${packageVersion}`);

// try to obtain the migrations from the registry directly
return await downloadPackageMigrationsFromRegistry(
packageName,
Expand All @@ -511,8 +561,11 @@ async function getPackageMigrationsUsingRegistry(

function resolveNxMigrationConfig(json: Partial<PackageJson>) {
const parseNxMigrationsConfig = (
fromJson: string | NxMigrationsConfiguration
fromJson?: string | NxMigrationsConfiguration
): NxMigrationsConfiguration => {
if (!fromJson) {
return {};
}
if (typeof fromJson === 'string') {
return { migrations: fromJson, packageGroup: [] };
}
Expand Down
6 changes: 5 additions & 1 deletion packages/nx/src/utils/package-json.ts
View file
Expand Up @@ -9,9 +9,13 @@ export interface NxProjectPackageJsonConfiguration {
targets?: Record<string, PackageJsonTargetConfiguration>;
}

export type PackageGroup =
| (string | { package: string; version: string })[]
| Record<string, string>;

export interface NxMigrationsConfiguration {
migrations?: string;
packageGroup?: (string | { package: string; version: string })[];
packageGroup?: PackageGroup;
}

export interface PackageJson {
Expand Down

0 comments on commit c047151

Please sign in to comment.