[workflows] Add workflow for building revive in a debian container. #55
Conversation
|
|
||
| - name: build-container | ||
| run: | | ||
| (cd utils && ./${{ env.DEBIAN_CONTAINER_BUILDER}} . ) |
There was a problem hiding this comment.
why did you decide to use your own docker. With this ci-unified
you have rust already installed and you can control the versions using docker tags.
The only issue which I see is that they are using debian 11 and remix backend is based on debian 12.
There was a problem hiding this comment.
It's a good point, the debian version in the builder should probably match the version where the binary is run. By keeping the images being used in sync we achieve it for free.
Note that if we settle on ci-unifed the supported rust version is 1.77 which is quite old. It'll probably able to build revive but I didn't try that.
There was a problem hiding this comment.
What is the issue with installing rust in a Debian container?
There was a problem hiding this comment.
I don't see any issue; I just prefer to avoid maintaining additional tools/images if I have something ready to use.
| run: | | ||
| (cd utils && ./${{ env.DEBIAN_CONTAINER_BUILDER}} --build-arg RUST_VERSION=${{ env.RUST_VERSION}} . ) | ||
|
|
||
| - name: build-revive-debian |
There was a problem hiding this comment.
I think it would be good to print the versions of the tools being used. Please take a look here
| DEBIAN_CONTAINER_RUNNER: run-debian-builder.sh | ||
| REVIVE_DEBIAN_INSTALL: ${{ github.workspace }}/target/release | ||
| REVIVE_DEBIAN_BINARY: resolc | ||
| RUST_VERSION: stable |
There was a problem hiding this comment.
The problem here is stable will change over time, but we want to have manual control over it. We can stick to 1.80 (slightly older version should do fine too).
| libmpfr-dev libgmp-dev libmpc-dev ncurses-dev \ | ||
| git curl | ||
| EOF | ||
| ARG RUST_VERSION=stable |
There was a problem hiding this comment.
Same here, I think sticking to specific version should be better.
What is missing is a rust version in the top level Cargo.toml:
rust-version = "1.80.0"
Could you add this here and to the top level Cargo.toml?
There was a problem hiding this comment.
The value in the Dockerfile is a default value, the value used is set in in the workflow file.
Changed the Cargo.toml file to specify the rust-version is 1.80
utils/build-revive.sh
Outdated
| REVIVE_INSTALL_DIR=$OPTARG | ||
| ;; | ||
| \?) echo "Error: Invalid option" | ||
| exit;; |
There was a problem hiding this comment.
Should probably return non-zero value here?
| exit;; | |
| exit 1;; |
| done | ||
| echo "Installing to ${REVIVE_INSTALL_DIR}" | ||
|
|
||
| $(pwd)/build-llvm.sh |
There was a problem hiding this comment.
To get rid of doing this every time we release, we can have another workflow (could even be a separate repository).
The idea is that this separate workflow only builds and releases LLVM, and here we can just download the LLVM release. The assumption is that we do not change the LLVM version often, should be way less often than we release revive.
However it can be done later, it shouldn't block us for now.
| jobs: | ||
| build-revive-debian-x86: | ||
| name: debian-container-x86 | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
If the run takes more than half an hour, maybe we should use beefier runners. I remember we had ubuntu-latest-8-cores and ubuntu-latest-16-cores runners.
There was a problem hiding this comment.
Yes we should use the more beefy ones. But building resolc itself takes seconds to minutes even on potato machines. I guess the main problem is LLVM and this is solved by having a dedicated release workflow for LLVM. So I think we should try both: Beefy runners + more economic release workflows.
Makefile: Add target 'install-revive' to build revive with the installation path specified by variable REVIVE_INSTALL_DIR. Add utils directory with scripts for building revive in a container. Add utils/build-revive.sh taking option argument '-o <install-dir>' to build revive with the specified install directory. Add utils/revive-builder-debian.dockerfile to make a docker container for building revive in a Debian environment.
ghost
merged commit 
Makefile: Add target 'install-revive' to build revive with the installation path specified by variable REVIVE_INSTALL_DIR.
Add utils directory with scripts for building revive in a container.
Add utils/build-revive.sh taking option argument '-o ' to build revive with the specified install directory.
Add utils/revive-builder-debian.dockerfile to make a docker container for building revive in a Debian environment.