Skip to content

Commit 64fa8c1

Browse files
committed
Target specific scripts in "Check Scripts" template
Background ---------- Previously, the approach taken in the "Check Scripts" template was to recursively search the entire repository for shell scripts. There were several problems with that system: The `-regextype` flag used in the `find` commands of the `shell:check` and `shell:check-mode` tasks is not supported by the BSD/macOS version of `find`, meaning the tasks would fail if a contributor using a macOS machine tried to run them: ``` find: -regextype: unknown primary or operator ``` The `shell:check` and `shell:check-mode` tasks only provided coverage for files with `.sh` and `.bash` file extensions, whereas the practice of omitting a file extension on shell scripts is unfortunately quite common. There was no obvious way to exclude paths of externally maintained files from coverage by the `shell:format` task. Alternative Solutions --------------------- The first of the problems listed above could be overcome by configuring the tasks to adjust the `find` commands to use different flags depending on the operating system of the machine. The second could be overcome by using `shfmt --files` (which searches recursively for shell scripts by checking for the presence of a shebang inside files and outputs a list of the paths) as a replacement for `find`. The third could be overcome by documenting the usage of shfmt's poorly advertised feature of ignoring the paths assigned an `ignore` attribute in the .editorconfig file. Chosen Solution --------------- After careful consideration, the decision was made to abandon the previous approach of attempting to automatically discover script files and instead instead use the strategy of configuring the template with the specific paths of the scripts to be checked for each project it is installed into. Although such an approach would not be appropriate in the case of a check on files that tend to be present in greater abundance and regularly added and moved in a project, this is not the case for shell scripts in Arduino projects. A project is more likely to contain a few scripts at most and their paths tend to be reasonably stable. Code Duplication in Workflow ---------------------------- In order to make it easier to interpret results, navigate logs, and reduce duration of workflow runs, a separate workflow job is used for each of the distinct checks. Unfortunately it is necessary for the project maintainer to configure the script paths redundantly in the matrix of each of the three jobs. Intuitively we would expect this could be avoided by defining the paths at workflow scope via the env key. However, this is not feasible due to the env context not being available for use in the jobs.<job name>.strategy.matrix key. It is technically possible to accomplish this by adding a job that converts the data from the env context into a job output (the `needs` context is available for use in the `matrix` key). However the significant increase in complexity this would bring to the workflow outweighs the benefit of avoiding duplication.
1 parent e9c50c4 commit 64fa8c1

File tree

5 files changed

+181
-101
lines changed

5 files changed

+181
-101
lines changed

.github/workflows/check-shell-task.yml

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ jobs:
5050
echo "result=$RESULT" >> $GITHUB_OUTPUT
5151
5252
lint:
53-
name: ${{ matrix.configuration.name }}
53+
name: ${{ matrix.configuration.name }} (${{ matrix.script }})
5454
needs: run-determination
5555
if: needs.run-determination.outputs.result == 'true'
5656
runs-on: ubuntu-latest
@@ -75,6 +75,8 @@ jobs:
7575
# ShellCheck's "tty" output format is most suitable for humans reading the log.
7676
format: tty
7777
continue-on-error: false
78+
script:
79+
- other/installation-script/install.sh
7880

7981
steps:
8082
- name: Set environment variables
@@ -118,15 +120,23 @@ jobs:
118120
continue-on-error: ${{ matrix.configuration.continue-on-error }}
119121
with:
120122
linters: gcc
121-
run: task --silent shell:check SHELLCHECK_FORMAT=${{ matrix.configuration.format }}
123+
run: task --silent shell:check SCRIPT_PATH="${{ matrix.script }}" SHELLCHECK_FORMAT=${{ matrix.configuration.format }}
122124

123125
formatting:
126+
name: formatting (${{ matrix.script }})
124127
needs: run-determination
125128
if: needs.run-determination.outputs.result == 'true'
126129
runs-on: ubuntu-latest
127130
permissions:
128131
contents: read
129132

133+
strategy:
134+
fail-fast: false
135+
136+
matrix:
137+
script:
138+
- other/installation-script/install.sh
139+
130140
steps:
131141
- name: Set environment variables
132142
run: |
@@ -162,18 +172,30 @@ jobs:
162172
echo "${{ env.SHFMT_INSTALL_PATH }}" >> "$GITHUB_PATH"
163173
164174
- name: Format shell scripts
165-
run: task --silent shell:format
175+
run: |
176+
task \
177+
--silent \
178+
shell:format \
179+
SCRIPT_PATH="${{ matrix.script }}"
166180
167181
- name: Check formatting
168182
run: git diff --color --exit-code
169183

