Remove more tools from Bento configuration #1150
Conversation
| flake8: | ||
| ignore: | ||
| - bare-except-bugbear | ||
| run: true | ||
| r2c.registry.latest: |
There was a problem hiding this comment.
This is running https://r2c.dev/default-r2c-checks, which should be covered by our semgrep-run-r2c-config job.
| ignore: | ||
| - use-timeout | ||
| run: true | ||
| semgrep: |
There was a problem hiding this comment.
Covered by semgrep-run-r2c-config.
There was a problem hiding this comment.
This does let semgrep run automatically as a pre-commit hook. Should we add semgrep to .pre-commit.yaml first?
There was a problem hiding this comment.
Required a bit more work, but I updated the pre-commit configuration as well.
| ignore: | ||
| - use-timeout | ||
| run: true | ||
| semgrep: |
There was a problem hiding this comment.
Required a bit more work, but I updated the pre-commit configuration as well.
| rev: 'v0.12.0' | ||
| hooks: | ||
| - id: semgrep | ||
| args: ['--config', 'https://semgrep.live/p/python', '--include', '*.py', '--precommit', '--error'] |
There was a problem hiding this comment.
Trying this with something like /p/r2c was really slow and produced lots of parse errors. Using /p/python and --include *.py sped things up.
| @@ -1,52 +1,39 @@ | |||
| ## semgrep build | |||
|
|
|||
| FROM python:3.7.7-alpine3.11 as build-semgrep | |||
There was a problem hiding this comment.
A lot of this is no longer necessary now that we're not burning the commit hash into the package build/install. I.e. the .git directory is no longer necessary, and we can greatly simplify the install process (a simple pip install should do the trick).
| USER opam | ||
|
|
||
| WORKDIR /home/opam/opam-repository | ||
| RUN git pull && opam update && opam switch create 4.10.0+musl+static+flambda | ||
|
|
||
| COPY --chown=opam . /home/opam/semgrep/ |
There was a problem hiding this comment.
COPYing everything in significantly slows down the build process because the Docker context is huge. I.e. if anything changes in the working directory we have to rebuild semgrep-core from scratch, which takes a long time. Now we only rebuild if anything in .git, pfff, semgrep-core, or .gitmodules changes, which helpfully avoids the rebuild if semgrep/ changes.
| COPY --from=build-semgrep-core /home/opam/semgrep/semgrep-core/_build/default/bin/Main.exe /bin/semgrep-core | ||
| RUN semgrep-core --help | ||
| COPY semgrep /semgrep | ||
| RUN HOMEBREW_SYSTEM='NOCORE' python -m pip install /semgrep |
There was a problem hiding this comment.
HOMEBREW_SYSTEM='NOCORE' is now a misnomer referencing an env variable used in setup.py, but we can change that in a separate PR.
| ENTRYPOINT [ "/root/.local/bin/semgrep" ] | ||
| ENV PYTHONUNBUFFERED=1 | ||
| ENTRYPOINT ["semgrep"] | ||
| CMD ["--help"] |
There was a problem hiding this comment.
If no args are passed to docker run print the --help.
| @@ -27,6 +27,7 @@ | |||
| IN_DOCKER = "SEMGREP_IN_DOCKER" in os.environ | |||
| IN_GH_ACTION = "GITHUB_WORKSPACE" in os.environ | |||
| REPO_HOME_DOCKER = "/home/repo/" | |||
| REPO_HOME_DOCKER_PRECOMMIT = "/src/" | |||
There was a problem hiding this comment.
Here was the big blocker that was breaking pre-commit - it mounts the code at /src/. We can re-purpose the --precommit flag to handle this case.
| @@ -305,6 +306,10 @@ def close(self) -> None: | |||
|
|
|||
| if self.final_error: | |||
| raise self.final_error | |||
| elif self.rule_matches and self.settings.error_on_findings: | |||
There was a problem hiding this comment.
The --error flag wasn't doing anything prior to this. It's functionality got lost somewhere along the line in a refactor.
mschwager
force-pushed
the
mschwager-more-bento-removal
branch
from
7ddf9b9
to
e3b0604
Compare
| ENV PYTHONIOENCODING=utf8 | ||
| ENTRYPOINT [ "/root/.local/bin/semgrep" ] |
There was a problem hiding this comment.
This was another issue with running semgrep via pre-commit. pre-commit runs Docker containers with the UID and GID of the host user. This was breaking this entrypoint because an unprivileged user was trying to run something from /root/.... Now that we simply pip install semgrep we can access the script as an unprivileged user 👍
| WORKDIR /home/pythonbuild/semgrep | ||
| # downgrade to 20.1 since 20.1.1 reverts building in place | ||
| # https://github.com/pypa/pip/issues/7555 | ||
| RUN pip install pip==20.1 |
There was a problem hiding this comment.
Need to keep this pinned https://github.com/docker-library/python/blob/7dc395a6b9fabce8685009dc385fa98f8a944d5c/3.7/alpine3.11/Dockerfile#L148 base image changed the pip version it uses
There was a problem hiding this comment.
No longer needed cause of comment above re: no longer needing .git directory
There was a problem hiding this comment.
We shouldn't need to pin the version anymore since we're no longer using that functionality to burn in SCM information.
Comments inline.