Skip to content

Commit

Permalink
improvement(fluid-build): Fix group task dependencies and reduce mess…
Browse files Browse the repository at this point in the history
…age 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 <Abram.sanderson@gmail.com>
Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 13, 2025
1 parent 96a1073 commit 7a16748
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 21 deletions.
24 changes: 12 additions & 12 deletions build-tools/packages/build-tools/src/fluidBuild/fluidBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 12 additions & 8 deletions build-tools/packages/build-tools/src/fluidBuild/fluidUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();

/**
* 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class GroupTask extends Task {
if (prevTask !== undefined) {
const leafTasks = new Set<LeafTask>();
prevTask.collectLeafTasks(leafTasks);
task.addDependentLeafTasks(leafTasks.values());
task.addDependentLeafTasks(leafTasks);
}
prevTask = task;
}
Expand Down

0 comments on commit 7a16748

Please sign in to comment.