improvement(build-tools): support 1-deep transitive deps

[jason-ha]

In policy checks transitive dependencies are not deeply considered. But one level deep checks are now added to handle regular tasks that are expanded to be complex (multiple tasks).

Example

Splitting common build:esnext into two steps should not require downstream consumers to change dependencies as dependency on build:esnext still ensures that all of the build steps are run. The policy checkers know that build:esnext:main and/or build:esnext:experimental are required, but not that build:esnext ensures they are run.

-		"build:esnext": "tsc --project ./tsconfig.json",
+		"build:esnext": "npm run build:esnext:main && npm run build:esnext:experimental",
+		"build:esnext:experimental": "tsc --project ./tsconfig.json",
+		"build:esnext:main": "tsc --project ./tsconfig.main.json",

Infrastructure changes in support:

• Add "children" to Task Definitions to understand what other tasks are run when running a task. This enables policy checks to work with a multi-step task as a dependency and not requiring explicit dependency on one or each part.

  "children" property of Task Definition is computed from script command line explicitly and is not manually configurable. Scripts listed via npm run (no other arguments) and concurrently are recognized.

• Refactors existing logic from taskFactory to support concurrently parsing.

• Add broad readonly attribute to Task Definitions and Configs that shouldn't be changed. Add cloning where mutation may occur. (Note that TypeScript doesn't detect all cases where something readonly is assigned to non-readonly context. See Strict variance and read-only versions of types TypeScript#18770.) Where mutability is needed in policy fix-up handler, cast away to writeable.

[jason-ha]

Reviewers may want to review each of the first 3 commits separately. The third is all about readonly changes to help see where cloning should be used to avoid comingling of changes.

[tylerbutler]

Can you provide an example of what the task definitions look like before and after this change? I'm havbing a little trouble understanding what this addresses, but maybe it's because we don't have any tasks yet for which this is needed?

The includes property seems to be useful, but couldn’t it be sourced from the task graph itself rather than parsing the args separatel? I would expect the task graph to already have done the parsing and task decomposition.

For example, if I have a task definition like this:

"clean": "concurrently npm:clean:build npm:clean:manifest"

If I run fluid-build . -t clean, I see two tasks:

Start tasks 'clean' in 1 matched packages (2 total tasks in 3 packages)
[1/2] ✓ dill-cli: rimraf esm *.tsbuildinfo *.done.build.log - 0.067s (non-incremental)
[2/2] ✓ dill-cli: rimraf oclif.manifest.json - 0.065s (non-incremental)

So with this change, the "includes" property for the "clean" task would contain rimraf esm *.tsbuildinfo *.done.build.log and rimraf oclif.manifest.json, right?

Separately, have you tested this against the main branch? I tried using the approach described here but got an error during build graph creation:

⠋ Creating build graph...
ERROR: @fluid-internal/devtools-view: Failed to load build package in /home/tylerbu/code/FluidFramework/packages/tools/devtools/devtools-view

[jason-ha]

Can you provide an example of what the task definitions look like before and after this change? I'm havbing a little trouble understanding what this addresses, but maybe it's because we don't have any tasks yet for which this is needed?

The includes property seems to be useful, but couldn’t it be sourced from the task graph itself rather than parsing the args separatel? I would expect the task graph to already have done the parsing and task decomposition.

Example provided in updated description.
The task graph is not used by the policy checkers. It would be possible to determine includes on the fly, but it seemed better to keep it with the other data.

[jason-ha]

For example, if I have a task definition like this:

"clean": "concurrently npm:clean:build npm:clean:manifest"

If I run fluid-build . -t clean, I see two tasks:

Start tasks 'clean' in 1 matched packages (2 total tasks in 3 packages)
[1/2] ✓ dill-cli: rimraf esm *.tsbuildinfo *.done.build.log - 0.067s (non-incremental)
[2/2] ✓ dill-cli: rimraf oclif.manifest.json - 0.065s (non-incremental)

So with this change, the "includes" property for the "clean" task would contain rimraf esm *.tsbuildinfo *.done.build.log and rimraf oclif.manifest.json, right?

The includes property would be ["clean:build", "clean:manifest"]. Doesn't look to the contents of the script.

[jason-ha]

Separately, have you tested this against the main branch? I tried using the approach described here but got an error during build graph creation:

⠋ Creating build graph...
ERROR: @fluid-internal/devtools-view: Failed to load build package in /home/tylerbu/code/FluidFramework/packages/tools/devtools/devtools-view

Yep. It is sniffing out incorrect package script definitions. There is a separate PR to clean those: #23865

[tylerbutler]

The includes property would be ["clean:build", "clean:manifest"]. Doesn't look to the contents of the script.

Ah, got it, that makes sense.

I'm still confused about the issue this is addressing. Looking at your example:

-		"build:esnext": "tsc --project ./tsconfig.json",
+		"build:esnext": "npm run build:esnext:main && npm run build:esnext:experimental",
+		"build:esnext:experimental": "tsc --project ./tsconfig.json",
+		"build:esnext:main": "tsc --project ./tsconfig.main.json",

It's not clear to me how these changes are needed to enable those sorts of script changes. Doesn't this example work as expected today? Alternatively, couldn't the root build:esnext task definition be changed to include build:esnext:experimental and build:esnext:main, and then any packages that have those scripts would automatically run them as deps of build:esnext. Packages would opt in to the new behavior by adding those scripts.

And much like other top-level scripts like build, build:esnext could just call fluid-build uniformly - fluid-build . -t build:esnext.

[jason-ha]

-		"build:esnext": "tsc --project ./tsconfig.json",
+		"build:esnext": "npm run build:esnext:main && npm run build:esnext:experimental",
+		"build:esnext:experimental": "tsc --project ./tsconfig.json",
+		"build:esnext:main": "tsc --project ./tsconfig.main.json",

It's not clear to me how these changes are needed to enable those sorts of script changes. Doesn't this example work as expected today? Alternatively, couldn't the root build:esnext task definition be changed to include build:esnext:experimental and build:esnext:main, and then any packages that have those scripts would automatically run them as deps of build:esnext. Packages would opt in to the new behavior by adding those scripts.

The build graph and dependencies understood by fluid-build are fine. The policy checks are not as they don't hydrate Tasks and BuildGraph. The graph of dependencies seems to be derivable from tasks in config files except for such GroupTasks that TaskFactory.Create forms. I refactored logic from TaskFactory.Create to parse concurrently in the same way and the simplified npm run support is pretty straight-forward.

And much like other top-level scripts like build, build:esnext could just call fluid-build uniformly - fluid-build . -t build:esnext.

And I think that implies then defining that "build:esnext" depends on those "children" tasks. It feels wrong to me to invoke fluid-build where npm run x && npm run y or concurrently would suffice. (fluid-build invocations are also slow.) I would like to see less use of explicit tasks configuration - have the system figure it out.

First commit of PR would still be needed to support making "build:esnext" into a fluid-build task. The policy checker knows that a certain script is required in the upstream package.

[jason-ha]

/azp run "Build - build-tools"

[azure-pipelines]

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

[azure-pipelines]

No pipelines are associated with this pull request.