Skip to content

Sync infrastructure assets from upstream #20

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 9 commits into from
Sep 14, 2022
Merged

Sync infrastructure assets from upstream #20

merged 9 commits into from
Sep 14, 2022

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Sep 14, 2022

This project's infrastructure uses standardized assets from a collection maintained in a dedicated repository:

https://github.com/arduino/tooling-project-assets

Some advancements have been made to the files upstream, which are pulled into this project here.

See the individual commit messages for details on each individual change.

The tools used for the development, maintenance, and project management of this repository must be configured to ignore
irrelevant files. Standardized configurations intended to be applicable to any Arduino tooling project are maintained in
a centralized collection of such "template" assets:

https://github.com/arduino/tooling-project-assets

Some development has occurred on the upstream configurations since the time the assets were established in this
repository.

This project's assets are hereby brought up to date with the latest upstream versions.
The GitHub Actions workflows contain some comments that provide references to more information about parts that might
not be clear to maintainers or contributors.

Some small improvements are hereby made to these URLs, including enabling localization.
Some of this project's tool dependencies are Python packages. In order to allow the project to benefit from ongoing
development of those dependencies, the maintainers must keep these packages updated.

This can be done in an efficient manner by using the Dependabot service.

Dependabot will periodically check the versions of all Python packages used by this project. If any are found to be
outdated, it will submit a pull request to update them.
GitHub Actions workflows used to validate project content are configured to be triggered by any push or pull request
that modifies relevant files. These workflows rely on external resources such as the GitHub Actions infrastructure,
GitHub Actions actions, dependencies. Inevitably, breaking changes will occur in these resources, requiring adaptation
of the project infrastructure to fix.

If the workflow is only configured to run on changes to project files, breakage caused by external changes will only be
revealed by contributor activity. This results in confusion for the contributor who will assume the failing workflow run
was caused by the change they made. This results in inconvenience for the maintainer who must explain the situation to
the contributor and do without the benefit of the automated validation of the contribution.

For this reason, it is best practices to always configure such GitHub Actions workflows to also be triggered on a
schedule. This allows breakage to be detected and resolved before it impacts contributions.
The trunk-based development strategy is used by some tooling projects (e.g., Arduino CLI). Their release branches may
contain a subset of the history of the default branch.

The status of the GitHub Actions workflows should be evaluated before making a release. However, this is not so simple as
checking the status of the commit at the tip of the release branch. The reason is that, for the sake of efficiency, the
workflows are configured to run only when the processes are relevant to the trigger event (e.g., no need to run unit
tests for a change to the readme).

For this reason, it is necessary to to trigger all relevant workflows on the creation of a release branch.

This project does not use release branches, so does not benefit from the added support for them. However, these
workflows are designed to be reusable for any Arduino tooling project with the minimum amount of project-specific
configuration. The benefits of reduced installation and maintenance effort that comes from this standardization outweigh
any harm done by asset content that is superfluous for a specific project.
…m links

The link check job of the "Check Markdown" workflow was subject to frequent intermittent spurious failures in other
repositories, caused by links under the docs.github.com domain returning 403 HTTP status.

Others experiencing the same problem reported that they were able to work around the problem by providing a custom
`Accept-Encoding` HTTP request header. Although I was not able to find any explanation of why, it does resolve the
problem.

Even if this specific project does not contain any links to the problematic domain, this is a standardized configuration
file maintained in a centralized collection of reusable assets which are intended to be applicable to any Arduino
tooling project. So the copies of these assets should be kept in sync with the upstream files, even in cases where the
sync does not provide any immediate benefit to the project.
The Task task runner tool uses an integrated Bash shell interpreter to allow the use of standard POSIX/Bash syntax and
built-in utilities while providing cross-platform support for users who have made the poor choice of the non-standard
Windows cmd or PowerShell shells.

While this is a nice feature of Task, distinguishing it from other alternatives, it is still often necessary to use
non-built-in utilities. So use of a Bash shell is a requirement to run our tasks even the developers using Windows.

Although high quality Bash shells for Windows such as the Git Bash included as part of the Git for Windows installation
generally provide a seamless experience, there is the occasional "gotcha".

