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

chore: try GitHub Action #2166

Merged
merged 5 commits into from
Nov 8, 2023
Merged

chore: try GitHub Action #2166

merged 5 commits into from
Nov 8, 2023

Conversation

SteveLauC
Copy link
Member

@SteveLauC SteveLauC commented Oct 8, 2023

What does this PR do

Our first try with GHA

This PR migrates our:

  1. BUILD
  2. TEST

to two custom GitHub composite actions, and uses them on x86_64-apple-darwin

Update: 33 tasks (we have 35 in total) have been migrated, see my last comment

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@SteveLauC SteveLauC changed the title chore: try GitHub Action [skip ci] chore: try GitHub Action Oct 8, 2023
@SteveLauC
Copy link
Member Author

Well, no idea why GHA is not running with this PR, [skip ci] in the PR title should not disable it(but only the Cirrus CI) as I didn't find anything related to PR title in their documentation

@SteveLauC SteveLauC mentioned this pull request Oct 8, 2023
@Jan561
Copy link
Contributor

Jan561 commented Oct 8, 2023

I've found something that sounds interesting:

https://github.com/cirruslabs/cirrus-cli/blob/master/INSTALL.md#github-actions

If I understand this correctly, the Cirrus CLI allows tasks configured for Cirrus to be run in other CIs, including GHA

@SteveLauC
Copy link
Member Author

I've found something that sounds interesting:

https://github.com/cirruslabs/cirrus-cli/blob/master/INSTALL.md#github-actions

If I understand this correctly, the Cirrus CLI allows tasks configured for Cirrus to be run in other CIs, including GHA

This is cool, just installed Cirrus CLI on my host, it can execute a specific task, which means if we separate the jobs that should be migrated to GHA into new tasks, then everything should just work?

@Jan561
Copy link
Contributor

Jan561 commented Oct 8, 2023

I hope so, that would be very nice. I'll play around a bit on my fork too and keep you updated

@SteveLauC
Copy link
Member Author

SteveLauC commented Oct 8, 2023

I hope so, that would be very nice. I'll play around a bit on my fork too and keep you updated

Thanks!


I just migrated 33 tasks to GitHub Action:

  1. macOS
  2. cross(QEMU)
  3. Linux Amd64 Native Builds
  4. Latest Rust Stable
  5. Corss-compiling
  6. Redox
  7. Tier3
  8. Minver
  9. Rustfmt

leaving the following tasks on Cirrus

  1. FreeBSD
  2. Linux Arm64 Native Builds

You can preview it here


Both ways are acceptable if they work!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Jan561
Copy link
Contributor

Jan561 commented Oct 8, 2023

I've looked into GHA + Cirrus CLI, and I believe the primary usecases for that CLI are

  • You need a self-hosted runner that receives certain tasks from the Cirrus Cloud (Persistent Worker) or
  • You really like the Cirrus-CI configuration format and want to use them for other CIs as well (while not using Cirrus)

I didn't find a way to tell Cirrus to skip certain tasks that get handled by GHA, other than setting up a persistent worker in GHA, letting it fetch and execute some tasks from the Cirrus Cloud and stopping it afterwards.

I think a separate configuration for GHA is by far the better option.

Edit: I got it working ... at least a little bit. I marked the tasks that should run on GHA as manual, and called them explicitly in the GHA workflow.

I ended up with

'Linux arm gnueabi' Task
  Started 'Linux arm gnueabi' Task
  unsupported instance type: got nil instance which means it's probably not supported by the CLI
  'Linux arm gnueabi' Task skipped in 0.0s!

No idea why this fails, and it's probably not worth the effort to get this working, so I suggest we should just use your approach

@SteveLauC
Copy link
Member Author

Thanks for taking the time to investigate this tool!

Copy link
Contributor

@Jan561 Jan561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked out your workflow files on my fork. These are the changes I needed to implement to make the CI pass.

.github/actions/build/action.yml Show resolved Hide resolved
.github/actions/test/action.yml Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
@SteveLauC
Copy link
Member Author

@Jan561 Thanks for testing it on your fork, I can see that it is working here. And I am actually surprised by how fast GHA is, it finishes 33 tasks in 6 minutes, unbelievable...

@Jan561
Copy link
Contributor

