Option to merge the latest config from $CIRRUS_BRANCH_BASE for PRs

[maflcko]

Currently, the exact commit of the branch of the pull request is used. This is problematic for several reasons:

• Creating a patch on top of a historic commit (to be merged into multiple recent branches) is going to use the cirrus config file of the historic commit as opposed to the config of the branch the patch is going to be merged into
• Using an old config might lead even to a failed build because the old configuration no longer works because packages or the OS itself has reached EOL. (e.g. https://cirrus-ci.com/task/4650278425198592?command=install#L3)

This should be fixed by fetching the config (and the code) from the pull+base_branch merged together. (Btw, GitHub already offers these merged commits).

This would also bring the behaviour in line with other ci providers such as travis/appveyor/etc.

[fkorotkov]

I'm not sure if other CI do use configs from the base branch. I only know that GitHub Actions are always using workflows defined in the default branch. IMO this will make impossible to test changes to .cirrus.yml files.

Will a simple rebase work for your case?

[maflcko]

Other cis use the config not of the base branch, but the base branch merged with the pull request. So changes to .cirrus.yml will still be tested.

You can try it with

git fetch origin 'refs/pull/$PULL_ID/head'
# vs
git fetch origin 'refs/pull/$PULL_ID/merge'

Will a simple rebase work for your case?

In theory yes. In practise, probably no. We'd have to ask everyone to rebase every pull request when a (breaking) change to the .cirrus.yml is merged to master. Sadly we have a lot of open pull requests, so forcing everyone to rebase might be too tedious.

[fkorotkov]

We'd have to ask everyone to rebase every pull request when a (breaking) change to the .cirrus.yml is merged to master.

You'll have the same issue for the existing PRs, you'll need to re-trigger them. Only new PRs will get the rebased config.

Also if a PR is based of a very old branch shouldn't you want the author to rebase regardless?

I think some functionality about automating having an a pretty recent branch can be add to the Rebase Action or the Submit Queue App. 🤔

[maflcko]

Unfortunately we couldn't use GitHub actions due to the permissions they required: bitcoin/bitcoin#18031

Also if a PR is based of a very old branch shouldn't you want the author to rebase regardless?

"on top of a historic commit" was a bit unclear wording from me, it should have probably said "prior to the latest commit that changed .cirrus.yml", which could also be recent.
In general we don't ask authors to rebase their pull requests unless there is an explicit merge conflict. Reason is that our review process is based on the commit ids. The commit ids are changed during a rebase, which our review process currently treats as all review being "reset".

[maflcko]

Btw, for the source code we use a workaround (fetch and merge $CIRRUS_BASE_BRANCH), but that doesn't work for the config, obviously:

https://github.com/bitcoin/bitcoin/blob/8783bcc099e9c8e31010ff59260cf4dbac430355/.cirrus.yml#L34

[maflcko]

Another example we are facing right now is that the memory of one container had to be bumped (OOM). Automatic rebase only works when the author has given permission, not to mention that we try to avoid rebase in some circumstances due to our process. Asking everyone to manually rebase on each push of the roughly 400 pull requests would also be tedious work.

[maflcko]

Adjusted the title from "use" to "merge" because we really want the union of the config in the base branch, plus the config changes in the pull (if there are any).

[fkorotkov]

Will an option to choose where to load the config from work for you? Without the mergin part. I'm thinking of a repository setting liek this:

• Same SHA - behaviour like now
• Target Branch - the latest config of the branch a PR is targeting to
• Default Branch - always load the latest config from the default branch. Similar to what GitHub Actions do.

It's not clear here how to merge the configs in case of conflicts. 🤔

A problem with such settings is that it won't be possible to test the config that easily from a fork.

[maflcko]

If the config doesn't merge with master, CI could skip [1] or fail [2] the build because merging the pull isn't possible anyway without solving conflicts manually. (Alternatively, it could fallback to building Same SHA)

The config really needs to be merged because otherwise it won't be possible to test changes to the config that are made in a pull request.

[1]: I think travis does this.

[2]: appveyor does this:

[fkorotkov]

Just verified this behaviour and indeed Travis merges the changes on PR builds but not branch builds. This really surprises me! IMO more intuitive will be for a PR build to pick the config file from the target branch but if the PR contains changes to the config file itself then to use it without merging target branch files.

Merging two files from different SHAs to get a unique config that doesn't exist in Git history feels too magical to me.

Will Prefer Target Branch for PRs option which will take the latest config from the target branch if there are no changes to the config work for you?

[maflcko]

They don't exist in git history, but they exist on GitHub ;)

[yaml]$ git fetch origin refs/pull/655/merge && git log -1 FETCH_HEAD 
From https://github.com/go-yaml/yaml
 * branch            refs/pull/655/merge -> FETCH_HEAD
commit 1e6516e75521ff7c5d0ffd5b7516e0fd68783fdf
Merge: eeeca48 2a631ff
Author: Feresey <33696249+Feresey@users.noreply.github.com>
Date:   Fri Aug 28 19:47:13 2020 +0000

    Merge 2a631ff564589e5af714c7dde95c773af81349b8 into eeeca48fe7764f320e4870d231902bf9c1be2c08

which is go-yaml/yaml@1e6516e

Will Prefer Target Branch for PRs option which will take the latest config from the target branch if there are no changes to the config work for you?

Sure, this would work as well.