Semantic Conflicts on PR merge

Hey everyone,

we rarely have semantic conflicts when merging PRs.

A semantic conflict might happen if two PRs are merged at the same time with their CI checks from the same base. Even though the individual checks work out fine and the combination git merges without issue, they might fail.

An example would be one PR requiring each feature to be present in both Makefile.dep and Kconfig, and another PR adding a feature to just one of them. When merged at the same time, they conflict.

Rust is using a tool called borg to prevent this. See https://bors.tech for more info.

I don’t think this is feasible for RIOT-OS/RIOT ATM for performance reasons, but I’d like to set this up for RIOT-OS/riotdocker. In that repo, we already require the CI results to be from a build agains current master, requiring quite some manual rebasing and rebuilding.

What do you think, should we give this a try for the riotdocker repository?

image

This basically tests before merging?

It is probably good to get some experience with a tool like this.

Maybe if we do want to apply it to RIOT we can set a minimal test (static tests, and build tests for only a few boards)

Well, CI does test and only allow merging when the tests pass. On RIOT-OS/RIOT, there can still be semantic conflicts, because the test result is not invalidated on master merge.

bors would, when you comment “bors +r”, collect currently waiting PRs, push them to the “staging” branch, build them all in combination against the current master, then merge on success.

It would replace the regular “press merge button to merge” workflow.

For riotdocker, we do invalidate outdated test results. So if multiple PR’s have passed tests, then one gets merged, the other PRs need to be rebased (IIUC) to trigger a re-build against current master.

bors would do that, and if there are multiples that do that at the same time, merge them together and then if the tests succeed, merge into master together.

Yeah. I guess we can assume it works fine. It is simple enough and Rust uses it (which has a massive PR turnaround).

That’s why I propose to use it first for riotdocker, as that already enforces the same strictness, but requires manual interaction (manual rebase/retrigger) to do so.

1 Like

That’s why I propose to use it first for riotdocker, as that already enforces the same strictness, but requires manual interaction (manual rebase/retrigger) to do so.

Ah, and riotdocker is using travis, which makes it trivial to actually implement this.

If this is no additional effort for the maintainer that clicks merge and doesn’t take much extra time… it sounds like a good idea to avoid these edge cases that are annoying to debug.

well, I imagine the biggest hurdle to overcome is a mental one - we’d no longer click “merge”, but add a comment saying “bors +r”…

bors repo looks promising. Reading their example I am astounded that github even allows these problems.

Is there no smarter way to trigger bors with a “merge with bors” button and make that the default button (like “squash and merge”)? EDIT: there probably isn’t or the bors guys would tell.

not that I now of :frowning:

at least github allows to invalidate outdated builds…