Skip to content
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

Allow z3-solver<=4.13.0.0, streamline Dockerfile #1867

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

dbast
Copy link
Contributor

@dbast dbast commented Aug 7, 2024

This updates the z3-solver version constrain to allow the version 4.13.0.0, which now has proper manylinux2014_aarch64 wheels, see manylinux2014_aarch64.whl

This together with the fact that blake2b >=0.3.0 now has compilable source files see + cytoolz have weels see allow to streamline the Dockerfile, by creating all the wheels via one single command pip wheel -r /run/requirements.txt, without any special handling of z3-solver or blake2b or extra installing cython.

Also:

  • Add pre-commit hooks for linting Dockerfiles (hadolint) and shell scripts (shellcheck + shfmt).
  • Fixing the shellcheck + hadolint lint findings
  • Adding a container test workflow that builds the container without pushing it via Github Action. This only runs in PRs when certain files change (this is not that easy with circleci) and executes the container smoke tests before any PR merge.
Solved shellcheck findings
In all_tests.sh line 3:
echo -n "Checking Python version... "
   ^-- SC3037 (warning): In POSIX sh, echo flags are undefined.


In docker_build_and_deploy.sh line 12:
if [ ! -z $CIRCLE_TAG ]; then
   ^-- SC2236 (style): Use -n instead of ! -z.
        ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
if [ ! -z "$CIRCLE_TAG" ]; then


In docker_build_and_deploy.sh line 25:
echo "$DOCKERHUB_PASSWORD" | docker login -u $DOCKERHUB_USERNAME --password-stdin
                                           ^-----------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
echo "$DOCKERHUB_PASSWORD" | docker login -u "$DOCKERHUB_USERNAME" --password-stdin

For more information:
https://www.shellcheck.net/wiki/SC3037 -- In POSIX sh, echo flags are undef...
https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
https://www.shellcheck.net/wiki/SC2236 -- Use -n instead of ! -z.
Solved hadolint findings
Dockerfile:6:29 unexpected ':' expecting '@', '\', a new line followed by the next instruction, or the image tag

Dockerfile:16 DL4006 warning: Set the SHELL option -o pipefail before RUN with a pipe in it. If you are using /bin/sh in an alpine image or if your shell is symlinked to busybox then consider explicitly setting your SHELL to /bin/ash, or disable this check
Dockerfile:24 DL3013 warning: Pin versions in pip. Instead of `pip install <package>` use `pip install <package>==<version>` or `pip install --requirement <requirements file>`
Dockerfile:24 DL3042 warning: Avoid use of cache directory with pip. Use `pip install --no-cache-dir <package>`
Dockerfile:55 DL3042 warning: Avoid use of cache directory with pip. Use `pip install --no-cache-dir <package>`
Dockerfile:55 DL3013 warning: Pin versions in pip. Instead of `pip install <package>` use `pip install <package>==<version>` or `pip install --requirement <requirements file>`
Dockerfile:73 DL3003 warning: Use WORKDIR to switch to a directory
Dockerfile:83 DL3042 warning: Avoid use of cache directory with pip. Use `pip install --no-cache-dir <package>`

This all in all should make it much easier to do changes to the Dockerfile + requirements.txt file.

@dbast
Copy link
Contributor Author

dbast commented Aug 9, 2024

rebased

@dbast
Copy link
Contributor Author

dbast commented Aug 9, 2024

Happy to reduce the content in this PR, refraining from adding more pre-commit hooks or just commenting out things in the Dockerfile instead of fully removing the sophisticated and previously required things there.

Copy link
Collaborator

@norhh norhh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, this should be quite useful!

@norhh norhh merged commit 1384ef5 into Consensys:develop Aug 9, 2024
4 checks passed
@dbast dbast deleted the z3-solver branch August 10, 2024 06:11
@dbast dbast mentioned this pull request Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants