Skip to content

Commit

Permalink
fix(build-tools): ignore cross group deps for policy (#21238)
Browse files Browse the repository at this point in the history
For tsc dependency checks limit dependencies to those within a
workspace. With special exception for
repo release groups (mostly restored historical check).

Add a filter function when acquiring possible predecessors. (More
efficient than a post check looking through groups for consistent
package name prefixes.)
  • Loading branch information
jason-ha authored May 28, 2024
1 parent d532964 commit d6ed4c6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ export class FluidBuildDatabase {
* @param packageGroup - map (cache) of packageName's related packages
* @param packageName - package name
* @param script - packages script name
* @param ignorePackage - optional filter function to ignore select packages
* @returns Array of groups of possible predecessor tasks
*/
public getPossiblePredecessorTasks(
packageGroup: ReadonlyMap<PackageName, Package>,
packageName: PackageName,
script: Script,
ignorePackage?: (packageInfo: { name: string; version: string; dev: boolean }) => boolean,
): BuildScript[][] {
const pkg = packageGroup.get(packageName);
if (pkg === undefined) {
Expand All @@ -254,6 +256,9 @@ export class FluidBuildDatabase {
// Note that combinedDependencies (as of 2024-05-13) does not consider peer
// dependencies that could be linked.
for (const dep of pkg.combinedDependencies) {
if (ignorePackage?.(dep) ?? false) {
continue;
}
const depPackageName = dep.name;
const depBuildScripts = this.packageBuildScripts.get(depPackageName);
if (depBuildScripts !== undefined) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
updatePackageJsonFile,
} from "@fluidframework/build-tools";
import * as JSON5 from "json5";
import * as semver from "semver";
import { TsConfigJson } from "type-fest";
import { Handler, readFile } from "./common";
import { FluidBuildDatabase } from "./fluidBuildDatabase";
Expand Down Expand Up @@ -549,10 +550,37 @@ function getTscCommandDependencies(
}
}

const curPkgRepoGroup = packageMap.get(json.name)?.group;
const tscPredecessors = fluidBuildDatabaseCache.getPossiblePredecessorTasks(
packageMap,
json.name,
script,
// ignore filter function
(depSpec: { name: string; version: string }) => {
// Never ignore workspace linked dependencies
if (depSpec.version.includes("workspace:")) {
return false;
}
// Historically, a semantic version check was also considered sufficient
// to indicate a possible dependency. This was probably the case for lerna
// managed repo. The check is preserved here, but only allowed when the
// packages are within the same release group.
// Note: packages may be symlinked across workspace boundaries and in those
// situations, it is up to the user to build in the correct order. To enact
// a full repo ordering, support would be needed to recognize tooling
// dependencies used to run scripts apart from compile time dependencies,
// especially since the module type is irrelevant for execution dependencies.
const depPackage = packageMap.get(depSpec.name);
if (depPackage === undefined) {
// Not known to repo, can be ignored.
return true;
}
if (depPackage.group !== curPkgRepoGroup) {
return true;
}
const satisfied = semver.satisfies(depPackage.version, depSpec.version);
return !satisfied;
},
);

// eslint-disable-next-line unicorn/prefer-spread
Expand Down

0 comments on commit d6ed4c6

Please sign in to comment.