From 7a16748ee069e01041f1928e39f04f05d90429c3 Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Mon, 13 Jan 2025 10:36:49 -0800 Subject: [PATCH] improvement(fluid-build): Fix group task dependencies and reduce message clutter (#23523) Fix group task dependencies: For script task that has "&&" in the command, a sequential group task is created. When building the dependency graph, the latter tasks need to depend on the previous task. However, an "iterator" of previous leaf task is passed to add as dependency for the latter tasks, and if the latter tasks have multiple subtasks, the iterator will only iterate once for the first subtask and miss the other subtask since it doesn't reset. The fix is to just pass the Set into `addDependentLeafTasks` Reduce message clutter: - For set up without config file, avoid having a big warning message. Merge the indication with the "Build Root" message. - Don't show symlink status unless it not in the default mode. Additionally, make `getFluidBuildConfig` correctly track which `searchDir` is using a default config in case it is used with multiple `searchDir` in the future. --------- Co-authored-by: Abram Sanderson Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com> --- .../build-tools/src/fluidBuild/fluidBuild.ts | 24 +++++++++---------- .../build-tools/src/fluidBuild/fluidUtils.ts | 20 +++++++++------- .../src/fluidBuild/tasks/groupTask.ts | 2 +- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/build-tools/packages/build-tools/src/fluidBuild/fluidBuild.ts b/build-tools/packages/build-tools/src/fluidBuild/fluidBuild.ts index d59d07af33ab..64bc56cf0f4a 100644 --- a/build-tools/packages/build-tools/src/fluidBuild/fluidBuild.ts +++ b/build-tools/packages/build-tools/src/fluidBuild/fluidBuild.ts @@ -10,6 +10,7 @@ import { defaultLogger } from "../common/logging"; import { Timer } from "../common/timer"; import { BuildGraph, BuildResult } from "./buildGraph"; import { commonOptions } from "./commonOptions"; +import { DEFAULT_FLUIDBUILD_CONFIG } from "./fluidBuildConfig"; import { FluidRepoBuild } from "./fluidRepoBuild"; import { getFluidBuildConfig, getResolvedFluidRoot } from "./fluidUtils"; import { options, parseOptions } from "./options"; @@ -21,15 +22,20 @@ parseOptions(process.argv); async function main() { const timer = new Timer(commonOptions.timer); const resolvedRoot = await getResolvedFluidRoot(true); - - log(`Build Root: ${resolvedRoot}`); + const fluidConfig = getFluidBuildConfig(resolvedRoot, false); + const isDefaultConfig = fluidConfig === DEFAULT_FLUIDBUILD_CONFIG; + const suffix = isDefaultConfig + ? ` (${chalk.yellowBright("inferred packages and tasks")})` + : ""; + log(`Build Root: ${resolvedRoot}${suffix}`); // Load the packages const repo = new FluidRepoBuild({ repoRoot: resolvedRoot, gitRepo: new GitRepo(resolvedRoot), - fluidBuildConfig: getFluidBuildConfig(resolvedRoot), + fluidBuildConfig: fluidConfig, }); + timer.time("Package scan completed"); // Set matched package based on options filter @@ -87,15 +93,9 @@ async function main() { let failureSummary = ""; let exitCode = 0; if (options.buildTaskNames.length !== 0) { - log( - `Symlink in ${ - options.fullSymlink - ? "full" - : options.fullSymlink === false - ? "isolated" - : "non-dependent" - } mode`, - ); + if (options.fullSymlink !== undefined) { + log(chalk.yellow(`Symlink in ${options.fullSymlink ? "full" : "isolated"} mode`)); + } // build the graph let buildGraph: BuildGraph; diff --git a/build-tools/packages/build-tools/src/fluidBuild/fluidUtils.ts b/build-tools/packages/build-tools/src/fluidBuild/fluidUtils.ts index 5d32da2311ca..bb6021031dae 100644 --- a/build-tools/packages/build-tools/src/fluidBuild/fluidUtils.ts +++ b/build-tools/packages/build-tools/src/fluidBuild/fluidUtils.ts @@ -145,32 +145,36 @@ const configExplorer = cosmiconfigSync(configName, { }); /** - * Set to true when the default config is returned by getFluidBuildConfig so that repeated calls to the function don't - * result in repeated searches for config. + * Contains directories previously used to start search but where we didn't find an explicit fluidBuild config file. + * This allows avoiding repeated searches for config. */ -let defaultConfigLoaded = false; +const defaultSearchDir = new Set(); /** * Get an IFluidBuildConfig from the fluidBuild property in a package.json file, or from fluidBuild.config.[c]js. * * @param searchDir - The path to search for the config. The search will look up the folder hierarchy for a config in * either a standalone file or package.json + * @param warnNotFound - Whether to warn if no fluidBuild config is found. * @returns The the loaded fluidBuild config, or the default config if one is not found. */ export function getFluidBuildConfig( searchDir: string, + warnNotFound = true, log = defaultLogger, ): IFluidBuildConfig { - if (defaultConfigLoaded) { + if (defaultSearchDir.has(searchDir)) { return DEFAULT_FLUIDBUILD_CONFIG; } const configResult = configExplorer.search(searchDir); if (configResult?.config === undefined) { - log.warning( - `No fluidBuild config found when searching ${searchDir}; default configuration loaded. Packages and tasks will be inferred.`, - ); - defaultConfigLoaded = true; + if (warnNotFound) { + log.warning( + `No fluidBuild config found when searching ${searchDir}; default configuration loaded. Packages and tasks will be inferred.`, + ); + } + defaultSearchDir.add(searchDir); return DEFAULT_FLUIDBUILD_CONFIG; } diff --git a/build-tools/packages/build-tools/src/fluidBuild/tasks/groupTask.ts b/build-tools/packages/build-tools/src/fluidBuild/tasks/groupTask.ts index 9f898238b34d..809a150c0824 100644 --- a/build-tools/packages/build-tools/src/fluidBuild/tasks/groupTask.ts +++ b/build-tools/packages/build-tools/src/fluidBuild/tasks/groupTask.ts @@ -33,7 +33,7 @@ export class GroupTask extends Task { if (prevTask !== undefined) { const leafTasks = new Set(); prevTask.collectLeafTasks(leafTasks); - task.addDependentLeafTasks(leafTasks.values()); + task.addDependentLeafTasks(leafTasks); } prevTask = task; }