170184
executable:
185+
name: executable (${{ matrix.script }})
171186
needs: run-determination
172187
if: needs.run-determination.outputs.result == 'true'
173188
runs-on: ubuntu-latest
174189
permissions:
175190
contents: read
176191

192+
strategy:
193+
fail-fast: false
194+
195+
matrix:
196+
script:
197+
- other/installation-script/install.sh
198+
177199
steps:
178200
- name: Checkout repository
179201
uses: actions/checkout@v4
@@ -185,4 +207,8 @@ jobs:
185207
version: 3.x
186208

187209
- name: Check for non-executable scripts
188-
run: task --silent shell:check-mode
210+
run: |
211+
task \
212+
--silent \
213+
shell:check-mode \
214+
SCRIPT_PATH="${{ matrix.script }}"

Taskfile.yml

Lines changed: 33 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ tasks:
3434
- task: python:lint
3535
- task: python:test
3636
- task: shell:check
37+
vars:
38+
SCRIPT_PATH: other/installation-script/install.sh
3739
- task: shell:check-mode
40+
vars:
41+
SCRIPT_PATH: other/installation-script/install.sh
3842
- task: yaml:lint
3943

4044
fix:
@@ -49,6 +53,8 @@ tasks:
4953
- task: markdown:fix
5054
- task: python:format
5155
- task: shell:format
56+
vars:
57+
SCRIPT_PATH: other/installation-script/install.sh
5258

5359
ci:sync:
5460
desc: Sync CI workflows from templates
@@ -64,7 +70,6 @@ tasks:
6470
"{{.WORKFLOW_TEMPLATES_PATH}}/check-npm-task.yml" \
6571
"{{.WORKFLOW_TEMPLATES_PATH}}/check-prettier-formatting-task.yml" \
6672
"{{.WORKFLOW_TEMPLATES_PATH}}/check-python-task.yml" \
67-
"{{.WORKFLOW_TEMPLATES_PATH}}/check-shell-task.yml" \
6873
"{{.WORKFLOW_TEMPLATES_PATH}}/check-taskfiles.yml" \
6974
"{{.WORKFLOW_TEMPLATES_PATH}}/check-yaml-task.yml" \
7075
"{{.WORKFLOW_TEMPLATES_PATH}}/sync-labels-npm.yml" \
@@ -832,81 +837,62 @@ tasks:
832837
cmds:
833838
- poetry run pytest workflow-templates/assets/deploy-mkdocs-versioned/siteversion/tests
834839

840+
# Parameter variables:
841+
# - SCRIPT_PATH: path of the script to be checked.
835842
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
836843
shell:check:
837844
desc: Check for problems with shell scripts
838845
cmds:
846+
- |
847+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
848+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
849+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
850+
exit 1
851+
fi
839852
- |
840853
if ! which shellcheck &>/dev/null; then
841854
echo "shellcheck not installed or not in PATH."
842855
echo "Please install: https://github.com/koalaman/shellcheck#installing"
843856
exit 1
844857
fi
845858
- |
846-
# There is something odd about shellcheck that causes the task to always exit on the first fail, despite any
847-
# measures that would prevent this with any other command. So it's necessary to call shellcheck only once with
848-
# the list of script paths as an argument. This could lead to exceeding the maximum command length on Windows if
849-
# the repository contained a large number of scripts, but it's unlikely to happen in reality.
850859
shellcheck \
851860
--format={{default "tty" .SHELLCHECK_FORMAT}} \
852-
$(
853-
# The odd method for escaping . in the regex is required for windows compatibility because mvdan.cc/sh gives
854-
# \ characters special treatment on Windows in an attempt to support them as path separators.
855-
find . \
856-
-type d -name '.git' -prune -or \
857-
-type d -name '.licenses' -prune -or \
858-
-type d -name '__pycache__' -prune -or \
859-
-type d -name 'node_modules' -prune -or \
860-
\( \
861-
-regextype posix-extended \
862-
-regex '.*[.](bash|sh)' -and \
863-
-type f \
864-
\) \
865-
-print
866-
)
861+
"{{.SCRIPT_PATH}}"
867862
863+
# Parameter variables:
864+
# - SCRIPT_PATH: path of the script to be checked.
868865
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
869866
shell:check-mode:
870867
desc: Check for non-executable shell scripts
871868
cmds:
872869
- |
873-
EXIT_STATUS=0
874-
while read -r nonExecutableScriptPath; do
875-
# The while loop always runs once, even if no file was found
876-
if [[ "$nonExecutableScriptPath" == "" ]]; then
877-
continue
878-
fi
879-
880-
echo "::error file=${nonExecutableScriptPath}::non-executable script file: $nonExecutableScriptPath";
881-
EXIT_STATUS=1
882-
done <<<"$(
883-
# The odd approach to escaping `.` in the regex is required for windows compatibility because mvdan.cc/sh
884-
# gives `\` characters special treatment on Windows in an attempt to support them as path separators.
885-
find . \
886-
-type d -name '.git' -prune -or \
887-
-type d -name '.licenses' -prune -or \
888-
-type d -name '__pycache__' -prune -or \
889-
-type d -name 'node_modules' -prune -or \
890-
\( \
891-
-regextype posix-extended \
892-
-regex '.*[.](bash|sh)' -and \
893-
-type f -and \
894-
-not -executable \
895-
-print \
896-
\)
897-
)"
898-
exit $EXIT_STATUS
870+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
871+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
872+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
873+
exit 1
874+
fi
875+
- |
876+
test -x "{{.SCRIPT_PATH}}"
899877
878+
# Parameter variables:
879+
# - SCRIPT_PATH: path of the script to be formatted.
900880
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
901881
shell:format:
902882
desc: Format shell script files
903883
cmds:
884+
- |
885+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
886+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
887+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
888+
exit 1
889+
fi
904890
- |
905891
if ! which shfmt &>/dev/null; then
906892
echo "shfmt not installed or not in PATH. Please install: https://github.com/mvdan/sh#shfmt"
907893
exit 1
908894
fi
909-
- shfmt -w .
895+
- shfmt -w "{{.SCRIPT_PATH}}"
910896

911897
# Make a temporary file named according to the passed TEMPLATE variable and print the path passed to stdout
912898
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/windows-task/Taskfile.yml

workflow-templates/assets/check-shell-task/Taskfile.yml

Lines changed: 27 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,78 +2,59 @@
22
version: "3"
33

44
tasks:
5+
# Parameter variables:
6+
# - SCRIPT_PATH: path of the script to be checked.
57
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
68
shell:check:
79
desc: Check for problems with shell scripts
810
cmds:
11+
- |
12+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
13+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
14+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
15+
exit 1
16+
fi
917
- |
1018
if ! which shellcheck &>/dev/null; then
1119
echo "shellcheck not installed or not in PATH."
1220
echo "Please install: https://github.com/koalaman/shellcheck#installing"
1321
exit 1
1422
fi
1523
- |
16-
# There is something odd about shellcheck that causes the task to always exit on the first fail, despite any
17-
# measures that would prevent this with any other command. So it's necessary to call shellcheck only once with
18-
# the list of script paths as an argument. This could lead to exceeding the maximum command length on Windows if
19-
# the repository contained a large number of scripts, but it's unlikely to happen in reality.
2024
shellcheck \
2125
--format={{default "tty" .SHELLCHECK_FORMAT}} \
22-
$(
23-
# The odd method for escaping . in the regex is required for windows compatibility because mvdan.cc/sh gives
24-
# \ characters special treatment on Windows in an attempt to support them as path separators.
25-
find . \
26-
-type d -name '.git' -prune -or \
27-
-type d -name '.licenses' -prune -or \
28-
-type d -name '__pycache__' -prune -or \
29-
-type d -name 'node_modules' -prune -or \
30-
\( \
31-
-regextype posix-extended \
32-
-regex '.*[.](bash|sh)' -and \
33-
-type f \
34-
\) \
35-
-print
36-
)
26+
"{{.SCRIPT_PATH}}"
3727
28+
# Parameter variables:
29+
# - SCRIPT_PATH: path of the script to be checked.
3830
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
3931
shell:check-mode:
4032
desc: Check for non-executable shell scripts
4133
cmds:
4234
- |
43-
EXIT_STATUS=0
44-
while read -r nonExecutableScriptPath; do
45-
# The while loop always runs once, even if no file was found
46-
if [[ "$nonExecutableScriptPath" == "" ]]; then
47-
continue
48-
fi
49-
50-
echo "::error file=${nonExecutableScriptPath}::non-executable script file: $nonExecutableScriptPath";
51-
EXIT_STATUS=1
52-
done <<<"$(
53-
# The odd approach to escaping `.` in the regex is required for windows compatibility because mvdan.cc/sh
54-
# gives `\` characters special treatment on Windows in an attempt to support them as path separators.
55-
find . \
56-
-type d -name '.git' -prune -or \
57-
-type d -name '.licenses' -prune -or \
58-
-type d -name '__pycache__' -prune -or \
59-
-type d -name 'node_modules' -prune -or \
60-
\( \
61-
-regextype posix-extended \
62-
-regex '.*[.](bash|sh)' -and \
63-
-type f -and \
64-
-not -executable \
65-
-print \
66-
\)
67-
)"
68-
exit $EXIT_STATUS
35+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
36+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
37+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
38+
exit 1
39+
fi
40+
- |
41+
test -x "{{.SCRIPT_PATH}}"
6942
43+
# Parameter variables:
44+
# - SCRIPT_PATH: path of the script to be formatted.
7045
# Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-shell-task/Taskfile.yml
7146
shell:format:
7247
desc: Format shell script files
7348
cmds:
49+
- |
50+
if [[ "{{.SCRIPT_PATH}}" == "" ]]; then
51+
echo "Path to script file must be passed to this task via the SCRIPT_PATH taskfile variable."
52+
echo "See: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-shell-task.md#usage"
53+
exit 1
54+
fi
7455
- |
7556
if ! which shfmt &>/dev/null; then
7657
echo "shfmt not installed or not in PATH. Please install: https://github.com/mvdan/sh#shfmt"
7758
exit 1
7859
fi
79-
- shfmt -w .
60+
- shfmt -w "{{.SCRIPT_PATH}}"

workflow-templates/check-shell-task.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,23 @@ Install the [`check-shell-task.yml`](check-shell-task.yml) GitHub Actions workfl
2323

2424
The formatting style defined in `.editorconfig` is the official standardized style to be used in all Arduino tooling projects and should not be modified.
2525

26+
### Configuration
27+
28+
Configure the paths of the shell scripts to be checked as elements in the [job matrices](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix) of `check-shell-task.yml` at:
29+
30+
- `jobs.lint.strategy.matrix.script[]`
31+
- `jobs.formatting.strategy.matrix.script[]`
32+
- `jobs.executable.strategy.matrix.script[]`
33+
34+
#### Example:
35+
36+
```yaml
37+
matrix:
38+
script:
39+
- path/to/some-script.sh
40+
- path/to/another-script.sh
41+
```
42+
2643
### Readme badge
2744
2845
Markdown badge:
@@ -64,3 +81,39 @@ On every push or pull request that modifies one of the shell scripts in the repo
6481
- Runs [`shfmt`](https://github.com/mvdan/sh) to check formatting.
6582
- Checks for forgotten executable script file permissions.
6683
```
84+
85+
## Usage
86+
87+
In addition to the automated checks provided by the GitHub Actions workflow, the tasks can be ran locally.
88+
89+
### Prerequisites
90+
91+
The following development tools must be available in your local environment:
92+
93+
- [**ShellCheck**](https://github.com/koalaman/shellcheck#installing) - shell script static analysis tool.
94+
- [**shfmt**](https://github.com/mvdan/sh#shfmt) - shell script formatting tool.
95+
- [**Task**](https://taskfile.dev/installation/) task runner tool.
96+
97+
### Run static analysis
98+
99+
```text
100+
task shell:check SCRIPT_PATH="<script path>"
101+
```
102+
103+
(where `<script path>` is the path to the script file)
104+
105+
### Check file permissions
106+
107+
```text
108+
task shell:check-mode SCRIPT_PATH="<script path>"
109+
```
110+
111+
(where `<script path>` is the path to the script file)
112+
113+
### Format script
114+
115+
```text
116+
task shell:format SCRIPT_PATH="<script path>"
117+
```
118+
119+
(where `<script path>` is the path to the script file)

0 commit comments

Comments
 (0)