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

Add a linter runnable locally and in the CI to ease and enforce trailing blanks trimming #15115

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
8 changes: 8 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: trailing-whitespace
exclude: '\.patch$|^src/.*\.conf$|^files/.*\.j2$'
#TODO fix the few files in ./files and ./src folders whose clean-up makes build fail
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,13 @@ For example:
* Use a pull request to do code review
* Use issues to keep track of what is going on

## Linters

Some basic linters are performed via tox and pre-commit in the CI.
The same tools can be used locally to clean up code before pushing it to the community.
More details in the [tox and pre-commit guide](./tox_precommit.md)


This project has adopted the [Microsoft Open Source Code of Conduct](https://opensource.microsoft.com/codeofconduct/).
For more information see the [Code of Conduct FAQ](https://opensource.microsoft.com/codeofconduct/faq/)
or contact [[email protected]](mailto:[email protected])
Expand Down
12 changes: 12 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ variables:
value: y

stages:
- stage: Linters
pool: sonicbld
jobs:
- job: tox
steps:
- checkout: self
clean: true
displayName: 'Checkout code'
- script: |
./scripts/tox_entry_point.sh
displayName: 'Install and run tox'

- stage: BuildVS
pool: sonicbld
jobs:
Expand Down
28 changes: 28 additions & 0 deletions scripts/tox_entry_point.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/bin/sh

# Copyright © 2023 Orange
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

cd $(dirname $0)
cd ..
export DEBIAN_FRONTEND=noninteractive
sudo -E apt-get -qq -y update 2>&1 >/dev/null || \
apt-get -qq -y update 2>&1 >/dev/null
sudo -E apt-get -qq -y install git gzip python3-pip tox >&1 >/dev/null || \
apt-get -qq -y install git gzip python3-pip tox >&1 >/dev/null
# Run tests inside a python tox virtual environment
# the PBR_VERSION variable is normally not needed and is here to avoid troubles in some CI
# that raise an exception for versioning w/o sdsit tarball or access to an upstream git repo
export PBR_VERSION=5.10.0
tox
7 changes: 7 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[metadata]
name = sonic_linters
home_page = https://github.com/sonic-net/sonic-buildimage

[files]
packages = sonic_linters

9 changes: 9 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env python

import setuptools

# PBR doc available at https://docs.openstack.org/pbr/latest/

setuptools.setup(
setup_requires=['pbr'],
pbr=True)
47 changes: 47 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[tox]
minversion = 3.7.0
envlist =
pre-commit-ci
skipsdist = true

[testenv]
passenv = http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY
usedevelop = true
basepython = python3
deps =
setuptools>=7.0

[testenv:pre-commit-install]
basepython = python3
deps = pre-commit
commands =
pre-commit install
pre-commit install --hook-type commit-msg

[testenv:pre-commit-uninstall]
basepython = python3
deps = pre-commit
commands =
pre-commit uninstall
pre-commit uninstall --hook-type commit-msg

[testenv:pre-commit-autoupdate]
basepython = python3
deps = pre-commit
commands =
pre-commit autoupdate

[testenv:pre-commit]
basepython = python3
deps = pre-commit
passenv = HOME
commands =
pre-commit run --all-files

[testenv:pre-commit-ci]
basepython = python3
deps = pre-commit
passenv = HOME
commands =
pre-commit run --all-files --show-diff-on-failure --from-ref a73d443c1d02698d9f3e030947c469677798bd32 --to-ref HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

What is it "a73d443c1d02698d9f3e030947c469677798bd32"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is it "a73d443c1d02698d9f3e030947c469677798bd32"?

It is the hash that references the commit just below this PR.
Only file modified since this commit will be linted by pre-commit.

$ git log -4 --oneline
53568f4 (HEAD -> trailing_blanks, orangeopensource/trailing_blanks) [CI][doc] Add some documentation for tox and pre-commit
1b1341d [CI] Trigger tox/pre-commit linter(s) in Azure CI
1aebc16 [CI] Add a pre-commit trailing blanks linter via tox
a73d443 [CI][doc][build] Trim src folder files trailing blanks (#15162)


109 changes: 109 additions & 0 deletions tox_precommit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# Tox and pre-commit guide

## What is tox?

<span class="title-ref">Tox</span> is a tool written in Python to ease
tests automation and dependencies management. It provides a command line
tool that run tests inside a [Python virtual
environment](https://docs.python.org/3/glossary.html#term-virtual-environment).

This means that it will not modify your local system settings or package
and executable files. Instead, it uses a hidden folder
(<span class="title-ref">.tox</span>) to install the required Python
dependencies via pip ([the package installer for
Python](https://pip.pypa.io/)) before running tests.

You can find more details about tox at <https://github.com/tox-dev/tox>
. <span class="title-ref">Tox</span> is often used as a front-end to
Continuous Integration servers.

<span class="title-ref">Tox</span> configuration and behavior is very
portable across GNU+Linux distributions and others UNIX-like systems or
environments.

Once tox installed in a local environment with for example the following
command on Debian based systems:

$ sudo apt-get install tox

or on Red Hat based systems:

$ sudo yum install python-tox

the same test suite than the CI can be run locally by simply calling the
tox command from the project git clone local folder:

$ tox

## Tox configuration

<span class="title-ref">Tox</span> configuration is written in the
<span class="title-ref">tox.ini</span> file at the root folder of the
Git project. Please read [tox official
documentation](https://tox.readthedocs.io/) for more details. For tox
users, the most important parameter in the
<span class="title-ref">\[tox\]</span> section is
<span class="title-ref">envlist</span>. It specifies which profiles to
run by default (i.e. when tox is called without the option
<span class="title-ref">-e</span>). The option
<span class="title-ref">-e</span> overrides this parameter and allows to
choose which profiles to run. For example:

$ tox -e pre-commit-ci

will only run the <span class="title-ref">precommit-ci</span> profile. And:

$ tox -e pre-commit-ci,pre-commit

will run the <span class="title-ref">pre-commit-ci</span> and
<span class="title-ref">pre-commit</span> profiles.

## Pre-commit profiles

[Pre-commit](https://pre-commit.com/) is a wrapper for various linters
that relies on [Git Hooks](https://git-scm.com/docs/githooks). It is
particularly useful to address common programming issues such as triming
trailing whitespaces or removing tabs.

<span class="title-ref">Pre-commit</span> configuration can be found in
the <span class="title-ref">.pre-commit-config.yaml</span> file at the
Git project root. <span class="title-ref">Pre-commit</span> hooks, like
any other Git hooks, are run automatically when the command
<span class="title-ref">'git commit'</span> is called. This avoids
forgetting running linters. Although,
<span class="title-ref">pre-commit</span> can also be called directly
from its shell command. This is what is currently performed in SONiC
Azure CI.

The <span class="title-ref">pre-commit</span> profile allows to call
<span class="title-ref">pre-commit</span> inside tox virtualenv without
installing the <span class="title-ref">pre-commit</span> package in the
local system, what is pretty convenient:

$ tox -e pre-commit

This is also true to install/uninstall the corresponding Git hooks in
your Git folder thanks to the profiles
<span class="title-ref">pre-commit-install</span> and
\`pre-commit-uninstall\`:

$ tox -e pre-commit-install

and:

$ tox -e pre-commit-uninstall

To avoid linting all files in the repo in the CI, a dedicated profile
<span class="title-ref">pre-commit-ci</span> has been created to
lint only files modified or created after pre-commit introduction.

$ tox -e pre-commit-ci

As specified in the envlist, this is the only profile run by default
when launching tox without any option.

Linters versions in the <span class="title-ref">.pre-commit-config.yaml</span>
file can be automatically updated to their latest version by running the
<span class="title-ref">pre-commit-autoupdate</span> profile:

$ tox -e pre-commit-autoupdate