-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: always build guix, save artifacts #6197
ci: always build guix, save artifacts #6197
Conversation
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.
utACK 770651a
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.
LGTM 770651a
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.
post-utACK 770651a
but let's address nits in follow-up PR
needs: build-image | ||
# runs-on: [ "self-hosted", "linux", "x64", "ubuntu-core" ] | ||
runs-on: ubuntu-latest | ||
# if: ${{ contains(github.event.pull_request.labels.*.name, 'guix-build') }} |
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.
nit: dead code in comment?
# if: ${{ contains(github.event.pull_request.labels.*.name, 'guix-build') }} | ||
strategy: | ||
matrix: | ||
build_target: [x86_64-linux-gnu, arm-linux-gnueabihf, aarch64-linux-gnu, riscv64-linux-gnu, x86_64-w64-mingw32, x86_64-apple-darwin, arm64-apple-darwin] |
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.
nit: the list of targets are duplicated now between this yml file and contrib/guix/guix-build
:
# Default to building for all supported HOSTs (overridable by environment)
export HOSTS="${HOSTS:-x86_64-linux-gnu arm-linux-gnueabihf aarch64-linux-gnu riscv64-linux-gnu
x86_64-w64-mingw32
x86_64-apple-darwin arm64-apple-darwin}"
is any easy way to unify it and keep list somewhere such as contrib/guix/util/build_targets
?
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.
Not that I can think of... no... Open to ideas
${{ github.workspace }}/dash/depends/sources | ||
${{ github.workspace }}/dash/depends/work | ||
key: ${{ steps.guix-cache-restore.outputs.cache-primary-key }} | ||
${{ github.workspace }}/dash/guix-build*/output/${{ matrix.build_target }}/ |
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.
should there be quotes?
${{ github.workspace }}/dash/guix-build*/output/${{ matrix.build_target }}/ | |
"${{ github.workspace }}/dash/guix-build*/output/${{ matrix.build_target }}/" |
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.
umm, I don't think there are needed to be quotes
- name: Compute SHA256 checksums | ||
continue-on-error: true # It will complain on depending on only some hosts | ||
run: | | ||
HOSTS=${{ matrix.build_target }} ./dash/contrib/containers/guix/scripts/guix-check ${{ github.workspace }}/dash |
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.
should it be quotes there?
HOSTS=${{ matrix.build_target }} ./dash/contrib/containers/guix/scripts/guix-check ${{ github.workspace }}/dash | |
HOSTS=${{ matrix.build_target }} ./dash/contrib/containers/guix/scripts/guix-check "${{ github.workspace }}/dash" |
@@ -67,8 +105,8 @@ jobs: | |||
-v ${{ github.workspace }}/dash:/src/dash \ | |||
-v ${{ github.workspace }}/.cache:/home/ubuntu/.cache \ | |||
-w /src/dash \ | |||
guix_ubuntu:latest && \ | |||
docker exec guix-daemon bash -c '/usr/local/bin/guix-start' | |||
ghcr.io/${{ needs.build-image.outputs.repo-name }}/dashcore-guix-builder:${{ needs.build-image.outputs.image-tag }} && \ |
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.
should there be quotes?
ghcr.io/${{ needs.build-image.outputs.repo-name }}/dashcore-guix-builder:${{ needs.build-image.outputs.image-tag }} && \ | |
"ghcr.io/${{ needs.build-image.outputs.repo-name }}/dashcore-guix-builder:${{ needs.build-image.outputs.image-tag }}" && \ |
BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') | ||
REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') |
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 add quotes here:
BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') | |
REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') | |
BRANCH_NAME="$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]')" | |
REPO_NAME="$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]')" |
770651a set hosts in guix-check (pasta) 580bbe6 feat: improve guix building; run always, save artifacts (pasta) 101a315 refactor: simplify caching setup, add a restore key to actually cache besides 1 run (pasta) 1b139e4 feat: automatically run guix-build on all tags pushed (pasta) Pull request description: ## Issue being fixed or feature implemented Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts. In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour ## What was done? ## How Has This Been Tested? see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600 ## Breaking Changes None ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 770651a Tree-SHA512: 639b95c3b6a26f205ed00c138a9189f915cfc36a815516035e59ceda82675414b1bd31a361b33449b5e4c58a7655f3a7d616b362c23f7fa75e72b1284be06b9e
770651a set hosts in guix-check (pasta) 580bbe6 feat: improve guix building; run always, save artifacts (pasta) 101a315 refactor: simplify caching setup, add a restore key to actually cache besides 1 run (pasta) 1b139e4 feat: automatically run guix-build on all tags pushed (pasta) Pull request description: ## Issue being fixed or feature implemented Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts. In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour ## What was done? ## How Has This Been Tested? see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600 ## Breaking Changes None ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 770651a Tree-SHA512: 639b95c3b6a26f205ed00c138a9189f915cfc36a815516035e59ceda82675414b1bd31a361b33449b5e4c58a7655f3a7d616b362c23f7fa75e72b1284be06b9e
d627a6e chore: bump version to 21.1.1 (pasta) 5f9700c docs: release notes for v21.1.1 (pasta) 1c00726 Merge #6277: chore: add builder key for kittywhiskers (pasta) a2bc0f1 Merge #6290: chore: update pasta gpg key to reflect new subkeys (pasta) 167608c Merge #6338: ci: attest results of guix builds (pasta) 6fb4e49 Merge #6197: ci: always build guix, save artifacts (pasta) c0ca93c Merge #6340: fix: make 6336 compile in v21.1.x branch, using older CHECK_NONFATAL functionality (pasta) bb96df4 Merge #6336: fix: rpc getblock and getblockstats for blocks with withdrawal transactions (asset unlock) (pasta) 8e70262 Merge #6131: feat: make a support of Qt app to show Platform transfer Tx (pasta) 80ed279 Merge #6328: backport: bitcoin#30131, bitcoin#23258, bitcoin#30504 - fix bild for Ubuntu 24.10 + clang (pasta) bd772fb Merge #6229: fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (pasta) 9bf39a9 Merge #6222: fix: adjust payee predictions after mn_rr activation, add tests (pasta) 87bebfc Merge #6219: fix: correct is_snapshot_cs in VerifyDB (pasta) a4e6b8a Merge #6208: fix: persist coinjoin denoms options from gui over restarts (pasta) Pull request description: ## Issue being fixed or feature implemented See commits, backports, release notes, version bump ## What was done? ## How Has This Been Tested? ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: utACK d627a6e kwvg: ACK d627a6e UdjinM6: utACK d627a6e ogabrielides: utACK d627a6e Tree-SHA512: cde7e40760e16e9f48da8149c3742d18a34029b057405e4d55b87110da96acbcd19b47280451dd7b5ad1ccfc91fde655452cf5f0f0d1e01a41b4c685337c64b8
Issue being fixed or feature implemented
Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts.
In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour
What was done?
How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600
Breaking Changes
None
Checklist:
Go over all the following points, and put an
x
in all the boxes that apply.