Skip to content

Commit 8595539

Browse files
authored
Use Pre-Commit in CI (#159)
This PR unifies the linting and formatting with `pre-commit`. Before, we used a mixture of `pre-commit` and custom-installed tools. However, this could lead to version inconsistencies between different users and the CI. **I suggest to only look at 563138b. The other commit just reformatted all the files.** ## Changed - Switch CI to use pre-commit for linting - Update contributing guide - Adjust Make targets - Formatted all files ## Fixed - Fix missing dependency in pre-commit-config
1 parent d22d67d commit 8595539

17 files changed

Lines changed: 62 additions & 105 deletions

File tree

.github/workflows/ci-lint.yml

Lines changed: 20 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,55 +13,32 @@ name: CI • Lint & Licenses
1313
- "v*.*.*"
1414
pull_request:
1515
workflow_dispatch:
16-
inputs:
17-
docker_image_deeploy:
18-
description: "Deeploy Image to use"
19-
required: false
20-
default: "ghcr.io/pulp-platform/deeploy:devel"
21-
22-
concurrency:
23-
group: ${{ github.workflow }}-${{ github.ref }}
24-
cancel-in-progress: true
2516

2617
jobs:
27-
select-env:
28-
uses: ./.github/workflows/_select-env.yml
29-
with:
30-
docker_image_deeploy: ${{ inputs.docker_image_deeploy }}
31-
3218
linting:
33-
needs: select-env
34-
runs-on: ${{ needs.select-env.outputs.runner }}
35-
container:
36-
image: ${{ needs.select-env.outputs.image }}
19+
name: Run pre-commit
20+
runs-on: ubuntu-latest
3721
steps:
3822
- name: Checkout Repo
3923
uses: actions/checkout@v4
4024
with:
41-
submodules: recursive
42-
- name: Build Deeploy
43-
shell: bash
44-
run: |
45-
pip install .
46-
pip install -r requirements-dev.txt
47-
- name: Format Python
48-
shell: bash
49-
run: |
50-
yapf -rpd -e "*/TEST_*/" -e "*/third_party/" -e "install/" -e "toolchain/" .
51-
- name: Format Python Imports
52-
shell: bash
53-
run: |
54-
isort --quiet --sg "**/TEST_*/*" --sg "**/third_party/*" --sg "install/*" --sg "toolchain/*" ./ -c
55-
autoflake --quiet -c -r --remove-all-unused-imports --ignore-init-module-imports --exclude "**/third_party/*,**/install/*,**/toolchain/*" .
56-
- name: Format C
57-
shell: bash
58-
run: |
59-
python scripts/run_clang_format.py -e "*/TEST_*/*" -e "*/third_party/*" -e "*/install/*" -e "*/toolchain/*" -r --clang-format-executable=${LLVM_INSTALL_DIR}/bin/clang-format . scripts
60-
- name: Format YAML
61-
shell: bash
25+
fetch-depth: 0
26+
27+
- name: Set up Python
28+
uses: actions/setup-python@v4
29+
with:
30+
python-version: "3.11"
31+
32+
- name: Install dependencies
6233
run: |
63-
yamllint .
64-
- name: Check Licenses
65-
shell: bash
34+
python -m pip install --upgrade pip
35+
pip install pre-commit
36+
37+
- name: Run pre-commit (all files)
6638
run: |
67-
python scripts/reuse_skip_wrapper.py $(find . \( -name '*.py' -o -name '*.c' -o -name '*.h' -o -name '*.html' -o -name '*.rst' -o -name '*.yml' -o -name '*.yaml' \) -not -path "*toolchain*" -not -path "*third_party*" -not -path "*prebuilt*" -not -path "*.git/*" -not -path "*install/*" -type f)
39+
# Show diffs on failure and run across the entire repository to ensure consistency
40+
pre-commit run --show-diff-on-failure --all-files
41+
42+
- name: Show git status (debug)
43+
if: ${{ always() }}
44+
run: git status --porcelain || true

.github/workflows/package-publish.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#
33
# SPDX-License-Identifier: Apache-2.0
44

5+
---
56
name: Package • Publish to PyPi
67

78
on:

.pre-commit-config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ exclude: |
1313
| .*TEST_.*
1414
| .*TestFiles.*
1515
| .*runtime.*
16+
| .*prebuilt/.*
1617
)
1718
1819
repos:
@@ -28,6 +29,7 @@ repos:
2829
- id: reuse
2930
name: Check SPDX License Headers
3031
entry: scripts/reuse_skip_wrapper.py
32+
additional_dependencies: ["reuse==6.2.0"]
3133
language: python
3234
stages: [pre-commit, pre-merge-commit, pre-push, manual]
3335
types: [text]

