-
Notifications
You must be signed in to change notification settings - Fork 136
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: migrate ci setup from circle ci to github actions #464
Changes from 18 commits
94c119f
53ba34b
1b268ec
2c31e5b
98715d9
ce16d25
dedd25a
2129b06
b8c6e28
c167a6b
5df5d6f
540c302
f1ad9f6
d3aad6d
752f3c8
353fc4b
2c7c643
90e23dd
d0ed676
0e69123
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
name: Configure Environment Variables | ||
description: Configure environment variables for Filecoin FFI | ||
|
||
runs: | ||
using: 'composite' | ||
steps: | ||
- if: runner.os == 'Linux' && runner.arch == 'ARM64' | ||
run: | | ||
wget -q https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2204/sbsa/cuda-ubuntu2204.pin | ||
sudo mv cuda-ubuntu2204.pin /etc/apt/preferences.d/cuda-repository-pin-600 | ||
wget -q https://developer.download.nvidia.com/compute/cuda/12.5.1/local_installers/cuda-repo-ubuntu2204-12-5-local_12.5.1-555.42.06-1_arm64.deb | ||
sudo dpkg -i cuda-repo-ubuntu2204-12-5-local_12.5.1-555.42.06-1_arm64.deb | ||
sudo cp /var/cuda-repo-ubuntu2204-12-5-local/cuda-*-keyring.gpg /usr/share/keyrings/ | ||
sudo apt-get update | ||
sudo apt-get -y install cuda-toolkit-12-5 | ||
sudo mkdir -p /usr/lib/aarch64-linux-gnu/stubs | ||
sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so.1 | ||
sudo ln -s /usr/local/cuda-12.5/lib64/stubs/libcuda.so /usr/lib/aarch64-linux-gnu/stubs/libcuda.so | ||
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if that's really needed. It looks a bit like a direct port from the CircleCI config. I'd expect the files to be accessible through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tried the suggestion in #472, but it doesn't seem to cut it, unfortunately. |
||
shell: bash | ||
- run: | | ||
echo "FIL_PROOFS_PARAMETER_CACHE=${GITHUB_WORKSPACE}/filecoin-proof-parameters/" >> $GITHUB_ENV | ||
echo 'GO111MODULE=on' >> $GITHUB_ENV | ||
echo 'RUST_LOG=info' >> $GITHUB_ENV | ||
echo "GOPATH=${HOME}/go" >> $GITHUB_ENV | ||
echo "CARGO_TERM_COLOR=never" >> $GITHUB_ENV | ||
shell: bash | ||
- run: | | ||
echo "/usr/local/go/bin" >> $GITHUB_PATH | ||
echo "${GOPATH}/bin" >> $GITHUB_PATH | ||
echo "${HOME}/.cargo/bin" >> $GITHUB_PATH | ||
echo "${HOME}/.bin" >> $GITHUB_PATH | ||
shell: bash | ||
- if: runner.os == 'Linux' && runner.arch == 'ARM64' | ||
run: | | ||
echo "LD_LIBRARY_PATH=/usr/lib/aarch64-linux-gnu/stubs:${LD_LIBRARY_PATH}" >> $GITHUB_ENV | ||
echo "LIBRARY_PATH=/usr/lib/aarch64-linux-gnu/stubs:${LIBRARY_PATH}" >> $GITHUB_ENV | ||
shell: bash | ||
- if: runner.os == 'macOS' | ||
run: | | ||
echo "CPATH=$(brew --prefix)/include" >> $GITHUB_ENV | ||
echo "LIBRARY_PATH=$(brew --prefix)/lib" >> $GITHUB_ENV | ||
shell: bash | ||
- if: runner.os == 'Linux' | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install --no-install-recommends -y valgrind ocl-icd-opencl-dev libssl-dev libhwloc-dev nvidia-cuda-toolkit g++-10 pkgconf | ||
# Downgrade to GCC 10, as CUDA 11 doesn't play nice with GCC 11 | ||
sudo update-alternatives --install /usr/bin/c++ c++ /usr/bin/g++-10 10 | ||
sudo update-alternatives --set c++ /usr/bin/g++-10 | ||
# Check if we need to install cuda-toolkit-12-5 | ||
shell: bash | ||
- if: runner.os == 'macOS' | ||
run: | | ||
HOMEBREW_NO_AUTO_UPDATE=1 brew install pkg-config md5sha1sum jq hwloc | ||
shell: bash | ||
- uses: dtolnay/rust-toolchain@21dc36fb71dd22e3317045c0c31a3f4249868b17 | ||
with: | ||
toolchain: 1.73 | ||
vmx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- uses: actions/setup-go@v5 | ||
with: | ||
go-version: '1.21' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,221 @@ | ||
name: CI | ||
|
||
on: | ||
pull_request: | ||
push: | ||
branches: | ||
- master | ||
tags: | ||
- v* | ||
workflow_dispatch: | ||
inputs: | ||
save: | ||
description: 'Save Filecoin parameters' | ||
required: false | ||
default: 'false' | ||
publish: | ||
description: 'Publish the static library' | ||
required: false | ||
default: 'false' | ||
run-leak-detector: | ||
description: 'Run the CGO leak detector' | ||
required: false | ||
default: 'false' | ||
ref: | ||
description: 'The ref to build' | ||
required: false | ||
|
||
defaults: | ||
run: | ||
shell: bash | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.ref }} | ||
cancel-in-progress: ${{ github.event_name == 'pull_request' }} | ||
|
||
permissions: | ||
contents: read | ||
|
||
# Can we apply these to the entire workflow? | ||
env: | ||
# Build the kernel only for the single architecture. This should reduce | ||
# the overall compile-time significantly. | ||
EC_GPU_CUDA_NVCC_ARGS: --fatbin --gpu-architecture=sm_75 --generate-code=arch=compute_75,code=sm_75 | ||
BELLMAN_CUDA_NVCC_ARGS: --fatbin --gpu-architecture=sm_75 --generate-code=arch=compute_75,code=sm_75 | ||
NEPTUNE_CUDA_NVCC_ARGS: --fatbin --gpu-architecture=sm_75 --generate-code=arch=compute_75,code=sm_75 | ||
DEBIAN_FRONTEND: noninteractive | ||
|
||
jobs: | ||
check: | ||
name: Check code style and run linters | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
- uses: ./.github/actions/configure-environment | ||
- if: github.event.inputs.ref != '' | ||
uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
ref: ${{ github.event.inputs.ref }} | ||
- name: Run shellcheck | ||
run: shellcheck ./install-filcrypto | ||
- name: Run cargo fmt | ||
run: | | ||
rustup component add rustfmt | ||
cargo fmt --manifest-path ./rust/Cargo.toml --all -- --check | ||
- name: Run cargo clippy | ||
run: cd rust && cargo clippy --all-targets --features blst-portable,opencl -- -D warnings | ||
- name: Run go fmt | ||
# `! go fmt ./... 2>&1 | read"` doesn't work, this one does, thanks | ||
# https://carsonip.me/posts/go-fmt-and-ci/ | ||
run: | | ||
output=$(go fmt ./...) | ||
echo "${output}" | ||
test -z "${output}" | ||
- name: Run various linters | ||
run: | | ||
go install github.com/golangci/golangci-lint/cmd/[email protected] | ||
galargh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
make go-lint | ||
cgo-bindings: | ||
name: Build and test CGO bindings (${{ matrix.runner }}) | ||
runs-on: ${{ matrix.runner }} | ||
strategy: | ||
matrix: | ||
runner: ['ubuntu-latest', ["self-hosted", "linux", "arm64", "xlarge"], 'macos-latest'] | ||
fail-fast: false | ||
# # Is it OK if we operate in the default working directory? | ||
# defaults: | ||
# run: | ||
# working-directory: go/src/github.com/filecoin-project/filecoin-ffi | ||
steps: | ||
- run: echo "Running on $RUNNER_OS $RUNNER_ARCH" | ||
- uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
- uses: ./.github/actions/configure-environment | ||
- if: github.event.inputs.ref != '' | ||
uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
ref: ${{ github.event.inputs.ref }} | ||
- if: runner.os == 'macOS' | ||
run: cd rust && cargo fetch | ||
- name: Build project | ||
run: make | ||
- name: Build project without CGO | ||
run: env CGO_ENABLED=0 go build . | ||
- if: runner.os == 'Linux' | ||
uses: actions/cache/restore@v3 | ||
with: | ||
key: v28-proof-params-${{ runner.os }}-${{ runner.arch }} | ||
path: | | ||
./filecoin-proof-parameters | ||
- if: runner.os == 'Linux' | ||
name: Obtain Filecoin parameters | ||
run: | | ||
DIR=$(pwd) | ||
cd $(mktemp -d) | ||
go install github.com/filecoin-project/go-paramfetch/paramfetch@latest | ||
$GOPATH/bin/paramfetch 2048 "${DIR}/parameters.json" "${DIR}/srs-inner-product.json" | ||
- if: runner.os == 'Linux' && ((github.event == 'push' && !startsWith(github.ref, 'refs/tags/')) || github.event.inputs.save == 'true') | ||
uses: actions/cache/save@v3 | ||
with: | ||
key: v28-proof-params-${{ runner.os }}-${{ runner.arch }} | ||
path: | | ||
./filecoin-proof-parameters | ||
- if: runner.os == 'Linux' | ||
run: cd rust && rustup target add wasm32-unknown-unknown | ||
- if: github.event.inputs.run-leak-detector == 'true' | ||
run: make cgo-leakdetect | ||
- if: runner.os == 'Linux' | ||
run: cd rust && FIL_PROOFS_PARAMETER_CACHE="${GITHUB_WORKSPACE}/filecoin-proof-parameters/" RUST_LOG=info cargo test --all --release -- --test-threads 1 && cd .. | ||
- if: runner.os == 'Linux' | ||
run: GOEXPERIMENT=cgocheck2 RUST_LOG=info go test -p 1 -timeout 60m | ||
- if: runner.os == 'macOS' | ||
name: Build project and tests, but don't actually run the tests (used to verify that build/link works with Darwin) | ||
run: GOEXPERIMENT=cgocheck2 RUST_LOG=info go test -run=^$ | ||
supraseal: | ||
name: Build with CUDA supraseal | ||
runs-on: ubuntu-latest | ||
# # Is it OK if we operate in the default working directory? | ||
# defaults: | ||
# run: | ||
# working-directory: go/src/github.com/filecoin-project/filecoin-ffi | ||
galargh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
- uses: ./.github/actions/configure-environment | ||
- if: github.event.inputs.ref != '' | ||
uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
ref: ${{ github.event.inputs.ref }} | ||
- name: Build project with `FFI_USE_CUDA_SUPRASEAL=1` | ||
run: FFI_BUILD_FROM_SOURCE=1 FFI_USE_CUDA_SUPRASEAL=1 make | ||
publish: | ||
needs: [check, cgo-bindings, supraseal] | ||
if: (github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')) || github.event.inputs.publish == 'true' | ||
rvagg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: Publish the static library (${{ matrix.runner }}) | ||
runs-on: ${{ matrix.runner }} | ||
strategy: | ||
matrix: | ||
runner: ['ubuntu-latest', ['self-hosted', 'linux', 'arm64', 'xlarge'], 'macos-latest'] | ||
fail-fast: false | ||
steps: | ||
- run: echo "Running on $RUNNER_OS $RUNNER_ARCH" | ||
- uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
- uses: ./.github/actions/configure-environment | ||
- if: github.event.inputs.ref != '' | ||
uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
ref: ${{ github.event.inputs.ref }} | ||
- if: runner.os == 'macOS' | ||
run: | | ||
cd rust && rustup target add x86_64-apple-darwin | ||
cd rust && cargo fetch | ||
- if: runner.os == 'Linux' | ||
name: Build and publish the standard release | ||
run: | | ||
cd rust | ||
|
||
REPOSITORY_NAME=${GITHUB_REPOSITORY##*/} | ||
|
||
TARBALL_PATH="/tmp/${REPOSITORY_NAME}-$(uname)-$(uname -m)-standard.tar.gz" | ||
RELEASE_NAME="${REPOSITORY_NAME}-$(uname)-$(uname -m)-standard" | ||
|
||
# Note: the blst dependency uses the portable configuration for maximum compatibility | ||
./scripts/build-release.sh build --verbose --no-default-features --features multicore-sdr,opencl,blst-portable | ||
./scripts/package-release.sh $TARBALL_PATH | ||
./scripts/publish-release.sh $TARBALL_PATH $RELEASE_NAME | ||
- if: runner.os == 'Linux' | ||
name: Build the optimized release | ||
run: | | ||
cd rust | ||
|
||
REPOSITORY_NAME=${GITHUB_REPOSITORY##*/} | ||
|
||
TARBALL_PATH="/tmp/${CIRCLE_PROJECT_REPONAME}-$(uname)-$(uname -m)-optimized.tar.gz" | ||
RUSTFLAGS="-C target-feature=$(cat rustc-target-features-optimized.json | jq -r '.[].rustc_target_feature' | tr '\n' ',')" | ||
|
||
./scripts/build-release.sh build --verbose --no-default-features --features multicore-sdr,opencl | ||
./scripts/package-release.sh $TARBALL_PATH | ||
- if: runner.os == 'macOS' | ||
name: Build and publish the universal standard release | ||
run: | | ||
cd rust | ||
|
||
REPOSITORY_NAME=${GITHUB_REPOSITORY##*/} | ||
|
||
RELEASE_NAME="${CIRCLE_PROJECT_REPONAME}-$(uname)-standard" | ||
TARBALL_PATH="/tmp/${RELEASE_NAME}.tar.gz" | ||
|
||
# Note: the blst dependency uses the portable configuration for maximum compatibility | ||
./scripts/build-release.sh lipo --verbose --no-default-features --features multicore-sdr,opencl,blst-portable | ||
./scripts/package-release.sh $TARBALL_PATH | ||
./scripts/publish-release.sh $TARBALL_PATH $RELEASE_NAME |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,12 +121,10 @@ func BenchmarkBLSVerifyBatch(b *testing.B) { | |
func benchmarkBLSVerifyBatchSize(size int) func(b *testing.B) { | ||
return func(b *testing.B) { | ||
var digests []Digest | ||
var msgs []Message | ||
var sigs []Signature | ||
var pubks []PublicKey | ||
for i := 0; i < size; i++ { | ||
msg := Message(fmt.Sprintf("cats cats cats cats %d %d %d dogs", i, i, i)) | ||
msgs = append(msgs, msg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're touching a test, is a typo or expected? I guess that breaks some linting rules? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the linting rules started failing. The issue made sense to me so I fixed it. I suspect we're using newer version of some linting tool or something like that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://hub.docker.com/layers/cimg/go/1.21.2-node/images/sha256-59b3fff1c4745e9195ea35eef2cfbee4cb0b562616869e304b581644020ad4b8 - golangci-lint v1.54.2 vs v1.56.2 but, I don't see any version of golangci-lint failing on this file. staticcheck does but I don't see it being used here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw I'm not objecting to this fix, just interested in why this is showing up for this diff There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the failing run before I added this change - https://github.com/filecoin-project/filecoin-ffi/actions/runs/9805936223/job/27076653921 It does come from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is listed in the linters list there, too. Honestly, I'm not sure why it wasn't flagging this issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Needs investigation, golangci-lint is invoking staticcheck, I wonder if we need it installed for that to happen or if there's some config we're missing. Maybe we should just import the lotus config and be done with it. I'll try and get to figuring this out unless someone has a chance before me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is running in GHA - that's why I had to add the fix - so I wonder how much time we want to spend investigating this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apparently golangci-lint has "staticcheck" but not quite the same as "staticcheck", so this is even more confusing .. it's a good fix, so not a big deal, it would just be god to understand this and maybe fix our linting to do a better job not a blocker for this PR |
||
digests = append(digests, Hash(msg)) | ||
priv := PrivateKeyGenerate() | ||
sig := PrivateKeySign(priv, msg) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all new for GHA, is the arm64 image available to us different enough that there's no cuda available already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using an almost clean Ubuntu image for Linux arm64 self-hosted runners. We can add installing a version of cuda-toolkit in the machine image to the self-hosted runners backlog if you think it'd be good to have a default available on a machine. One question I have is how much we care about the version of the toolkit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to hear @vmx on this; it's his area, cuda and nvidia driver versions for me are just a source of frustration rather than something I know a whole lot about!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For development I recommend installing CUDA drivers from the NVIDIA website as the distro ones often don't work well (for whatever reason). Though for CI, I think using the distro packages could work (KISS). If it works for Linux, it should also work for ARM (I hope).
We should support old enough versions and for filecoin-ffi it's more of a sanity check. So whatever the CUDA version is, as long as the tests pass, we are good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it; I'd move on with installing it on linux arm during the run first, and once we add A version of it to the base image, we'll come back here and remove that step.