Reject patches creating empty commits #1063
Labels
Comments
xmo-odoo
added a commit
that referenced
this issue
Feb 25, 2025
Verify that the tree is different before and after applying the patch, otherwise if there's a mistake made (e.g. a script does not check that they have content and request a patch applying an existing commit which is how odoo/enterprise#612a9cf3cadba64e4b18d535ca0ac7e3f4429a08 occurred) we end up with a completely empty commit and a duplicated commit message. Fixes #1063 Note: using `rev-parse` to retrieve the commit's tree would be 50% faster, but we're talking 3.2 versus 2.4ms and it requires string formatting instead of nice clean arguments so it's a bit meh.
xmo-odoo
added a commit
that referenced
this issue
Feb 25, 2025
Commits can take some time to propagate through the network (I guess), or human error can lead to the wrong commit being set. Either way, because the entire thing was done using a single fetch in `check=True` the cron job would fail entirely if any of the patch commits was yet unavailable. Update the updater to: - fallback on individual fetches - remove the patch from the set of applicable patch if we (still) can't find its commit I'd have hoped `fetch` could retrieve whatever it found, but apparently the server just crashes out when it doesn't find the commit we ask for, and `fetch` doesn't update anything. No linked issue because I apparently forgot to jot it down (and only remembered about this issue with the #1063 patching issue) but this was reported by mat last week (2025-02-21) when they were wondering why one of their patches was taking a while: - At 0832 patch was created by automated script. - At 0947, an attempt to apply was made, the commit was not found. - At 1126, a second attempt was made but an other patch had been created whose commit was not found, failing both. - At 1255, there was a concurrency error ("cannot lock ref" on the target branch). - Finally at 1427 the patch was applied. All in all it took 6 hours to apply the patch, which is 3-4 staging cycles.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
odoo/enterprise@612a9cf3cadba64e4b18d535ca0ac7e3f4429a08 is an empty commit, looking at the events log for the repo it was pushed by robodoo so it's not any sort of human mistake or anything.
The answer lies in the patcher system, which has a job requesting commit 33509a09535644e5e27ea38db2a9ecf657795bd1
odoo/enterprise@33509a09535644e5e27ea38db2a9ecf657795bd1 is an older commit on 18.0
The issue flow is:
The low level git model does not really care about empty commits, checks for that are handled in higher level commands, which the mergebot generally can not use (because they tend to require a working copy, which it doesn't have).
The mergebot has a check for creating empty commits while staging (b21fbaf) however I almost certainly forgot about this condition when implementing the patcher, likely because I didn't consider that other employees could be faillible (somehow).
The text was updated successfully, but these errors were encountered: