[rush] Add the phased command feature-related properties to command-line.json

[iclanton]

Summary

This PR adds properties and the related validation for the phased commands feature (#2300) to command-line.json.

How it was tested

Ensured that these files validate correctly on this repo: https://github.com/elliot-nelson/rush-phased-builds-example/tree/phased

[elliot-nelson]

👍 Was looking over latest commits and everything makes sense to me.

Was chatting with @octogonz about this skipPhasesForCommand idea and something he highlighted was that the meaning of this is not intrinsic to the parameter, but to the situation it is running in.

As an example, suppose you define a parameter --no-compile:

{
    "name": "--no-compile",
    "parameterKind": "flag",
    "skipPhasesForCommand": ["_phase:compile"],
    "associatedCommands": ["build", "update-readme"]
}

// note: build is defined as ["_phase:compile", "_phase:lint", "_phase:test"]
// note: update-readme is defined as ["_phase:compile", "_phase:update-readme"]

Running rush update-readme --no-compile is totally safe, because the phases _phase:compile and _phase:update-readme do not depend on each other. Rush should be able to correctly read/write from build cache for only the phases that are run (in this case, just update-readme).

However, running rush build --no-compile is unsafe. It is asking Rush to pretend it has built each project, without reading or writing the build cache. This idea of an "unsafe" task is the same as --only (when selecting projects). In both cases, Rush should disable writing any build cache entries, because the results aren't deterministic. (I'm actually not sure if using --only invalidates the build cache but it probably should.)

(Although similar to this new parameter, --only ALSO depends on what's actually in the tree to traverse. Running rush build --only stand-alone-project is not an unsafe operation. So in both cases "safety" depends on the situation.)

Ideally:

• An unsafe operation (but only one that is ACTUALLY unsafe, not just POTENTIALLY unsafe) should disable any build cache writes.
• We perhaps should notify the user somehow that they are performing an unsafe operation.

[iclanton]

@elliot-nelson, I think I've addressed all of the comments. Here's a change to your example repo with changes we discussed here elliot-nelson/rush-phased-builds-example#3

[elliot-nelson]

@iclanton Merged!

I notice the addition of incremental on the phases, but that does leave the command definition of build and rebuild in the example repo identical... I wonder if we did that wrong.

Some possible solutions:

(1) Move incremental to commands only, so you can define build and rebuild phasedCommands.

(2) Have incremental property on both commands and phases. (The issue is deciding which wins, which seems sort of non-obvious.)

(3) Keep incremental property on only phases. This would imply that the maintainer has to create non-incremental versions of the phases, i.e.:

// build
"phases": ["_phase:compile", "_phase:lint", "_phase:test"]

// rebuild
"phases": ["_phase:recompile", "_phase:relint", "_phase:retest"]

This might be the most correct/flexible option but it seems pedantic for the repo maintainer, and we would need to solve cache hitting for [x]/re[x]. (Maybe with something like David's PR #2897.)

[iclanton]

Hmm the build vs rebuild thing is a good point. I think the simplest option will be to move incremental to the command instead.

Made that change, and made a PR against the example repo: elliot-nelson/rush-phased-builds-example#4

[octogonz]

🎉 I'm super excited to see this feature moving along! 🎉