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

CI: add reusable actions #2951

Merged
merged 1 commit into from
Sep 11, 2023
Merged

CI: add reusable actions #2951

merged 1 commit into from
Sep 11, 2023

Conversation

fufexan
Copy link
Member

@fufexan fufexan commented Aug 12, 2023

Describe your PR, what does it fix/add?

Allows reusing actions across the org, meaning less copied code and less chances of errors slipping in when a change is made.

  • Nix: define strategy, which builds in parallel, faster than doing nix flake check (which does not guarantee the builds are run as the dependency graph dictates.)

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

See my comment below.

Is it ready for merging, or does it need work?

I also want to create a conditional job:

  • when wlroots gets updated, don't run Nix builds
  • when there's no wlroots update, run the builds

The above conditional was replaced by simply running the wlroots update first, and re-cloning HEAD from the branch on the subsequent package build job.

@memchr
Copy link
Contributor

memchr commented Sep 11, 2023

FYI, there is a tool called act that lets you run and debug github actions locally, it's in nixpkgs I think.

@fufexan
Copy link
Member Author

fufexan commented Sep 11, 2023

Damn, didn't know that. Thanks. However I think I've mostly finished this PR.

@fufexan
Copy link
Member Author

fufexan commented Sep 11, 2023

Ok, so if there are both a pull_request and a push trigger, the wlroots update action will fail for one of them. Doesn't affect builds at all though. This should happen very rarely, only if vax decides to bump wlroots inside a PR. I think it's good enough for now.

Alternatively I could make it so that pull_request is never run if the PR ref is from this repo, but it's not worth the complication.

@fufexan fufexan merged commit ed51fe7 into main Sep 11, 2023
20 checks passed
@memchr
Copy link
Contributor

memchr commented Sep 12, 2023

Maybe add a condition to skip this job in PR?

wlroots / wlroots ( nix-update-wlroots.yml )
Secret PAT is required, but not provided while calling.

Every PR has failed this check since this commit. It generates extra noise in emails.

@fufexan
Copy link
Member Author

fufexan commented Sep 12, 2023

Oops. Okay, I'll fix it.

@memchr
Copy link
Contributor

memchr commented Sep 12, 2023

if: ${{ !startsWith(github.ref, "refs/pull/") && !endsWith(github.ref, "/merge") }}

@fufexan
Copy link
Member Author

fufexan commented Sep 12, 2023

I came up with something like this

name: Nix

on: [push, pull_request, workflow_dispatch]

jobs:
  wlroots:
    if: github.event_name == 'push'
    uses: ./.github/workflows/nix-update-wlroots.yml
    secrets: inherit

  build:
    if: always() && !cancelled() && !contains(needs.*.result, 'failure')
    needs: wlroots
    uses: ./.github/workflows/nix-build.yml
    secrets: inherit

since we still want the packages to be built even if wlroots updating was skipped.

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.

2 participants