-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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] Automate preparation of release binaries for publishing (via artifacts) #12181
Conversation
ad1dabc
to
18da769
Compare
249d3ea
to
20c330a
Compare
20c330a
to
359b448
Compare
359b448
to
e29e196
Compare
This passed CI. Here's a successful run of Now I'm removing the temporary commits - the job will only run on tags from now on. |
acba133
to
46e6dbb
Compare
22b897c
to
e3d79ea
Compare
46e6dbb
to
d040d35
Compare
e1d56d7
to
a006b21
Compare
ab3b17f
to
9dbc57f
Compare
a006b21
to
253f600
Compare
9dbc57f
to
f8853c9
Compare
253f600
to
4bb26e7
Compare
246f161
to
c76a8a7
Compare
4bb26e7
to
d3259e9
Compare
This is now reviewable too. |
d3259e9
to
85de766
Compare
command: | | ||
mkdir github/ | ||
cd github/ | ||
../scripts/solc-bin/download-circleci-binaries.sh "$CIRCLE_WORKFLOW_ID" |
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.
Wouldn't it be easier, cleaner and safer to rename the binaries in the respective builds already and attach them to a common workspace similar to what we do for the bytecode run?
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 necessarily easier but might be doable. The biggest issue is that I need to get all binaries in the workspace but under different paths. You get an error if a job depends on multiple jobs adding a workspace file under the same path. Doing it via artifacts does not have this problem.
When I started this PR I actually did not understand our workspace layout all that well. It's often mounted at different locations in similar jobs and not in a consistent way. It was really confusing to me and I tried to clean that up in #12450. Unfortunately this resulted in a conflict between binaries on Linux and macOS now that the paths are consistent.
But after working on it I think that the current gnarly layout would work without a conflict. So yeah, I'm going to try to redo it like that.
Another option I was thinking of was rewriting the Bash script here in Python. In #12818 I already have a Python module with helpers for CircleCI and Github API so this would replace the big script in this PR with something much smaller. But if the workspace solution works, maybe it's better.
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.
Yeah, I know the problem with path conflicts in merging workspaces... but I thought since we have the same problem with the bytecode comparison runs, it may just work or at least not be too hard to achieve, but not entirely sure...
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.
Yeah, you must be right. t_bytecode_compare
does attach the workspace so it should work. It's just that I did not realize that at first and thought that trying to do it via workspace would be very messy and require renaming binaries. I only see now that it shouldn't :)
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 should really clean up the workspace layout though. Once we do this, we will have a conflict. My current idea to sidestep that is to name each binary after the platform and use symlinks in #12450.
Waiting for answer from @cameel |
Answered. Basically, I need to decide which way to go with this and update it one way (workspace) or another (Python-based script). |
85de766
to
36ba987
Compare
9af52f8
to
e32daef
Compare
Closing in favor of #12929. |
Closes #9933.
Depends on #12173 (draft until it's merged).Merged.It's not complete automation of the publishing process as stated in #9933 but it at least takes care of the most tedious and error prone part of the process so I think it's good enough.
The PR adds:
c_release_binaries
job that uses the script to gather artifacts from jobs that build release binaries, puts them in two directories (one for github release, one for solc-bin) and names files in each directory according to our naming conventions. Then attaches the results as its own artifacts.A small fix for missingMerged separately.*workflow_trigger_on_tags
in some of the existing jobs.The new job is supposed to run only on tagged commits. Note that I can only really test it on a PR. On a tagged commit some things will be different (CircleCI env vars, version string, etc.). We'll have to test that in practice once the PR is merged.