[BUG] Locate the merge-base of a PR branch instead of a relying on the fetch-depth.

[lpulley]

Is there an existing issue for this?

• I have searched the existing issues

Does this issue exist in the latest version?

• I'm using the latest release

Describe the bug?

We tried fetch-depth: 0, but it was too slow in our large repository. fetch-depth: 2 usually works, but occasionally changed-files seems to be determining the wrong GITHUB_EVENT_PULL_REQUEST_BASE_SHA.

To Reproduce

We're encountering this issue on a private repository. What I can say is that in this case, I have a developer who branched from the main branch, did work, committed a few commits, and created a PR. In the meantime, the main branch had some commits merged into it. Business as usual.

All the previous changed-files jobs in this PR worked fine, but the most recent one didn't. Its GITHUB_EVENT_PULL_REQUEST_BASE_SHA seems to be wrong -- the GitHub merge commit that the job ran on has a parent that is a descendant of GITHUB_EVENT_PULL_REQUEST_BASE_SHA.

We check out 746c624:

whose main branch parent is df0d28b:

and for some reason changed-files is expecting to find df0d28b's ancestor e7b29ba:

but it can't, so it complains that we haven't fetched deeply enough:

What OS are you seeing the problem on?

all

Expected behavior?

I expect fetch-depth: 2 to work as it usually does. I also would expect GITHUB_EVENT_PULL_REQUEST_BASE_SHA to be the parent of the check merge commit, and not some older ancestor of it.

Relevant log output

No response

Anything else?

Git version is 2.38.1. Runner is our own (self-hosted).

Code of Conduct

• I agree to follow this project's Code of Conduct

[github-actions]

Thanks for reporting this issue, don't forget to star this project if you haven't already to help us reach a wider audience.

[jackton1]

@lpulley Given that this is an expected behaviour I’ll recommend you take a look at https://github.com/orgs/community/discussions/25437#discussioncomment-3247910

You can also increase the depth of the base branch history that is pulled by using the target_branch_fetch_depth input.

See inputs for more information.

[lpulley]

I understand how the GitHub merge commits work and that they are not entirely up to date with the target branch if the target branch moves after the PR has been synchronized. I get that.

My problem is that changed-files is occasionally (but not most of the time, apparently) expecting that some arbitrary (to me) ancestor of the GitHub merge commit is checked out already, when it is not -- fetch-depth: 2 fetches the GitHub merge commit and its parents, from which changed-files usually does some more checkouts, figures out the merge base, and provides the changed files.

The reason I believe this is a bug is that in the case I outlined above, changed-files is failing with fetch-depth: 2 because it is expecting to already have a commit that is not one of the commits that would be checked out with fetch-depth: 2. Doesn't that seem like a problem?

[lpulley]

Are you saying that this is happening because it's giving up after fetching 20 commits back by default?

[lpulley]

@jackton1 is there a reason why changed-files doesn't just git fetch --deepen until the merge base is found? That way, this would never just fail.

[jackton1]

Are you saying that this is happening because it's giving up after fetching 20 commits back by default?

Yes

[jackton1]

@jackton1 is there a reason why changed-files doesn't just git fetch --deepen until the merge base is found? That way, this would never just fail.

You can increase the number i.e target_branch_fetch_depth: 100for now. I'll take a stab at implementing it this weekend.

[jackton1]

@lpulley @ericLemanissier This should now be available in the latest release.