Skip to content

test: add an easy way to run linters locally #26906

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

Merged
merged 2 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions ci/lint/04_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,25 @@ export LC_ALL=C
${CI_RETRY_EXE} apt-get update
${CI_RETRY_EXE} apt-get install -y curl git gawk jq xz-utils

PYTHON_PATH=/tmp/python
if [ ! -d "${PYTHON_PATH}/bin" ]; then
(
git clone https://github.com/pyenv/pyenv.git
cd pyenv/plugins/python-build || exit 1
./install.sh
)
# For dependencies see https://github.com/pyenv/pyenv/wiki#suggested-build-environment
${CI_RETRY_EXE} apt-get install -y build-essential libssl-dev zlib1g-dev \
libbz2-dev libreadline-dev libsqlite3-dev curl llvm \
libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev \
clang
env CC=clang python-build "$(cat "${BASE_ROOT_DIR}/.python-version")" "${PYTHON_PATH}"
if [ -z "${SKIP_PYTHON_INSTALL}" ]; then
PYTHON_PATH=/tmp/python
if [ ! -d "${PYTHON_PATH}/bin" ]; then
(
git clone https://github.com/pyenv/pyenv.git
cd pyenv/plugins/python-build || exit 1
./install.sh
)
# For dependencies see https://github.com/pyenv/pyenv/wiki#suggested-build-environment
${CI_RETRY_EXE} apt-get install -y build-essential libssl-dev zlib1g-dev \
libbz2-dev libreadline-dev libsqlite3-dev curl llvm \
libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev \
clang
env CC=clang python-build "$(cat "${BASE_ROOT_DIR}/.python-version")" "${PYTHON_PATH}"
fi
export PATH="${PYTHON_PATH}/bin:${PATH}"
command -v python3
python3 --version
fi
export PATH="${PYTHON_PATH}/bin:${PATH}"
command -v python3
python3 --version

${CI_RETRY_EXE} pip3 install codespell==2.2.1
${CI_RETRY_EXE} pip3 install flake8==5.0.4
Expand All @@ -34,5 +36,6 @@ ${CI_RETRY_EXE} pip3 install pyzmq==24.0.1
${CI_RETRY_EXE} pip3 install vulture==2.6

SHELLCHECK_VERSION=v0.8.0
curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/
export PATH="/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}"
curl -sL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | \
tar --xz -xf - --directory /tmp/
mv "/tmp/shellcheck-${SHELLCHECK_VERSION}/shellcheck" /usr/bin/
6 changes: 5 additions & 1 deletion ci/lint/06_script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@

export LC_ALL=C

if [ -n "$CIRRUS_PR" ]; then
if [ -n "$LOCAL_BRANCH" ]; then
# To faithfully recreate CI linting locally, specify all commits on the current
# branch.
COMMIT_RANGE="$(git merge-base HEAD master)..HEAD"
elif [ -n "$CIRRUS_PR" ]; then
COMMIT_RANGE="HEAD~..HEAD"
echo
git log --no-merges --oneline "$COMMIT_RANGE"
Expand Down
29 changes: 29 additions & 0 deletions ci/lint/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# See test/lint/README.md for usage.
#
# This container basically has to live in this directory in order to pull in the CI
# install scripts. If it lived in the root directory, it would have to pull in the
# entire repo as docker context during build; if it lived elsewhere, it wouldn't be
Comment on lines +4 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

If it lived in the root directory, it would have to pull in the entire repo as docker context during build;

Would you have preferred it lived in root? I think it makes more sense in /ci/lint, but if it would be better you could use .dockerignore to prevent adding everything to your docker context, as in this diff:

git diff
diff --git a/.dockerignore b/.dockerignore
new file mode 100644
index 000000000..142df6af9
--- /dev/null
+++ b/.dockerignore
@@ -0,0 +1,2 @@
+*
+!/ci/lint
\ No newline at end of file
diff --git a/ci/lint/Dockerfile b/Dockerfile
similarity index 91%
rename from ci/lint/Dockerfile
rename to Dockerfile
index 03c20c728..87c9bb85a 100644
--- a/ci/lint/Dockerfile
+++ b/Dockerfile
@@ -16,8 +16,8 @@ ENV LC_ALL=C.UTF-8
 ENV SKIP_PYTHON_INSTALL=1
 
 # Must be built from ./ci/lint/ for these paths to work.
-COPY ./docker-entrypoint.sh /entrypoint.sh
-COPY ./04_install.sh /install.sh
+COPY ./ci/lint/docker-entrypoint.sh /entrypoint.sh
+COPY ./ci/lint/04_install.sh /install.sh
 
 RUN /install.sh && \
   echo 'alias lint="./ci/lint/06_script.sh"' >> ~/.bashrc && \

Copy link
Member

Choose a reason for hiding this comment

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

For reference, you can avoid passing the full context by using buildkit or podman. See

# able to make back-references to pull in the install scripts. So here it lives.

FROM python:3.7-buster

ENV DEBIAN_FRONTEND=noninteractive
ENV LC_ALL=C.UTF-8

# This is used by the 04_install.sh script; we can't read the Python version from
# .python-version for the same reasons as above, and it's more efficient to pull a
# preexisting Python image than it is to build from source.
ENV SKIP_PYTHON_INSTALL=1

# Must be built from ./ci/lint/ for these paths to work.
COPY ./docker-entrypoint.sh /entrypoint.sh
COPY ./04_install.sh /install.sh

RUN /install.sh && \
echo 'alias lint="./ci/lint/06_script.sh"' >> ~/.bashrc && \
chmod 755 /entrypoint.sh && \
rm -rf /var/lib/apt/lists/*


WORKDIR /bitcoin
ENTRYPOINT ["/entrypoint.sh"]
12 changes: 12 additions & 0 deletions ci/lint/docker-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bash
export LC_ALL=C

# Fixes permission issues when there is a container UID/GID mismatch with the owner
# of the mounted bitcoin src dir.
git config --global --add safe.directory /bitcoin

if [ -z "$1" ]; then
LOCAL_BRANCH=1 bash -ic "./ci/lint/06_script.sh"
else
exec "$@"
fi
18 changes: 18 additions & 0 deletions test/lint/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
This folder contains lint scripts.

Running locally
===============

To run linters locally with the same versions as the CI environment, use the included
Dockerfile:

```sh
cd ./ci/lint
docker build -t bitcoin-linter .

cd /root/of/bitcoin/repo
Copy link
Contributor

Choose a reason for hiding this comment

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

since we're already assuming starting in the root 2 lines up, I'd just replace this with

Suggested change
cd /root/of/bitcoin/repo
cd ../..

docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
```

After building the container once, you can simply run the last command any time you
want to lint.


check-doc.py
============
Check for missing documentation of command line options.
Expand Down