[rush] ProjectChangeAnalyzer recognizes version+bump changes as diffs

[elliot-nelson]

Summary

Using the new "git change" (git:) project selector, you may want to implement a build step in a CI pipeline that determines whether each service in a monorepo has been affected, and if it has, you'll deploy it.

For example: rush list --from git:$LKG_TAG | grep <some list of prefab projects>

This query has a flaw: the "Version" and "Bump" commits created by your publishing step look like diffs, even though the libraries you are publishing did not change in a meaningful way. This results in downstream services being deployed twice -- the first time after a change that matters in the library, and the second time after the library is published to an NPM rergistry.

   ---> library change ---> merge ---> CI ---> library publish ---> merge ---> CI
                                       |                                       |
                                      \|/                                     \|/
                                    deploy                                  deploy
                                       (good!)                               (no good!)

If we could change the ProjectChangeAnalyzer so that the second deploy didn't happen (either by default or using a special flag), that would be ideal.

Repro steps

To repro, run rush list --only git:$COMMIT, where $COMMIT is any sha before a Rush version+bump commit.

Details

I'm not sure if there's an easy fix to the problem. One possible solution would be to adjust how we calculate the git files changed.

For example, today, ProjectChangeAnalyzer runs a command like this:

git --no-optional-locks diff-index --color=never --no-renames --no-commit-id --cached -z $REVISION

A way you could get a similar output (list of files touched, minus the version + bump commits) might be this series of commands:

git --no-optional-locks format-patch --color=never --no-renames --no-commit-id -z $REVISION -o all-patches
rm all-patches/*-skip-ci.patch
cat all-patches/*.patch | grep '^+++'

Using the same revision as above in the repro, this should return the list of files touched (leading to the list of projects touched), without counting projects that were only touched by [skip ci] commits.

Admittedly, the approach to deciding what to skip is a little messy. If you use the out-of-the-box defaults, your patch files will look like all_patches/0005-Update-changelogs-skip-ci.patch and all_patches/0006-Bump-versions-skip-ci.patch, but that changes if you've customized these messages in your rush config. Perhaps the regex/glob for what commits to skip needs to be configurable as well.

Another possible approach: if this is a bridge too far for the ProjectChangeAnalyzer to handle itself, maybe we could use a new simpler project selector -- files: -- and feed it a list of filenames, which it will break down into project names. Example:

cat all-patches/*.patch | grep '^+++' | cut -c 6- > files.lst
rush list --from files:files.lst

Standard questions

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

Question	Answer
@microsoft/rush globally installed version?	5.70.0
rushVersion from rush.json?	5.70.0
useWorkspaces from rush.json?	Yes
Operating system?	Mac
Would you consider contributing a PR?	Yes
Node.js version (node -v)?	14.x

[iclanton]

My concern is that we don't really have a way to determine if the version bump is or isn't a "meaningful" change. What if a project has a webpack step that bundles its own package.json? Some projects include their own version number in their output.

[elliot-nelson]

My concern is that we don't really have a way to determine if the version bump is or isn't a "meaningful" change. What if a project has a webpack step that bundles its own package.json? Some projects include their own version number in their output.

It's worth noting such a project would not work with our (or Rushstack's) publish pipeline today. rush version && rush publish would need another heft build in the middle, to get the just-changed version inside the bundle before publishing.

But, let's say you did want that workflow, and you did design that publishing pipeline. I think my suggestion is still compatible with the way Rush works:

1. Post-merge CI starts, does an imaginary rush list --impacted-by $GIT_LKG --ignore-skipped-commits. The libraries that were just bumped and published are ignored, so they and their dependencies are not included. However! Various apps that do depend on that library are still included in the list due to other changes.
2. For each affected project, rush build --to <project> runs. This command does not ignore skipped commits, so when it attempts to pull cached builds for each library, it doesn't grab old ones, it does get the latest and greatest lib/ and dist/ folders (or it has to build them inline if not cached yet).
3. Now a rush deploy happens with various services, including the latest webpack libraries bundled into the service being deployed.

I think the key thing is that the concept of "ignoring skipped commits" is a querying tool, not a building tool. It gives you a list of targets, but you'd never skip commits while e.g. computing a build cache hash.