microsoft/rushstack

[rush] Rush shouldn't need to determine the merge base if commit hash has been provided

kenrick95 opened this issue · 2 comments

Summary

I was meeting issue like #2082

However, in this case, I already know the merge base commit (because it is provided as env variable by the CI Runner).

More specifically, in my setup:

I am running in Gitlab CI. CI runner is set to shallow clone with depth = 1. The scripts that I run are:

git fetch origin $CI_MERGE_REQUEST_DIFF_BASE_SHA
git fetch origin $CI_COMMIT_SHA
node common/scripts/install-run-rush.js install
node common/scripts/install-run-rush.js build --verbose --to git:$CI_MERGE_REQUEST_DIFF_BASE_SHA

The first two commands tells the CI runner to fetch the two specific commits (the base and the current commit). The values of both variables are provided by the CI runner.

Rush threw error on the 4th command:

The rush.json configuration requests Rush version 5.117.8
Invoking "rush build --verbose --to git:REDACTED_GIT_COMMIT_HASH"
---------------------------------------------------------------------------------
Rush Multi-Project Build Tool 5.117.8 - https://rushjs.io
Node.js version is 18.19.1 (LTS)
Starting "rush build"
Unable to determine merge base for branch "REDACTED_GIT_COMMIT_HASH". This can occur if the current clone is a shallow clone. If this clone is running in a CI pipeline, check your pipeline settings to ensure that the clone depth includes the expected merge base. If this clone is running locally, consider running "git fetch --deepen=<depth>".

Details

When I investigated the rush source codes, I found that Rush seem to take the provided option as branch name. However, if I pass in a commit hash, Rush shouldn't need to determine the merge base anymore:

const mergeCommit: string = this._git.getMergeBase(targetBranchName, terminal, shouldFetch);

Skipping this part where it determines the merge base will speed up the CI time too since we can fully use a shallow clone of the repository in CI.

Though... I wonder if there's a reliable way to determine whether a provided git expression is a commit or a branch name 🤔 🤔

p.s. specific to my setup, I can do things like git fetch origin $CI_COMMIT_SHA --deepen=10 and it will work-around the issue

Standard questions

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

Question Answer
@microsoft/rush globally installed version? 5.117.8
rushVersion from rush.json? 5.117.8
useWorkspaces from rush.json? yes
Operating system? MacOS
Would you consider contributing a PR? Yes
Node.js version (node -v)? 18.19.1

It seems like this should be working. Are you sure CI_MERGE_REQUEST_DIFF_BASE_SHA is the parent commit of your current commit? To perform diffing, Rush needs all of the commits between your HEAD and the ref you're diffing against. I just tried running rush build --to git:<SHA> and it worked for me, but I didn't try it on a shallow clone.

We usually recommend doing a shallow clone of depth 2, and what you're doing looks like it should be essentially that, so I'm not sure why this isn't working. Being able to take in a commit SHA is a useful feature, so if something is broken, we'd happily take the fix if you can get what you need working.

BTW - you can resolve a ref to a commit by using git rev-parse --verify <ref>, which will return the commit SHA of the ref.

CI_MERGE_REQUEST_DIFF_BASE_SHA is not necessarily the parent commit, but it's the "merge base" of the current branch.

To perform diffing, Rush needs all of the commits between your HEAD and the ref you're diffing against.

So in this snippet of libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

    const mergeCommit: string = this._git.getMergeBase(targetBranchName, terminal, shouldFetch); // (1)

    const repoChanges: Map<string, IFileDiffStatus> = getRepoChanges(repoRoot, mergeCommit, gitPath); // (2)

My understanding is that it doesn't need all the commits between current branch & the given ref, because:

  1. it uses the given "git:" as parameter to getMergeBase (which runs git merge-base -- HEAD <ref> and returns a commit hash)
  2. then it calls getRepoChanges (which runs git diff-index <the commit hash from (1)> --

So I was wondering if the given parameter is already a commit, we can skip (1) and go directly to (2). (2) will work fine since git is able to compare two different commits and return the list of files changed between the two refs, even if the two doesn't share any ancestors.


I just tried running rush build --to git: and it worked for me, but I didn't try it on a shallow clone.

Yup, it will work on a non-shallow clone since the command in (1) will run successfully


Submitted a PR on my proposal: #4592 Please help to check if that makes sense