.vscode/launch.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
},
6868
"labelTransform": {
6969
"text": "${fileDirname}",
70-
"find" : "${workspaceFolder}/DeeployTest/Tests/",
70+
"find": "${workspaceFolder}/DeeployTest/Tests/",
7171
"replace": ""
7272
},
7373
"valueTransform": {

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,21 @@ This file contains the changelog for the Deeploy project. The changelog is divid
55

66

77
### List of Pull Requests
8+
- Use Pre-Commit in CI [#159](https://github.com/pulp-platform/Deeploy/pull/159)
89
- Deeploy-GAP9 Platform [#143](https://github.com/pulp-platform/Deeploy/pull/143)
910
- Update CLI interface Across Project, Fix Tutorial, and Remove Legacy Test [#157](https://github.com/pulp-platform/Deeploy/pull/157)
1011

1112
### Added
1213
- Added GAP9 Platform Support: Deployer, Bindings, Templates, Tiler, DMA (L3Dma/MchanDma), target library, CI workflows
1314

1415
### Changed
16+
- Switch CI to use pre-commit for linting
1517
- Update `pulp-nnx` and `pulp-nn-mixed` submodules to their latest versions
1618
- PULP-NN moved to TargetLibraries third-party folder
1719
- Aligned CLI commands across the project
1820

1921
### Fixed
22+
- Fix missing dependency in pre-commit-config
2023
- Fix test paths in Deeploy 101 tutorial
2124

2225
### Removed

CONTRIBUTING.md

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,42 +36,36 @@ Additionally, add the title and link to the pull request in the list of pull req
3636
- Remove the link to the precompiled LLVM 12 in the `deeployRunner` for Snitch and in the CI.
3737
[...]
3838
```
39-
4039
## Style guide
4140

42-
Deeploy mainly consists of code implemented in C, Makefile, and Python. To facilitate efficient collaboration among users and contributors, it is important to maintain a consistent coding style. To achieve this, it is strongly recommend to use autoformatting tools with the provided configuration files. Additionally, the Continuous Integration (CI) system checks the adherence to the style guide for each pushed commit. Currently configuration for C using `clang-format` and for Python using `yapf` and `isort` are provided.
43-
44-
You can format all relevant files by running:
45-
```bash
46-
make format
47-
```
48-
49-
Alternatively, to only lint the files without modifying them, you can run:
50-
```bash
51-
make lint
52-
```
53-
54-
### Pre-commit
41+
Deeploy mainly consists of code implemented in C, Makefile, and Python. To facilitate efficient collaboration among users and contributors, it is important to maintain a consistent coding style. We use [pre-commit](https://pre-commit.com) with autoformatting tools to maintain this consistency. Configuration is provided for C using `clang-format` and for Python using `yapf` and `isort`.
5542

56-
Additionally, we provide the [pre-commit](https://pre-commit.com) configuration file which you can use to install github hooks that execute the formatting commands on your changes.
43+
### Setting up pre-commit
5744

58-
You will need to manually install pre-commit since it's not added as a dependency to the `pyproject.toml`:
45+
Install pre-commit (not included in `pyproject.toml`):
5946
```bash
6047
pip install pre-commit
6148
```
6249

63-
The configuration sets the default stage for all the hooks to `pre-push` so to install the git hooks run:
50+
Install the git hooks configured to run at the `pre-push` stage:
6451
```bash
6552
pre-commit install --hook-type pre-push
6653
```
67-
The hooks will run before each push, making sure the pushed code can pass linting checks and not fail the CI on linting.
6854

69-
If you change your mind and don't want the git hooks:
55+
The hooks will automatically format your code before each push, ensuring it passes linting checks and CI validation.
56+
57+
To uninstall the git hooks:
7058
```bash
7159
pre-commit uninstall
7260
```
7361

74-
_Note:_ This configures only the python formatting git hooks. The c formatting is not supported at the moment.
62+
You can also manually run formatting without pushing:
63+
```bash
64+
pre-commit run --all-files
65+
66+
# Or by running the Makefile target:
67+
make format
68+
```
7569

7670
## Licensing
7771

Container/Dockerfile.deeploy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ ENV SOFTHIER_INSTALL_DIR=/app/install/softhier
8181
ENV MINIMALLOC_INSTALL_DIR=/app/install/minimalloc
8282
ENV MEMPOOL_HOME=/app/install/mempool
8383
ENV BENDER_INSTALL_DIR=/app/install/bender
84-
ENV PATH=/app/install/qemu/bin:/app/install/banshee:/app/install/bender:$PATH
84+
ENV PATH=/app/install/qemu/bin:/app/install/banshee:/app/install/bender:$PATH
8585

8686
WORKDIR /app
8787

Deeploy/Targets/PULPOpen/Templates/FloatLayernormTemplate.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@
2424
int8_t ${nodeName}_log2Core = log2(NUM_CORES);
2525
2626
int32_t ${nodeName}_seq_length = ${size} / ${lastDimLength};
27-
int32_t ${nodeName}_chunk = (${nodeName}_seq_length >> ${nodeName}_log2Core) +
27+
int32_t ${nodeName}_chunk = (${nodeName}_seq_length >> ${nodeName}_log2Core) +
2828
((${nodeName}_seq_length & (NUM_CORES-1)) != 0);
2929
int32_t ${nodeName}_start = MIN(${nodeName}_chunk * ${nodeName}_core_id, ${nodeName}_seq_length);
3030
int32_t ${nodeName}_end = MIN(${nodeName}_start + ${nodeName}_chunk, ${nodeName}_seq_length);
31-
31+
3232
int32_t ${nodeName}_elem_start = ${nodeName}_start * ${lastDimLength};
3333
int32_t ${nodeName}_elem_end = ${nodeName}_end * ${lastDimLength};
3434
int32_t ${nodeName}_elem_count = ${nodeName}_elem_end - ${nodeName}_elem_start;
35-
35+
3636
const float${grad_in_type.referencedType.typeWidth}_t* ${nodeName}_grad_in_ptr = ${grad_in} + ${nodeName}_elem_start;
3737
const float${data_in_type.referencedType.typeWidth}_t* ${nodeName}_data_in_ptr = ${data_in} + ${nodeName}_elem_start;
3838
float${grad_out_type.referencedType.typeWidth}_t* ${nodeName}_grad_out_ptr = ${grad_out} + ${nodeName}_elem_start;
39-
39+
4040
if (${nodeName}_elem_count > 0) {
4141
LayernormGrad_fp${grad_in_type.referencedType.typeWidth}_fp${grad_out_type.referencedType.typeWidth}(
4242
${nodeName}_grad_in_ptr, // Upstream gradient (dy)

Deeploy/Targets/PULPOpen/Templates/SGDTemplate.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
temp_mul[3] = learning_rate * ref_${grad}[i+3];
3232
temp_mul[4] = learning_rate * ref_${grad}[i+4];
3333
temp_mul[5] = learning_rate * ref_${grad}[i+5];
34-
34+
3535
// Unrolled subtraction operations
3636
ref_${weight_updated}[i] = ref_${weight}[i] - temp_mul[0];
3737
ref_${weight_updated}[i+1] = ref_${weight}[i+1] - temp_mul[1];

DeeployTest/testUtils/core/execution.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
def generate_network(config: DeeployTestConfig, skip: bool = False) -> None:
1818
"""
1919
Generate network code from ONNX model.
20-
20+
2121
Raises:
2222
RuntimeError: If network generation fails
2323
"""
@@ -151,7 +151,7 @@ def build_binary(config: DeeployTestConfig) -> None:
151151
def run_simulation(config: DeeployTestConfig, skip: bool = False) -> TestResult:
152152
"""
153153
Run simulation and parse output.
154-
154+
155155
Raises:
156156
RuntimeError: If simulation cannot be executed
157157
"""

0 commit comments

Comments
 (0)