ci: dont upload PRs to releases.drivechain.info #5
Conversation
octobocto
force-pushed
the
2024-10-07-use-commit-hash-for-artifacts
branch
2 times, most recently
from
6370977
to
34fb3b8
Compare
octobocto
changed the title
.github/workflows/build-release.yml
Outdated
| ESCAPED_BRANCH_NAME=$(echo "$BRANCH_NAME" | sed 's/[\/\\ ]/\\&/g') | ||
| if [ "$ESCAPED_BRANCH_NAME" != "master" ]; then | ||
| BITCOIN_PATCHED_VERSION="${ESCAPED_BRANCH_NAME}-${BITCOIN_PATCHED_VERSION}" | ||
| BRANCH_NAME=$(echo $GITHUB_REF | sed 's/refs\/heads\///' | sed 's/refs\/pull\/\([0-9]*\)\/merge/pr-\1/') |
There was a problem hiding this comment.
IIUC, this would potentially cause collisions, ie.
branch-name and branch/name.
Maybe append a hash of the original branch name, if the original branch name contains forbidden characters
https://github.com/actions/toolkit/blob/main/packages/artifact/docs/faq.md#supported-characters
There was a problem hiding this comment.
If we want to avoid collisions, we need to do something else, eg branchname-commithash-version.
It’s ok to have multiple branches with the exact same name, if the previous one was deleted from GitHub.
There was a problem hiding this comment.
Collisions on the same branch are intentional. The branch name is in there so that different branches of this repo generate artifacts with different names.
There was a problem hiding this comment.
Yes.
You can still have collisions though. If you create a branch today called signet, merge it, delete it, and then one month down the line create a branch called signet, you’re going to have a collision
There was a problem hiding this comment.
I've pushed two new commits. The first makes sure to only upload master pushes to https://releases.drivechain.info, e.g not on PRs. That's in line with every other repo that uploads to releases. As a side-effect, we can remove the branch-tagging entirely.
There was a problem hiding this comment.
We are expecting to have multiple branches in this repo that should create releases, not just master. This repo is intended to be used for all forks of Bitcoin that we maintain
There was a problem hiding this comment.
Are you 100% sure all PRs should be uploaded to releases.drivechain.info? Torkels OP_RETURN PR would be uploaded. Same for all future changes and bug-fixes. One unique upload for each PR. 10 PRs later, this page is going to become very clobbered:
If we want to have multiple parallell forks, it might be better to agree to some sort of prefix to the branch, and only upload if that exists. Not all PRs.
octobocto
force-pushed
the
2024-10-07-use-commit-hash-for-artifacts
branch
4 times, most recently
from
e17be51
to
508872a
Compare
octobocto
changed the title
octobocto
changed the title
octobocto
force-pushed
the
2024-10-07-use-commit-hash-for-artifacts
branch
3 times, most recently
from
fe29ecc
to
509e0a0
Compare
octobocto
force-pushed
the
2024-10-07-use-commit-hash-for-artifacts
branch
from
8f7ef14
to
ec79211
Compare
* ci: extract branch name without sed * ci: only upload PRs and whitelisted branches
Prior to this change, the version set was invalid, and CI failed with error messages like
The artifact name is not valid: bitcoin-patched-refs\/pull\/4\/merge-28.99.0-x86_64-unknown-linux-gnu. Contains the following character: Backslash \In this PR we make sure to only upload white-listed branches (none as of yet) and master pushes to releases.drivechain.info. Also remove sed-extraction of the branch-name entirely. Use github.head_ref instead.