Jan561 commented Oct 10, 2023

You're welcome, I'm mind blown as well. And I've double-checked: no quotas, no fees, no limits, completely free (at least for me as a personal account, but I remember reading that this applies to all open-source projects)

@Jan561
Copy link
Contributor

Jan561 commented Oct 12, 2023

Optional, but we could allow GHA to check all branches, by setting

on:
  pull_request:
    types: [opened, synchronize, reopened]
  push:
    branches:
      - '*'

Not required for the nix repository though, but this would allow forks to use the CI on other branches as well

@SteveLauC
Copy link
Member Author

Optional, but we could allow GHA to check all branches, by setting

on:
  pull_request:
    types: [opened, synchronize, reopened]
  push:
    branches:
      - '*'

Not required for the nix repository though, but this would allow forks to use the CI on other branches as well

Yeah, this is a nice feature, our contributors can self-check their patches on the fork's CI with this

@SteveLauC
Copy link
Member Author

Friendly ping @asomers, could I suggest giving this PR a review when you have time, so that we can handle other PRs without running out of Cirrus credits

@asomers
Copy link
Member

asomers commented Oct 21, 2023 via email

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@SteveLauC
Copy link
Member Author

SteveLauC commented Nov 5, 2023

Let me remove the migrated tasks in our Cirrus CI so that we can finally merge this PR


We have a macro skip_if_cirrus!() to skip a CI test under Cirrus:

nix/test/common/mod.rs

Lines 54 to 62 in b28132b

#[cfg(any(target_os = "linux", target_os = "android"))]
#[macro_export]
macro_rules! skip_if_cirrus {
($reason:expr) => {
if std::env::var_os("CIRRUS_CI").is_some() {
skip!("{}", $reason);
}
};
}

It is only defined under Android and Linux, given that we have Linux tests on both Cirrus (Linux/arm64) and GHA, we should probably provide the following macros as well, and document them in CONTRIBUTING.md:

We detect if we are running on GHA with the GITHUB_ACTIONS env variable, ref: Check if we're in a GitHub Action / Travis CI / Circle CI etc. testing environment

#[cfg(any(target_os = "linux", target_os = "android"))]
#[macro_export]
macro_rules! skip_if_gha {
    ($reason:expr) => {
        if std::env::var_os("GITHUB_ACTIONS").is_some() {
            skip!("{}", $reason);
        }
    };
}

#[cfg(any(target_os = "linux", target_os = "android"))]
#[macro_export]
macro_rules! skip_if_ci {
    ($reason:expr) => {
        if std::env::var_os("GITHUB_ACTIONS").is_some() || std::env::var_os("CIRRUS_CI").is_some() {
            skip!("{}", $reason);
        }
    };
}

@asomers
Copy link
Member

asomers commented Nov 5, 2023

@SteveLauC do you know why GHA isn't running? I don't see anything in the repository settings that we would have to enable.

@SteveLauC
Copy link
Member Author

@SteveLauC do you know why GHA isn't running? I don't see anything in the repository settings that we would have to enable.

I actually have the same question:

Well, no idea why GHA is not running with this PR, [skip ci] in the PR title should not disable it(but only the Cirrus CI) as I didn't find anything related to PR title in their documentation

One guess is that when I originally created this PR, I added [skip ci] to the PR title, which could probably disable the GHA as well?

@Jan561
Copy link
Contributor

Jan561 commented Nov 6, 2023

It should work out of the box when the workflow files appear on the main branch

@asomers
Copy link
Member

asomers commented Nov 6, 2023

@SteveLauC how about we revert the .cirrus.yml changes and merge this PR. Hopefully both CI engines will run then, and we can reapply the .cirrus.yml changes in a second PR.

@SteveLauC
Copy link
Member Author

@SteveLauC how about we revert the .cirrus.yml changes and merge this PR. Hopefully both CI engines will run then, and we can reapply the .cirrus.yml changes in a second PR.

Yeah, this makes sense, just discarded my last commit

@SteveLauC SteveLauC added this pull request to the merge queue Nov 8, 2023
Merged via the queue into nix-rust:master with commit 6868489 Nov 8, 2023
35 checks passed
@SteveLauC SteveLauC deleted the GHA branch November 8, 2023 01: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