Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Share the nobuild GitHub workflow via dev-tools #20

Closed
wants to merge 1 commit into from

Conversation

CodingCanuck
Copy link
Contributor

Stubbing out workflows with replacement no-op workflows is the
recommended way to skip required workflows for some file types but to
require them for others.

A major downside of sharing only some workflows like this is that file
path inclusions / exclusions need to be synchronized across repositories
which can't be done atomically.

See 3rdparty/eventuals#176 and
https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks

Stubbing out workflows with replacement no-op workflows is the
recommended way to skip required workflows for some file types but to
require them for others.

A major downside of sharing only some workflows like this is that file
path inclusions / exclusions need to be synchronized across repositories
which can't be done atomically.

See 3rdparty/eventuals#176 and
https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks
@CodingCanuck CodingCanuck requested a review from benh January 11, 2022 01:28
@CodingCanuck
Copy link
Contributor Author

@benh WDYT?

I don't like how this is part is shared across repos:

  pull_request:
    branches:
      - "main"
    paths:
      - '**.md'

For two reasons:

  1. some repos use "main" (e.g. https://github.com/3rdparty/bazel-rules-rapidjson ) but others use "master" ( https://github.com/3rdparty/stout-stateful-tally ) or something else like "reboot-main" ( https://github.com/reboot-dev/envoy ). I wouldn't mind if all repos used a consistent main branch name, but they don't today.

  2. paths: since half of the (nobuild) workflow files are here and the other half (macos / ubuntu / windows) are scattered across our other repos, you can't atomically update the ignore paths: instead you have to do a multi-part commit where you first add more included paths in the dev-tools half then you followup with more ignored paths in every other repo that relies on this.

Reason 2 would be resolved by your suggestion in 3rdparty/eventuals#176 (comment) to share each platform's build and test workflows in dev-tools as well. A nice thing about centralizing this setup is it would be easier to make sure all of our repos have a continuous integration setup, which several (most?) repos currently lack: see discussion in 3rdparty/bazel-rules-curl#11 (comment)

I'm not sure what would be involved in sharing build workflows across repos. They currently differ in what look like material ways (though maybe we can either re-implement this, or omit this run skipping?) (one vs other), but also in ways that look like bugs (this file references a "master" branch that doesn't exist). This makes me think that it's worth an effort to unify these if possible. But I don't know what blockers that might hit or how long that might take.

@while-false
Copy link
Contributor

while-false commented Jan 11, 2022

Thanks for digging into this @CodingCanuck, it's a big and messy one though. In the past I have requested a unified (bazel) build action that we can use across the different (bazel) repos. I strongly belive that having a central repo with the actions and workflows is the only maintainable solution. In line with the discussion you refer to, I believe that this should address (2).

In regards to your (1). Yes, it is a bit inconveneint that different repos use different branch names for the main branch, however I think it is at most 3, namely master (legacy), main (new default), reboot-main (where we have forked an external project and should not consfuse our main with upstream main). To mitigate (1) I see 2 options, namely:

  1. Extend the branch clause in the script to trigger on either of the 3 main branch names we have. Something like:
  pull_request:
    branches:
      - "main"
      - "master"
      - "reboot-main"

or (if possible):

  1. Try and work around it using the environment variables available. I notice that there is a GITHUB_REF_PROTECTED variable available "if branch protections are configured for the ref that triggered the workflow run.". I do not have a crisp picture of how this can done in the workflow - maybe you do.

@benh
Copy link
Member

benh commented Jan 17, 2022

IIUC, to reuse the workflow you still have to have your own workflow file that calls the reused workflow, as shown here. So it seems like each of those workflow files can specify the pull_request: branches: themselves? Alternatively it looks like you can pass inputs to the reusable workflow. So maybe we can pass the branch name?

It is interesting that the amount of code to call the reusable workflow in this case is almost the same as having the workflow copied everywhere. I suppose once we add the comments in nobuild.yml explaining why it works then it'll be worth it?

@CodingCanuck
Copy link
Contributor Author

Closing: we're abandoning this build-skipping approach. See 3rdparty/eventuals#309 and https://github.com/reboot-dev/respect/pull/294 and 3rdparty/stout#25 .

@CodingCanuck CodingCanuck deleted the alexmc.reusable-workflows branch April 6, 2022 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants