From 223492645ff757059297b2da62df94c5e0e10e31 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:29:09 -0700 Subject: [PATCH 1/7] Always set workflow permissions at job level There are multiple scopes at which the permissions of the GITHUB_TOKEN access token (which is automatically generated for use in GitHub Actions workflow runs) can be configured: - enterprise - organization - repository - workflow - job The latter two scopes are configured using the `permissions` workflow key. The point of configuring permissions in the workflow is that each workflow may have different requirements. Granular configuration means that the "principle of least privilege" can be more closely followed, by only granting permissions in the specific scopes where they are needed. Previously, in cases where the same permissions configuration could be used for all jobs in a workflow, the configuration was done at the workflow scope. Even if functionally equivalent, I think it is semantically more appropriate to always set the permissions at the job scope. This more clearly communicates that the intention is to make the most granular possible permissions configuration. Hopefully that will serve as a model for any additional jobs added to the workflow in the future and make it more likely that the appropriate permissions configuration will be done there. --- .github/workflows/check-action-metadata-task.yml | 3 +++ .github/workflows/check-files-task.yml | 5 +++++ .github/workflows/check-general-formatting-task.yml | 3 +++ .github/workflows/check-license.yml | 4 ++++ .github/workflows/check-markdown-task.yml | 5 +++++ .github/workflows/check-npm-task.yml | 10 +++++++--- .github/workflows/check-prettier-formatting-task.yml | 3 +++ .github/workflows/check-python-task.yml | 5 +++++ .github/workflows/check-taskfiles.yml | 3 +++ .github/workflows/check-toc-task.yml | 3 +++ .github/workflows/check-workflows-task.yml | 2 ++ .github/workflows/check-yaml-task.yml | 3 +++ .github/workflows/spell-check-task.yml | 3 +++ .github/workflows/sync-labels-npm.yml | 6 ++++++ .github/workflows/test-python-poetry-task.yml | 3 +++ 15 files changed, 58 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check-action-metadata-task.yml b/.github/workflows/check-action-metadata-task.yml index 73c03f65..14b9f110 100644 --- a/.github/workflows/check-action-metadata-task.yml +++ b/.github/workflows/check-action-metadata-task.yml @@ -31,6 +31,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -56,6 +57,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-files-task.yml b/.github/workflows/check-files-task.yml index ddc5a7e3..f0aa84f1 100644 --- a/.github/workflows/check-files-task.yml +++ b/.github/workflows/check-files-task.yml @@ -15,6 +15,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -40,6 +41,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository @@ -58,6 +61,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-general-formatting-task.yml b/.github/workflows/check-general-formatting-task.yml index 1c3a94f5..c5e7c829 100644 --- a/.github/workflows/check-general-formatting-task.yml +++ b/.github/workflows/check-general-formatting-task.yml @@ -15,6 +15,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -40,6 +41,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Set environment variables diff --git a/.github/workflows/check-license.yml b/.github/workflows/check-license.yml index e83e2110..9ad4a3f3 100644 --- a/.github/workflows/check-license.yml +++ b/.github/workflows/check-license.yml @@ -35,6 +35,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -60,6 +61,9 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read + steps: - name: Checkout repository diff --git a/.github/workflows/check-markdown-task.yml b/.github/workflows/check-markdown-task.yml index beb934cd..b9a8f3dd 100644 --- a/.github/workflows/check-markdown-task.yml +++ b/.github/workflows/check-markdown-task.yml @@ -41,6 +41,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -66,6 +67,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository @@ -92,6 +95,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-npm-task.yml b/.github/workflows/check-npm-task.yml index 8bec6a8e..6d34f256 100644 --- a/.github/workflows/check-npm-task.yml +++ b/.github/workflows/check-npm-task.yml @@ -26,12 +26,10 @@ on: workflow_dispatch: repository_dispatch: -permissions: - contents: read - jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -57,6 +55,9 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read + steps: - name: Checkout repository @@ -80,6 +81,9 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read + steps: - name: Checkout repository diff --git a/.github/workflows/check-prettier-formatting-task.yml b/.github/workflows/check-prettier-formatting-task.yml index db19e30c..630d7417 100644 --- a/.github/workflows/check-prettier-formatting-task.yml +++ b/.github/workflows/check-prettier-formatting-task.yml @@ -209,6 +209,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -234,6 +235,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-python-task.yml b/.github/workflows/check-python-task.yml index f836e82d..8eaaaef7 100644 --- a/.github/workflows/check-python-task.yml +++ b/.github/workflows/check-python-task.yml @@ -35,6 +35,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -60,6 +61,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository @@ -92,6 +95,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-taskfiles.yml b/.github/workflows/check-taskfiles.yml index 9c3a60ef..494166b5 100644 --- a/.github/workflows/check-taskfiles.yml +++ b/.github/workflows/check-taskfiles.yml @@ -29,6 +29,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -55,6 +56,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read strategy: fail-fast: false diff --git a/.github/workflows/check-toc-task.yml b/.github/workflows/check-toc-task.yml index b3e2506f..84797d64 100644 --- a/.github/workflows/check-toc-task.yml +++ b/.github/workflows/check-toc-task.yml @@ -29,6 +29,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -55,6 +56,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read strategy: fail-fast: false diff --git a/.github/workflows/check-workflows-task.yml b/.github/workflows/check-workflows-task.yml index fefdaeca..f79a0c9d 100644 --- a/.github/workflows/check-workflows-task.yml +++ b/.github/workflows/check-workflows-task.yml @@ -28,6 +28,8 @@ on: jobs: validate: runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/check-yaml-task.yml b/.github/workflows/check-yaml-task.yml index 80bbf21f..6cad3416 100644 --- a/.github/workflows/check-yaml-task.yml +++ b/.github/workflows/check-yaml-task.yml @@ -47,6 +47,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -73,6 +74,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read strategy: fail-fast: false diff --git a/.github/workflows/spell-check-task.yml b/.github/workflows/spell-check-task.yml index 2a36af40..09c0c60b 100644 --- a/.github/workflows/spell-check-task.yml +++ b/.github/workflows/spell-check-task.yml @@ -15,6 +15,7 @@ on: jobs: run-determination: runs-on: ubuntu-latest + permissions: {} outputs: result: ${{ steps.determination.outputs.result }} steps: @@ -40,6 +41,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository diff --git a/.github/workflows/sync-labels-npm.yml b/.github/workflows/sync-labels-npm.yml index 31199d53..832de1ba 100644 --- a/.github/workflows/sync-labels-npm.yml +++ b/.github/workflows/sync-labels-npm.yml @@ -30,6 +30,8 @@ on: jobs: check: runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository @@ -65,6 +67,7 @@ jobs: download: needs: check runs-on: ubuntu-latest + permissions: {} strategy: matrix: @@ -92,6 +95,9 @@ jobs: sync: needs: download runs-on: ubuntu-latest + permissions: + contents: read + issues: write steps: - name: Set environment variables diff --git a/.github/workflows/test-python-poetry-task.yml b/.github/workflows/test-python-poetry-task.yml index 8a480c84..754a0815 100644 --- a/.github/workflows/test-python-poetry-task.yml +++ b/.github/workflows/test-python-poetry-task.yml @@ -41,6 +41,7 @@ jobs: runs-on: ubuntu-latest outputs: result: ${{ steps.determination.outputs.result }} + permissions: {} steps: - name: Determine if the rest of the workflow should run id: determination @@ -64,6 +65,8 @@ jobs: needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest + permissions: + contents: read steps: - name: Checkout repository From f0c9ef61cd08dc1216777a79fa41e26cc533b105 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:31:58 -0700 Subject: [PATCH 2/7] Add support to "Check License" for checking license files in multiple paths In cases where a project contains distinct components in subfolders, multiple license files might be present. Previously the "Check License" workflow only supported checking the license file in the root of the repository. Support for validating an arbitrary number of license files with arbitrary locations, types, and filenames is added. A job matrix is used to provide this support in a manner that makes application-specific configuration of the workflow easy and without code duplication. --- .github/workflows/check-license.yml | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/.github/workflows/check-license.yml b/.github/workflows/check-license.yml index 9ad4a3f3..8fb832a0 100644 --- a/.github/workflows/check-license.yml +++ b/.github/workflows/check-license.yml @@ -1,11 +1,6 @@ # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/check-license.md name: Check License -env: - EXPECTED_LICENSE_FILENAME: LICENSE.txt - # SPDX identifier: https://spdx.org/licenses/ - EXPECTED_LICENSE_TYPE: GPL-3.0 - # See: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows on: create: @@ -58,12 +53,22 @@ jobs: echo "result=$RESULT" >> $GITHUB_OUTPUT check-license: + name: ${{ matrix.check-license.path }} needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest permissions: contents: read + strategy: + fail-fast: false + + matrix: + check-license: + - path: ./ + expected-filename: LICENSE.txt + # SPDX identifier: https://spdx.org/licenses/ + expected-type: GPL-3.0 steps: - name: Checkout repository @@ -77,23 +82,27 @@ jobs: - name: Install licensee run: gem install licensee - - name: Check license file + - name: Check license file for ${{ matrix.check-license.path }} run: | EXIT_STATUS=0 + + # Go into folder path + cd ./${{ matrix.check-license.path }} + # See: https://github.com/licensee/licensee LICENSEE_OUTPUT="$(licensee detect --json --confidence=100)" DETECTED_LICENSE_FILE="$(echo "$LICENSEE_OUTPUT" | jq .matched_files[0].filename | tr --delete '\r')" echo "Detected license file: $DETECTED_LICENSE_FILE" - if [ "$DETECTED_LICENSE_FILE" != "\"${EXPECTED_LICENSE_FILENAME}\"" ]; then - echo "::error file=${DETECTED_LICENSE_FILE}::detected license file $DETECTED_LICENSE_FILE doesn't match expected: $EXPECTED_LICENSE_FILENAME" + if [ "$DETECTED_LICENSE_FILE" != "\"${{ matrix.check-license.expected-filename }}\"" ]; then + echo "::error file=${DETECTED_LICENSE_FILE}::detected license file $DETECTED_LICENSE_FILE doesn't match expected: ${{ matrix.check-license.expected-filename }}" EXIT_STATUS=1 fi DETECTED_LICENSE_TYPE="$(echo "$LICENSEE_OUTPUT" | jq .matched_files[0].matched_license | tr --delete '\r')" echo "Detected license type: $DETECTED_LICENSE_TYPE" - if [ "$DETECTED_LICENSE_TYPE" != "\"${EXPECTED_LICENSE_TYPE}\"" ]; then - echo "::error file=${DETECTED_LICENSE_FILE}::detected license type $DETECTED_LICENSE_TYPE doesn't match expected \"${EXPECTED_LICENSE_TYPE}\"" + if [ "$DETECTED_LICENSE_TYPE" != "\"${{ matrix.check-license.expected-type }}\"" ]; then + echo "::error file=${DETECTED_LICENSE_FILE}::detected license type $DETECTED_LICENSE_TYPE doesn't match expected \"${{ matrix.check-license.expected-type }}\"" EXIT_STATUS=1 fi From ec3d12e0b5b4e97d9d57814db8176c6506949191 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:35:39 -0700 Subject: [PATCH 3/7] Only validate `package.json` files in specified paths The project infrastructure validates the package.json npm configuration files against their JSON schema. Previously, in order to provide validation coverage for all package.json files in any locations in the repository, a "globstar" was used to cause the validator to recursively search the entire file tree under the repository. That approach is problematic because the repository contains externally maintained files (e.g., the npm packages under the node_modules folder). Searching and validating these files is inefficient at best and the cause of spurious failures at worst. This is avoided by targeting the search. Support for a repository maintainer to configure any number of specific locations of npm-managed projects in the "Check npm" workflow has been added, so this system is used to target the validations. When the `npm:validate` task is ran by a contributor on their local clone, it defaults to the root of the repository, but the path can be configured by setting the PROJECT_PATH taskfile variable via an argument to the task invocation command. --- .github/workflows/check-npm-task.yml | 18 +++++++++++++++--- Taskfile.yml | 9 ++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-npm-task.yml b/.github/workflows/check-npm-task.yml index 6d34f256..b022fa33 100644 --- a/.github/workflows/check-npm-task.yml +++ b/.github/workflows/check-npm-task.yml @@ -52,12 +52,18 @@ jobs: echo "result=$RESULT" >> $GITHUB_OUTPUT validate: + name: validate (${{ matrix.project.path }}) needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest permissions: contents: read + strategy: + fail-fast: false + matrix: + project: + - path: . steps: - name: Checkout repository @@ -75,15 +81,21 @@ jobs: version: 3.x - name: Validate package.json - run: task --silent npm:validate + run: task --silent npm:validate PROJECT_PATH="${{ matrix.project.path }}" check-sync: + name: check-sync (${{ matrix.project.path }}) needs: run-determination if: needs.run-determination.outputs.result == 'true' runs-on: ubuntu-latest permissions: contents: read + strategy: + fail-fast: false + matrix: + project: + - path: . steps: - name: Checkout repository @@ -101,7 +113,7 @@ jobs: version: 3.x - name: Install npm dependencies - run: task npm:install-deps + run: task npm:install-deps PROJECT_PATH="${{ matrix.project.path }}" - name: Check package-lock.json - run: git diff --color --exit-code package-lock.json + run: git diff --color --exit-code "${{ matrix.project.path }}/package-lock.json" diff --git a/Taskfile.yml b/Taskfile.yml index bb86ac3f..59bed577 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -313,13 +313,19 @@ tasks: -i \ "{{.FILE_PATH}}" + # Parameter variables: + # - PROJECT_PATH: path of the npm-managed project. Default value: "./" # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/npm-task/Taskfile.yml npm:install-deps: desc: Install dependencies managed by npm run: once + dir: | + "{{default "./" .PROJECT_PATH}}" cmds: - npm install + # Parameter variables: + # - PROJECT_PATH: path of the npm-managed project. Default value: "./" # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-npm-task/Taskfile.yml npm:validate: desc: Validate npm configuration files against their JSON schema @@ -356,7 +362,8 @@ tasks: STYLELINTRC_SCHEMA_URL: https://json.schemastore.org/stylelintrc.json STYLELINTRC_SCHEMA_PATH: sh: task utility:mktemp-file TEMPLATE="stylelintrc-schema-XXXXXXXXXX.json" - INSTANCE_PATH: "**/package.json" + INSTANCE_PATH: >- + {{default "." .PROJECT_PATH}}/package.json PROJECT_FOLDER: sh: pwd WORKING_FOLDER: From 68c06643dedd2b9db2efaafa15ae8c285e5695cc Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:40:10 -0700 Subject: [PATCH 4/7] Fix macOS/BSD incompatibility in general:check-filenames task The "Check Files" (Task) template includes an asset task named `general:check-filenames` that checks for the presence of non-portable filenames in the project. Ironically, the task itself was non-portable. The problem was that it used the `--perl-regexp` flag in the `grep` command. This flag is not supported by the BSD version of grep used on macOS and BSD machines. This caused the task to fail spuriously with `grep: unrecognized option '--perl-regexp'` errors when ran on a macOS or BSD machine. The incompatibility is resolved by changing the `--perl-regexp` flag to `--extended-regexp`. This flag, which is supported by the BSD and GNU versions of grep, allows the use of the modern and reasonable capable POSIX ERE syntax on all platforms. Unfortunately the regular expression used in the previous command relied on one of the additional features only present in the PCRE syntax. This syntax was used to check for the presence of a range of characters prohibited by the Windows filename specification: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions > Use any character [...] except for the following: > - Integer value zero, sometimes referred to as the ASCII NUL character. > - Characters whose integer representations are in the range from 1 through 31 Due to the nature of these characters, they must be represented by code in the regular expression. This was done using the `\x{hhh..}` syntax supported by PCRE. Neither that syntax nor any of the equivalent escape patterns are supported by POSIX ERE. A solution is offered in the GNU grep documentation: https://www.gnu.org/software/grep/manual/grep.html#Matching-Non_002dASCII-and-Non_002dprintable-Characters > the command `grep "$(printf '\316\233\t\317\211\n')"` is a portable albeit hard-to-read alternative As also mentioned there: > none of these techniques will let you put a null character directly into a command-line pattern So the range of characters in the pattern can not include NUL. However, it turns out that even the previous command did not detect this character although it was present by the pattern. So this limitation doesn't result in any regression in practice. --- Taskfile.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 59bed577..ddb66021 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -104,8 +104,8 @@ tasks: ' \ basename "$0" | \ grep \ - --perl-regexp \ - --regexp='"'"'([<>:"/\\|?*\x{0000}-\x{001F}])|(.+\.$)'"'"' \ + --extended-regexp \ + --regexp='"'"'([<>:"/\\|?*'"'"'"$(printf "\001-\037")"'"'"'])|(.+\.$)'"'"' \ --silent \ && \ echo "$0" From f53c06c6b3aa60b80f00dcccfc2fc29949a50273 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:43:47 -0700 Subject: [PATCH 5/7] Simplify file discovery code in `markdown:check-links` task Since projects often contain many Markdown files and new files may be added or the paths of the existing files changed frequently, the best approach for validating Markdown files is to search the project file tree recursively for all Markdown files, with exclusions configured for the paths of any externally maintained files. The `markdown:check-links` task uses the markdown-link-check tool. This tool does not have a capability for discovering Markdown files so it is necessary to use the `find` command to discover the files, then pass their paths to the markdown-link-check tool. Previously the discovery code used `find` to generate an array of paths, which was iterated over passed individually to markdown-link-check in a `for` loop. The `for` loop is unnecessary because `find` has an `-exec` flag that can be used to execute commands using the discovered paths. Although the syntax and behavior of this flag is unintuitive, these disadvantages that come from its use are outweighed by the benefits of the significant amount of code that can be replaced by it. Since the `-exec`/`-execdir` flags are already in use in the assets and project infrastructure, the maintainer will be forced to work with them regardless. --- Taskfile.yml | 57 +++++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index ddb66021..63bba56e 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -241,46 +241,35 @@ tasks: echo "Please install: https://github.com/tcort/markdown-link-check#readme" exit 1 fi - # Default behavior of the task on Windows is to exit the task when the first broken link causes a non-zero - # exit status, but it's better to check all links before exiting. - set +o errexit - STATUS=0 # Using -regex instead of -name to avoid Task's behavior of globbing even when quoted on Windows # The odd method for escaping . in the regex is required for windows compatibility because mvdan.cc/sh gives # \ characters special treatment on Windows in an attempt to support them as path separators. - for file in $( - find . \ - -type d -name '.git' -prune -o \ - -type d -name '.licenses' -prune -o \ - -type d -name '__pycache__' -prune -o \ - -type d -name 'node_modules' -prune -o \ - -regex ".*[.]md" -print - ); do - markdown-link-check \ - --quiet \ - --config "./.markdown-link-check.json" \ - "$file" - STATUS=$(( $STATUS + $? )) - done - exit $STATUS - else - npx --package=markdown-link-check --call=' - STATUS=0 - for file in $( - find . \ - -type d -name '.git' -prune -o \ - -type d -name '.licenses' -prune -o \ - -type d -name '__pycache__' -prune -o \ - -type d -name 'node_modules' -prune -o \ - -regex ".*[.]md" -print - ); do + find . \ + -type d -name ".git" -prune -o \ + -type d -name ".licenses" -prune -o \ + -type d -name "__pycache__" -prune -o \ + -type d -name "node_modules" -prune -o \ + -regex ".*[.]md" \ + -exec \ markdown-link-check \ --quiet \ --config "./.markdown-link-check.json" \ - "$file" - STATUS=$(( $STATUS + $? )) - done - exit $STATUS + \{\} \ + + + else + npx --package=markdown-link-check --call=' + find . \ + -type d -name ".git" -prune -o \ + -type d -name ".licenses" -prune -o \ + -type d -name "__pycache__" -prune -o \ + -type d -name "node_modules" -prune -o \ + -regex ".*[.]md" \ + -exec \ + markdown-link-check \ + --quiet \ + --config "./.markdown-link-check.json" \ + \{\} \ + + ' fi From 8d327882af61dc86de9408e260bf9244e14459ed Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:45:34 -0700 Subject: [PATCH 6/7] Avoid platform-specific code in `markdown:check-links` task The `markdown:check-links` task uses the markdown-link-check tool. This tool does not have a capability for discovering Markdown files so it is necessary to use the `find` command to discover the files, then pass their paths to the markdown-link-check tool. Since it is managed as a project dependency using npm, the markdown-link-check tool is invoked using npx. Since the `find` command must be ran in combination with markdown-link-check, it is necessary to use the `--call` flag of npx. Even though Windows contributors are required to use a POSIX-compliant shell such as Git Bash when working with the assets, the commands ran via the `--call` flag are executed using the native shell, which means the Windows command interpreter on a Windows machine even if the task was invoked via a different shell. This causes commands completely valid for use on a Linux or macOS machine to fail to run on a Windows machine due to the significant differences in the Windows command interpreter syntax. During the original development of the task, a reasonably maintainable cross-platform command could not be found. Lacking a better option the hacky approach was taken of using a conditional to run a different command depending on whether the task was running on Windows or not, and not using npx for the Windows command. This resulted in a degraded experience for Windows contributors because they were forced to manually manage the markdown-link-check tool dependency and make it available in the system path. It also resulted in duplication of the fairly complex code contained in the task. Following the elimination of unnecessary complexity in the task code, it became possible to use a single command on all platforms. The Windows command interpreter syntax still posed a difficulty even for the simplified command: A beneficial practice, used throughout the assets, is to break commands into multiple lines to make them and the diffs of their development easier to read. With a POSIX-compliant shell this is accomplished by escaping the introduced newlines with a backslash. However, the Windows command interpreter does not recognize this syntax, making the commands formatted in that manner invalid when the task was ran on a Windows machine. The identified solution was to define the command via a Taskfile variable. The YAML syntax was carefully chosen to support the use of the familiar backslash escaping syntax, while also producing in a string that did not contain this non-portable escaping syntax after passing through the YAML parser. --- Taskfile.yml | 67 ++++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 39 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 63bba56e..02246137 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -228,50 +228,39 @@ tasks: # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-markdown-task/Taskfile.yml markdown:check-links: desc: Check for broken links - deps: - - task: docs:generate - - task: npm:install-deps - cmds: - - | - if [[ "{{.OS}}" == "Windows_NT" ]]; then - # npx --call uses the native shell, which makes it too difficult to use npx for this application on Windows, - # so the Windows user is required to have markdown-link-check installed and in PATH. - if ! which markdown-link-check &>/dev/null; then - echo "markdown-link-check not found or not in PATH." - echo "Please install: https://github.com/tcort/markdown-link-check#readme" - exit 1 - fi - # Using -regex instead of -name to avoid Task's behavior of globbing even when quoted on Windows - # The odd method for escaping . in the regex is required for windows compatibility because mvdan.cc/sh gives - # \ characters special treatment on Windows in an attempt to support them as path separators. + vars: + # The command is defined in a Taskfile variable to allow it to be broken into multiple lines for readability. + # This can't be done in the `cmd` object of the Taskfile because `npx --call` uses the native shell, which causes + # standard newline escaping syntax to not work when the task is run on Windows. + # + # Using -regex instead of -name to avoid Task's behavior of globbing even when quoted on Windows + # The odd method for escaping . in the regex is required for windows compatibility because mvdan.cc/sh gives + # \ characters special treatment on Windows in an attempt to support them as path separators. + # + # prettier-ignore + CHECK_LINKS_COMMAND: + " find . \ - -type d -name ".git" -prune -o \ - -type d -name ".licenses" -prune -o \ - -type d -name "__pycache__" -prune -o \ - -type d -name "node_modules" -prune -o \ - -regex ".*[.]md" \ + -type d -name \".git\" -prune -o \ + -type d -name \".licenses\" -prune -o \ + -type d -name \"__pycache__\" -prune -o \ + -type d -name \"node_modules\" -prune -o \ + -regex \".*[.]md\" \ -exec \ markdown-link-check \ --quiet \ - --config "./.markdown-link-check.json" \ - \{\} \ + --config \"./.markdown-link-check.json\" \ + \\{\\} \ + - else - npx --package=markdown-link-check --call=' - find . \ - -type d -name ".git" -prune -o \ - -type d -name ".licenses" -prune -o \ - -type d -name "__pycache__" -prune -o \ - -type d -name "node_modules" -prune -o \ - -regex ".*[.]md" \ - -exec \ - markdown-link-check \ - --quiet \ - --config "./.markdown-link-check.json" \ - \{\} \ - + - ' - fi + " + deps: + - task: docs:generate + - task: npm:install-deps + cmds: + - | + npx \ + --package=markdown-link-check \ + --call='{{.CHECK_LINKS_COMMAND}}' # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-markdown-task/Taskfile.yml markdown:fix: From bc8c3bd019f2f01ae42105c92df4a107ac8dd29a Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 13 Jun 2024 14:47:25 -0700 Subject: [PATCH 7/7] Install referenced schemas in "npm:validate" task The "npm:validate" task validates the repository's `package.json` npm manifest file against its JSON schema to catch any problems with its data format. In order to avoid duplication of content, JSON schemas may reference other schemas via the `$ref` keyword. The `package.json` schema was recently updated to share resources with some other schema, which caused the validation to start failing. The solution is to configure the workflow to download those schemas as well and also to provide their path to the avj-cli validator via a `-r` flag. --- Taskfile.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Taskfile.yml b/Taskfile.yml index 02246137..733078d1 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -316,6 +316,10 @@ tasks: AVA_SCHEMA_URL: https://json.schemastore.org/ava.json AVA_SCHEMA_PATH: sh: task utility:mktemp-file TEMPLATE="ava-schema-XXXXXXXXXX.json" + # Source: https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/base.json + BASE_SCHEMA_URL: https://json.schemastore.org/base.json + BASE_SCHEMA_PATH: + sh: task utility:mktemp-file TEMPLATE="base-schema-XXXXXXXXXX.json" # Source: https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/eslintrc.json ESLINTRC_SCHEMA_URL: https://json.schemastore.org/eslintrc.json ESLINTRC_SCHEMA_PATH: @@ -328,6 +332,10 @@ tasks: NPM_BADGES_SCHEMA_URL: https://json.schemastore.org/npm-badges.json NPM_BADGES_SCHEMA_PATH: sh: task utility:mktemp-file TEMPLATE="npm-badges-schema-XXXXXXXXXX.json" + # Source: https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/partial-eslint-plugins.json + PARTIAL_ESLINT_PLUGINS_SCHEMA_URL: https://json.schemastore.org/partial-eslint-plugins.json + PARTIAL_ESLINT_PLUGINS_PATH: + sh: task utility:mktemp-file TEMPLATE="partial-eslint-plugins-schema-XXXXXXXXXX.json" # Source: https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/prettierrc.json PRETTIERRC_SCHEMA_URL: https://json.schemastore.org/prettierrc.json PRETTIERRC_SCHEMA_PATH: @@ -349,9 +357,11 @@ tasks: cmds: - wget --quiet --output-document="{{.SCHEMA_PATH}}" {{.SCHEMA_URL}} - wget --quiet --output-document="{{.AVA_SCHEMA_PATH}}" {{.AVA_SCHEMA_URL}} + - wget --quiet --output-document="{{.BASE_SCHEMA_PATH}}" {{.BASE_SCHEMA_URL}} - wget --quiet --output-document="{{.ESLINTRC_SCHEMA_PATH}}" {{.ESLINTRC_SCHEMA_URL}} - wget --quiet --output-document="{{.JSCPD_SCHEMA_PATH}}" {{.JSCPD_SCHEMA_URL}} - wget --quiet --output-document="{{.NPM_BADGES_SCHEMA_PATH}}" {{.NPM_BADGES_SCHEMA_URL}} + - wget --quiet --output-document="{{.PARTIAL_ESLINT_PLUGINS_PATH}}" {{.PARTIAL_ESLINT_PLUGINS_SCHEMA_URL}} - wget --quiet --output-document="{{.PRETTIERRC_SCHEMA_PATH}}" {{.PRETTIERRC_SCHEMA_URL}} - wget --quiet --output-document="{{.SEMANTIC_RELEASE_SCHEMA_PATH}}" {{.SEMANTIC_RELEASE_SCHEMA_URL}} - wget --quiet --output-document="{{.STYLELINTRC_SCHEMA_PATH}}" {{.STYLELINTRC_SCHEMA_URL}} @@ -361,9 +371,11 @@ tasks: --all-errors \ -s "{{.SCHEMA_PATH}}" \ -r "{{.AVA_SCHEMA_PATH}}" \ + -r "{{.BASE_SCHEMA_PATH}}" \ -r "{{.ESLINTRC_SCHEMA_PATH}}" \ -r "{{.JSCPD_SCHEMA_PATH}}" \ -r "{{.NPM_BADGES_SCHEMA_PATH}}" \ + -r "{{.PARTIAL_ESLINT_PLUGINS_PATH}}" \ -r "{{.PRETTIERRC_SCHEMA_PATH}}" \ -r "{{.SEMANTIC_RELEASE_SCHEMA_PATH}}" \ -r "{{.STYLELINTRC_SCHEMA_PATH}}" \