Relative paths work the same for POSIX and Windows, but absolute POSIX format paths may not. These paths are handled as
expected by Task's integrated shell interpreter and by Bash, but when these paths are used outside that context they may
no longer be correct.

For example:

```
  foo:
    cmds:
      - mkdir "/tmp/posix-path-test-dir"
      - touch "/tmp/posix-path-test-dir/posix-path-test-file"
      - ls "/tmp/posix-path-test-dir"
      - cd "/tmp/posix-path-test-dir"
```

The first three commands work as expected, but the `cd` command fails:

```
$ task foo
task: [foo] mkdir "/tmp/posix-path-test-dir"
task: [foo] touch "/tmp/posix-path-test-dir/posix-path-test-file"
task: [foo] ls "/tmp/posix-path-test-dir"
posix-path-test-file
task: [foo] cd "/tmp/posix-path-test-dir"
task: Failed to run task "foo": exit status 1
```

The POSIX format `/tmp` is actually set to the system temporary directory:

```
$ cygpath -w "/tmp/posix-path-test-dir"
C:\Users\per\AppData\Local\Temp\posix-path-test-dir
```

But if treated as a Windows format path, this would be the `tmp` subfolder of the root of the current drive (e.g.,
`C:\tmp`). Even though the command was run from Git Bash, which provides a `cd` that handles this absolute path
perfectly, when run from Task the Windows native `cd` is used instead.

This resulted in several of the data file validation tasks, which download the JSON schema to the temporary folder, to
fail when ran on Windows.

The solution is to convert these absolute paths, the occurrence of which are relatively rare in our tasks, to Windows
format (which is handled fine by both the integrated, Bash, and external applications) when the task is ran on a Windows
machine. The cygpath utility provides this capability:

```
$ task foo
task: [foo] mkdir "C:\Users\per\AppData\Local\Temp/posix-path-test-dir"
task: [foo] touch "C:\Users\per\AppData\Local\Temp/posix-path-test-dir/posix-path-test-file"
task: [foo] ls "C:\Users\per\AppData\Local\Temp/posix-path-test-dir"
posix-path-test-file
task: [foo] cd "C:\Users\per\AppData\Local\Temp/posix-path-test-dir"
```
Some of the project infrastructure uses tools sourced from the npm software registry.

Previously, the version of the tools used was not controlled. This was problematic because:

- A different version of the tool may be used on the contributor's machine than on the CI runner, resulting in confusing
  failures.
- The project is immediately subject to disruption or breakage resulting from a release of the tool.

---

These tools were installed via either of the following methods:

`npx <pkg>`

This approach has the following behaviors of interest:

https://docs.npmjs.com/cli/v8/commands/npx#description

> If any requested packages are not present in the local project dependencies, then they are installed to a folder in
the npm cache, which is added to the PATH environment variable in the executed process.

> Package names provided without a specifier will be matched with whatever version exists in the local project. Package
names with a specifier will only be considered a match if they have the exact same name and version as the local
dependency.

This means that the version used was:

1. Whatever happens to be present in the local cache
2. The latest available version if it is not already present

`npm install --global <pkg>`

The latest available version of the package is used.

---
`
The new approach is to specify the version of the tools via the standard npm metadata files (package.json +
package-lock.json). This approach was chosen over the `npx <pkg>@<version>` alternative for the following reasons:

- Enables automated updates via Dependabot PRs
- Enables automated vulnerability alerts
- Separates dependency management from the asset contents (i.e., no need to mess with the taskfile or workflow on every
  update)
- Matches how we are already managing Python dependencies (pyproject.toml + poetry.lock)
@per1234 per1234 added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure labels Sep 14, 2022
@per1234 per1234 self-assigned this Sep 14, 2022
On every push and pull request that affects relevant files, and periodically:

- Validate package.json against its JSON schema.
- Check for forgotten package-lock.json syncs.
Copy link
Contributor

@MatteoPologruto MatteoPologruto left a comment

Choose a reason for hiding this comment

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

Amazing, thanks Per!

@per1234 per1234 merged commit ce63040 into arduino:master Sep 14, 2022
@per1234 per1234 deleted the sync-ci branch September 14, 2022 06:54
@per1234 per1234 linked an issue Sep 15, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bring CI up to tooling standard for the arduinoOTA repo
2 participants