CI: silent merge check #33145

pull m3dwards wants to merge 4 commits into bitcoin:master from m3dwards:250806-ci-silent-merge changing 5 files +256 −0
  1. m3dwards commented at 2:53 pm on August 6, 2025: contributor

    With the upcoming potential migration to cirrus runners (#32989) we need a way to periodically check for a silent merge conflict. Cirrus CI currently does this every week.

    This is part of a proposal that will look for a label on a PR as a trigger to run the lint and get_previous_releases test. This label could be added manually or better, Drahtbot could periodically add (and remove) this label.

    An example of this running on a test org: https://github.com/testing-cirrus-runners/bitcoin/actions/runs/16780213204/job/47516540034

  2. ci: add caching actions
    Add "Restore" and "Save" caching actions.
    
    These actions reduce boilerplate in the main ci.yml configuration file.
    
    These actions are implemented so that caches will be saved on `push`
    only.
    
    When a pull request is opened it will cache hit on the caches from the
    lastest push, or in the case of depends will hit on any matching depends
    hash, falling back to partial matches.
    
    Depends caches are hashed using
    `$(git ls-tree HEAD depends "ci/test/$FILE_ENV" | sha256sum | cut -d' ' -f1)`
    and this hash is passed in as an input to the actions. This means we
    direct cache hit in cases where depends would not be re-built, otherwise
    falling back to a partial match.
    
    The cirruslabs cache action will fallback transparently to GitHub's
    cache in the case that the job is not being run on a Cirrus Runner,
    making these compatible with running on forks (on free GH hardware).
    4f15d6918c
  3. ci: add configure environment action cdbaffa179
  4. ci: add configure-docker action
    Another action to reduce boilerplate in the main ci.yml file.
    
    This action will set up a docker builder compatible with caching build
    layers to a container registry using the `gha` build driver.
    
    It will then configure the docker build cache args.
    f1687d74d0
  5. ci: add silent merge ci check
    This workflow will be triggered when the label 'PeriodicMergeCICheck' is
    added to a PR. This label could be added manually or periodically by
    Drahtbot. It will check that since the PR was opened that there isn't a
    silent merge conflict that prevents compilation or tests passing.
    
    Co-authored-by: will <will@256k1.dev>
    1b5747233f
  6. DrahtBot added the label Tests on Aug 6, 2025
  7. DrahtBot commented at 2:53 pm on August 6, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33145.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32989 (ci: Migrate CI to hosted Cirrus Runners by willcl-ark)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  8. in .github/workflows/silentmerge.yml:39 in 1b5747233f
    34+            echo "::notice title=Runner Selection::Using GitHub-hosted runners"
    35+          fi
    36+
    37+  previous-releases:
    38+    name: 'Previous releases, depends DEBUG'
    39+    if: contains(github.event.label.name, 'PeriodicMergeCICheck')
    


    maflcko commented at 3:09 pm on August 6, 2025:

    Seems fine, but currently a vector of task names is accepted, see --task in https://github.com/maflcko/DrahtBot/blob/2038d4d541b89bf2601614b5656d7d30d73e17be/rerun_ci/src/main.rs#L7-L23

    It would be good to allow the same somehow?


    m3dwards commented at 2:29 pm on August 7, 2025:
    Can you elaborate a little more on what this would look like? Would this be different labels for different jobs? As in a label that runs the Previous Releases job and a different label that would run a different job?

    maflcko commented at 2:40 pm on August 7, 2025:

    I don’t know. Not sure if we want to see pull requests spammed with “added and removed” label events (https://github.com/testing-cirrus-runners/bitcoin/pull/19#event-19014793971).

    GitHub doesn’t make this easy and I wonder if we may want to just spin our own CI to detect silent merge conflicts.


    maflcko commented at 2:56 pm on August 7, 2025:

    To give some more context: We are now trying to add 200+ lines of code to this repo which have to be maintained, but they cause label spam, and likely aren’t sufficient to achieve the goal, because the ci-failed notification will go to the one adding the label and triggering the workflow?

    If so, we’d have to create a comment (or other notification) anyway. So if additional glue code is needed, we might as well handle all code externally.


    m3dwards commented at 4:14 pm on August 8, 2025:
    It’s a fair comment. I’ll see what the ci-failed notification could look like but you could be right that this approach is less than ideal. @0xB10C has suggested perhaps merging this into the main CI file but might come with a verbose predicate before each job.
  9. Sammie05 commented at 7:54 pm on August 6, 2025: none

    Nice work on this! The label-based trigger for PeriodicMergeCICheck is a clean approach.

    One thought though since this is driven by label events, do we need to consider cases where multiple labels are applied at once? Would the condition still behave as expected?

    Also agree with the suggestion from @maflcko having a vector of task names could as well improve flexibility

  10. maflcko commented at 8:32 am on August 7, 2025: member
    Other stuff to check would be that this works at all and doesn’t just re-run an ancient commit, like #32989 (comment)
  11. m3dwards commented at 2:26 pm on August 7, 2025: contributor

    Other stuff to check would be that this works at all and doesn’t just re-run an ancient commit, like #32989 (comment)

    Just ran a test to specifically test for this by updating the default (master) branch and adding and removing the label and it correctly picked an updated merge commit based on the PR merged into the updated master.

  12. 0xB10C commented at 10:41 am on August 8, 2025: contributor

    Have you considered doing something like the following in .github/workflows/ci.yml to avoid having to maintain separate files?

     0name: CI
     1
     2on:
     3  push:
     4  pull_request:
     5    types: [opened, reopened, synchronize, labeled]
     6
     7jobs:
     8  conditional-job:
     9    if: >
    10      github.event_name == 'push' ||
    11      (
    12        github.event_name == 'pull_request' &&
    13        (
    14          github.event.action == 'opened' ||
    15          github.event.action == 'reopened' ||
    16          github.event.action == 'synchronize' ||
    17          (
    18            github.event.action == 'labeled' &&
    19            contains(github.event.label.name, 'PeriodicMergeCICheck')
    20          )
    21        )
    22      )
    23    runs-on: ubuntu-latest
    24    steps:
    25      - run: echo "Running CI job"
    
  13. m3dwards commented at 4:10 pm on August 8, 2025: contributor

    Have you considered doing something like the following in .github/workflows/ci.yml to avoid having to maintain separate files?

    Could be a good shout, seems a bit of a verbose check if that has to be included in each job. I’ll have a think about it. This is exactly the type of feedback that’s good to consider.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-08-12 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me