Varzaru Razvan-Liviu added a comment - 2025-03-18 16:59 - edited cvicentiu / danblack I think we should consider telling buildbot to track refs/pull/#ID#/head. What do you think? https://docs.buildbot.net/latest/manual/configuration/wwwhooks.html https://docs.buildbot.net/latest/manual/configuration/steps/source_github.html pullrequest_ref ( default merge) Remote ref to test if a pull request is sent to the endpoint. See the GitHub developer manual for possible values for pull requests. (e.g. head) Something like: c[ "www" ][ "change_hook_dialects" ] = { "github" : { "secret" : config[ "private" ][ "gh_secret" ], "strict" : True, "pullrequest_ref" : "head" } } One of the best explanations I've read so far that motivates me to switch to refs/pull/head https://github.com/readthedocs/readthedocs.org/issues/6958#issuecomment-655203986 And we already have enforced that the developer must rebase before merging, at this point pull/head == pull/merge The Require branches to be up to date before merging box is checked on our branches.

Daniel Black added a comment - 2025-03-19 02:31 manual test $ git fetch https://github.com/MariaDB/server refs/pull/3826/merge remote: Enumerating objects: 56, done. remote: Counting objects: 100% (37/37), done. remote: Compressing objects: 100% (14/14), done. remote: Total 56 (delta 25), reused 23 (delta 23), pack-reused 19 (from 2) Unpacking objects: 100% (56/56), 463.68 KiB | 827.00 KiB/s, done. From https://github.com/MariaDB/server * branch refs/pull/3826/merge -> FETCH_HEAD (base)   $ git show FETCH_HEAD commit 0202b071ab0bcf6cf8663f48c1ab934a3f1b8f33 Merge: 839828e57fd ca561443a28 Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Mon Mar 17 08:52:31 2025 +0100   Merge ca561443a28dfbd1b3fb6315187315a12e9fa221 into 839828e57fdf734b15c81cb9cb76d3760a9161f1   $ git fetch https://github.com/MariaDB/server refs/pull/3826/head remote: Enumerating objects: 31, done. remote: Counting objects: 100% (31/31), done. remote: Compressing objects: 100% (14/14), done. remote: Total 31 (delta 16), reused 31 (delta 16), pack-reused 0 (from 0) Unpacking objects: 100% (31/31), 266.66 KiB | 3.33 MiB/s, done. From https://github.com/MariaDB/server * branch refs/pull/3826/head -> FETCH_HEAD (base)   $ git show FETCH_HEAD commit 183498270e06bdbd37169c20f37bc184088daf69 Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Tue Mar 18 18:39:39 2025 +0200 So it looks like the merge reference is a stale cache - as it has been for a long time it seems https://github.com/orgs/community/discussions/21553 Looks fetching head is going to be needed, however our BB step might need to attempt the merge itself, as its this impact of the merge that we want protected branches to test.

Varzaru Razvan-Liviu added a comment - 2025-03-19 07:31 - edited danblack I do not entirely agree. I believe it is important for the developer's code version to be tested so that during development, the result of their work is not influenced by a pseudo-merge that could introduce bugs from integrating with what has been contributed to upstream in the meantime. Not testing this pseudo-merge helps the developer ensure that bugs encountered in CI are easily reproducible in their local environment. By having the 'Require branches to be up to date before merging' option checked, integration testing will still take place within the pull request, as the developer will be required to bring in the latest changes from upstream to their branch, at which point refs/pull/head will be equal to refs/pull/merge. The life cycle of a pull request in the case of refs/head would be: The developer opens a pull request with their contribution. The CI tests their code. The code goes through iterations, during which the developer may or may not synchronize the branch with updates from upstream while fixing bugs. When the code is ready, free of bugs, the developer is required by policy to bring in the changes from upstream in order to merge. After the previous action, integration testing takes place, where if bugs appear, the code is further refined.

Daniel Black added a comment - 2025-03-19 21:44 Yep, I'm ok with that.

Varzaru Razvan-Liviu added a comment - 2025-04-02 14:40 cvicentiu or danblack , review please? https://github.com/MariaDB/buildbot/pull/734