rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[rush] --changed-projects-only is broken with buildCacheEnabled=true

Open octogonz opened this issue 3 years ago • 1 comments

Summary

[See this Zulip chat thread]

The rush build --changed-projects-only command is supposed to build only projects whose folders contain a source file that was changed since the last successful build by Rush. This feature is broken when buildCacheEnabled=true.

Details

This feature is implemented here:

https://github.com/microsoft/rushstack/blob/3be2c97ece8c962f1612a1533868411657f7c80d/libraries/rush-lib/src/logic/operations/OperationExecutionManager.ts#L350-L357

...which sets isSkipAllowed=false:

      if (blockSkip) {
        item.runner.isSkipAllowed = false;
      }

But as part of fixing #3311, the isSkipAllowed setting was disabled generally when the build cache is enabled:

https://github.com/microsoft/rushstack/blob/70619dc3ad1e9e78f7f0b5dc56e9f71b7388c0c3/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts#L411-L413

Thus --changed-projects-only no longer works. Architecturally, the build cache technically never skips projects based on changes. It always either builds them or extracts a tarball in that folder. Thus the old implementation isn't even on the right track.

Why didn't we notice this earlier? The --changed-projects-only parameter is not very clearly documented. People didn't clearly understand what it was supposed to do, so they didn't notice that it was broken.

The requirements were like this:

  1. I successfully build the relevant projects using Rush (not rushx)
  2. I modify some arbitrary source files
  3. When I do rush build --changed-projects-only, it calculates which files have changed since step 1, and rebuilds /only/ the projects containing those folders (but NOT the upstream/downstream "impacted by" projects)

(The change detection today is using the "skip detection" logic from the old incremental build.)

A new proposal

After chatting with @dmichon-msft, we arrived at the idea that change detection can be separated from "skipping."

  1. Let's deprecate --changed-projects-only, and make a small PR that reports an error if someone tries to use this command with buildCacheEnabled=true
  2. Instead, we will introduce a new project selection token. Tentatively, let's call it git:LAST_BUILD. Using Git combined with package-deps-hash, this token represents the set of projects containing a diff since their last successful build. (This diff is already tracked internally by Rush.)
  3. Instead of rush build --changed-projects-only the new syntax will be rush build --only git:LAST_BUILD. And you can do other combinations like rush build --from git:LAST_BUILD or rush build --to git:LAST_BUILD --only example-project.
  4. In the documentation, this will be marked as an (unsafe) command -- similar to --only, it can produce an invalid build, so it trusts that the user knows what to do
  5. @dmichon-msft also had ideas about how to make this work with Rush's "watch mode," however that should be tested separately, and should preserve the exact same semantics as above

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
rushVersion from rush.json? 5.75.0

octogonz avatar Jun 30 '22 01:06 octogonz

First steps:

  1. We need someone to make a small PR that blocks --change-projects-only when buildCacheEnabled=true, referring people to this GitHub issue
  2. The tentative name git:LAST_BUILD probably conflicts with Git's namespace -- we need to come up with a better token proposal

octogonz avatar Jun 30 '22 01:06 octogonz