[rush] support build/rebuild command in rush plugin

[chengbapi]

Summary

When rush load the default command line configuration, it adds the default build commands and register the action before loading commands from rush plugin.
This leaves no opportunity for the Rush plugin to set the build/rebuild command, even if there is no build/rebuild in the default command line configuration.

Details

Instead of including the default build/rebuild in the default command line configuration, I add a condition to check if a plugin contains a build/rebuild command. If so, the default inclusion behavior will be disabled by setting doNotIncludeDefaultBuildCommands.

To ensure that the build/rebuild commands are properly set, the plugin configuration will also extend the default build/rebuild commands if they exist in the configuration.

To provide a quick overview of the current and expected behavior:"

current behavior:

D: default

A: common/config/command-line.json	B: plugin/command-line.json	result	notes
(none)	(none)	D	(also if files are missing entirely)
"build" command	(none)	A + D
(none)	"build" command	error(name already exists)	NOT_EXPECTED
"build" command	"build" command	error(name already exists)

expected behavior:

A: common/config/command-line.json	B: plugin/command-line.json	result	notes
(none)	(none)	D	(also if files are missing entirely)
"build" command	(none)	A + D
(none)	"build" command	B + D
"build" command	"build" command	error(name already exists)

How it was tested

Four automated test cases are added.

1. When the common command-line is missing and the plugin command-line defines build command, rush build and rush rebuild should work as expected. The build command summary should indicate that the action is registered by the plugin
2. Similar to 1, but plugin defines rebuild command
3. When both common command-line and plugin command-line define a build command, an error should occur due to the existing command.
4. Similar to 3 but define rebuild command.

Affliated

[chengbapi]

@chengbapi please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="TikTok Inc"

[dmichon-msft]

If we're auto-filling rebuild based on the existing build, then the command should be an exact clone of whatever the current build command is, but with incremental: false.

[octogonz]

I agree that it should inherit more of the state. For example, if for some reason the rush build command is customized to have safeForSimultaneousRushProcesses: true or enableParallelism: false, those parameters should be inherited by rush rebuild.

But it should not be an EXACT clone as you say. For example, the original motivation of DEFAULT_BUILD_COMMAND_JSON and DEFAULT_REBUILD_COMMAND_JSON was to provide professionally worded summaries and descriptions, without requiring every custom command to copy+paste that text into command-line.json.

📌 But this request is arguably outside the scope of PR #5163. It can be a separate GitHub issue or PR, and certainly needs a separate changelog.

[dmichon-msft]

I had missed how few fields are currently within DEFAULT_REBUILD_COMMAND_JSON and that it was almost entirely the strings. This does make sense as currently written.

[dmichon-msft]

Please add clarity to the PR description that this PR is about allowing Rush Plugins to define commands at all, not just about build/rebuild in particular. Until now the model for Rush Plugins that interact with commands is that as part of installing the plugin you also add the necessary configuration to common/config/rush/command-line.json. That's how, e.g. @rushstack/rush-serve-plugin works.

Edit: I stand corrected that there is, in fact, currently support for plugins to define command-line.json files; I'd never seen that this existed.

[dmichon-msft]

Can we just make command-line.json support an extends field instead? Letting plugins inject themselves into command-line.json is a lot more confusing to reason about and examine compared to having in your rush command-line.json "extends": "../path/to/shared/command-line.json". We also wouldn't have to write extensive new machinery, just leverage @rushstack/heft-config-file's NonProjectConfigurationFile to do the loading.

[chengbapi]

Can we just make command-line.json support an extends field instead? Letting plugins inject themselves into command-line.json is a lot more confusing to reason about and examine compared to having in your rush command-line.json "extends": "../path/to/shared/command-line.json". We also wouldn't have to write extensive new machinery, just leverage @rushstack/heft-config-file's NonProjectConfigurationFile to do the loading.

Explicitly using extends in command-line.json makes it clearer. Should it support an array of strings?

Is this a breaking change for all the Rush plugins that contain command-line.json?

[dmichon-msft]

Can we just make command-line.json support an extends field instead? Letting plugins inject themselves into command-line.json is a lot more confusing to reason about and examine compared to having in your rush command-line.json "extends": "../path/to/shared/command-line.json". We also wouldn't have to write extensive new machinery, just leverage @rushstack/heft-config-file's NonProjectConfigurationFile to do the loading.

Explicitly using extends in command-line.json makes it clearer. Should it support an array of strings?

Is this a breaking change for all the Rush plugins that contain command-line.json?

The existing functionality can stay as-is, I'd just recommend we update the loader of command-line.json to be based on @rushstack/heft-config-file and add to the schema the extends property. If we want to make it support an array, that's a change we should make globally (I can see the utility). I'd expect that we handle merging in that scenario by fully evaluating each extends items in order, then merging it with the next one, until we have a single parent config, then merge with current file.

[chengbapi]

Can we just make command-line.json support an extends field instead? Letting plugins inject themselves into command-line.json is a lot more confusing to reason about and examine compared to having in your rush command-line.json "extends": "../path/to/shared/command-line.json". We also wouldn't have to write extensive new machinery, just leverage @rushstack/heft-config-file's NonProjectConfigurationFile to do the loading.

Explicitly using extends in command-line.json makes it clearer. Should it support an array of strings?
Is this a breaking change for all the Rush plugins that contain command-line.json?

The existing functionality can stay as-is, I'd just recommend we update the loader of command-line.json to be based on @rushstack/heft-config-file and add to the schema the extends property. If we want to make it support an array, that's a change we should make globally (I can see the utility). I'd expect that we handle merging in that scenario by fully evaluating each extends items in order, then merging it with the next one, until we have a single parent config, then merge with current file.

Currently, the CommandLineConfiguration is self-contained. It associates phases, commands, and parameters in order.
If we use extends to merge multiple configurations, should we still need to guarantee the self-containment of each configuration?
A bad case would be if the command and parameters are defined in different configuration files. Would this make issue tracking more difficult?

[octogonz]

Is this a breaking change for all the Rush plugins that contain command-line.json?

The existing functionality can stay as-is, I'd just recommend we update the loader of command-line.json to be based on @rushstack/heft-config-file and add to the schema the extends property.

I have some reservations about supporting "extends" for something so important as command-line.json phases, which define the entire mental model for how phases work in a given repo. The phase relationships will certainly be harder to understand if their definitions are split across multiple JSON files with complex merging rules.

Under the current design, a subset of related phases/commands is completely specified by exactly one self-contained command-line.json file. Sometimes that file comes from a plugin, but even so Rush's design ensures that the plugin's command-line.json file will be committed to Git and easily discoverable. The design change introduced by PR #5163 is merely fixing an oversight where the rush build command could not be defined by a plugin, because it has defaults, and there was a flaw in how those defaults are getting merged.

📌 In any case, if "The existing functionality can stay as-is", then this debatable proposal can be handled via a separate GitHub issue or PR. We don't need to block @chengbapi's PR behind that discussion.

[octogonz]

Seems like failing cases are being put in RushCommandLineParserFailureCases.test.ts instead

[chengbapi]

I do have this doubt, but seeing that there are also failing cases in L:257, L:269, and L:281, I thought I could put it in this file.

If this is indeed how it is designed, I can move it over.