[Test PR] Investigate "No space left on device" error (A) #3
+1,191
−735
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In GitHub Actions workflow files, this separates top-level keys within a job with blank lines, which was sometimes but not usually already done, since this seems to improve readability. Besides that, this also applies the prevailing style more consistently. This style tweak is in preparation for adding explicit `permissions` keys (so that doing so won't decrease readability).
This adds `permissions` keys at workflow or job level in a number of places, usually specifying minimal required permissions. When a `permissions` key contains at least one subkey setting some permission (in any way), all permissions it does not have subkeys for are treated as if they were present with a value of `none`. This relies on that rather than listing all unneeded permissions everywhere with `none`. So most `permissions` added here have only `contents: read`, and: - The only place where `none` is specified explicitly is in the CIFuzz workflow, where no permissions (of those controllable through `permissions`) are needed. - The only place any `write` permissions are specified are `contents: write` in the jobs of the release workflow that need it. All jobs involved in preparing a release currently have at least one step that requires this. But `contents: read` is still applied at the workflow level, because the `installation` job (which tests installing) does not need any `write` permissions. Note that some jobs that don't have any write permissions of the kind that is controlled under a `permissions` key do still perform writes to data accessible outside of their run: caching (including creating new caches), and uploading artifacts, still works. Relevant documentation: - https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#permissions - https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idpermissions - https://docs.github.com/en/rest/authentication/permissions-required-for-github-apps
This makes more stylistic adjustments to YAML workflows files.
(Previous recent adjustments were, for the most part, only those
needed to ensure clarity would be preserved while adding
`permissions:`. This commit does some others that weren't needed
for that.)
The changes made here are:
- Use `actions/checkout@v4`, not `actions/checkout@master`, in the
one place where `@master` was still used for it. This is not
strictly speaking a purely stylistic change, since it is
eventually expected to make a difference, by holding onto major
version 4 until the version is updated (which Depedabot
automates).
- Don't explicitly set the `CI` environment variable in the `test`
job. This variable is guaranteed to be set automatically with the
value of `true` by the GitHub Actions runner. Furthermore, no
environment variables to indicate CI are explicitly set for other
jobs, even though others rely on their presence as well due to
the use of `is_ci` through `gix-testtools`.
- Specify all environment variables' values as strings. Most
values in YAML are strings without explicit quoting. This does
not add quotation marks in those cases. But some values are
parsed as some other type, such as integer, unless quoted. That
makes it less obvious what the value will actually be in the
environment, since it will be implicitly converted to a string,
which does not always consist of the same sequence of characters
as the original expression. This effect is most prominent for
booleans (e.g. unquoted `yes` and `true` in YAML have the same
meaning) but not entirely limited to them. In addition, such
expressions may also lead to confusion if it is misread to mean
that they will retain any kind of type information. So this
quotes such values (except when other changes here remove them).
- Minor quoting style adjustments, for YAML strings are quoted.
Omit quotes when clearly not needed, and use single-quotes by
default when quotes are needed.
- Use 2-space indent inside scripts in script steps for consistency
with other scripts. Most existing multi-line script steps, as
well as all shell scripts, use 2-space indents already.
- Remove `\` in a script step where it was not needed for line
continuation because it followed a `|`.
- In the `wasm` job's script steps, put `name:` and, when present,
`if:`, ahead of `run:`, so it's clear what each step is for.
(This was already done where applicable in other jobs.)
- In the `wasm` job's script steps, split single-line `run:` values
with `&&` into separate commands, one per line, where doing so
has the same effect due to the `-e` option the CI runner
automatically passes to `bash` shells unless `shell:` is
overridden with a value that contains `{0}`.
- In the `wasm` job's script steps, split single-line `run:`
values with `;` into separate lines, including running loops over
multiple lines, for readability.
- In the `wasm` job, extract `${{ matrix.target }}` to an
environment variable. This seems to make the steps slightly more
readable, since `wasm` steps make heavy use of it.
- Extremely minor adjustment to array style, for consistency.
- In commands, use `--` where guaranteed to be supported if any
non-option argument begins with parameter expansion or a glob.
This was already almost, but not quite, already done. Since the
possible values can be inferred from nearby code and do not
begin with `-`, the reason is to clearly communicate that they
are non-option arguments, rather than to fix any actual bug.
(This continues to be avoided where is not guaranteed correct.)
- Use the `-e` option before a pattern argument to `grep` formed
through parameter expansion (same rationale as `--` above).
- Use `-E` rather than `-r` with `sed`, since `-E` is standardized.
In the past (GitoxideLabs#143), Dependabot version updates had been used to keep `cargo` dependencies up to date. This was removed in favor of doing manual updates based on automatic reports from `cargo deny` and the old `dependabot.yml` was kept but renamed to disable it and point people to GitoxideLabs#144 to learn about the change. Since then, Dependabot security updates, which are distinct from Dependabot version updates, were enabled (see GitoxideLabs#1254), and later, Dependabot version updates were reintroduced for GitHub Actions only (GitoxideLabs#1357). At that point, there were two Dependabot-related YAML files: the old disabled one, and the new one for GHA. This removes the old one, explaining the situation in a comment in the new one, including a link to GitoxideLabs#144. While doing so, this also adjusts the YAML code style there, to bring it in line with the style of most other YAML files in the repository.
This gives `name:` keys to the `test-pass` steps, turning the first step's comment into its name. This way, the output can be as clear as the workflow file itself.
This intentionally fails right now, by omitting `tests-pass` itself as a job that `tests-pass` should not depend on, in order to ensure that it is able to fail. Once this is observed (and any other bugs fixed), this omission should be corrected, and then it should pass.
- Show the job lists for better debugging. - Reword the `diff` step name to be clearer (albeit less precise).
Changes: - Bump the version of `cargo-diet` used in the `lint` job from 1.2.4 to 1.2.7. - Use the version tag not just as an operand to `--tag`, but also as the ref from which the installation script itself is obtained. (The rationale is that the effect of skew here would probably be unintuitive, and also that we had been specifying `master` but the default branch of the `cargo-diet` repository is now `main`.) - Instead of using `|| true` on the step that runs `just check-size` (which needs cargo-diet), split installing and running into separate steps, and mark the running step `continue-on-error`.
The WebAssembly CI job definition had an unconditional job-level `continue-on-error` set to `true`, so that it would always report a successful conclusion, even on failure. This made sense before PR auto-merge was set up. But these jobs do not typically fail, and their failures should be more apparent, even if they should not yet block PRs from being auto-merged. This commit changes the `wasm` jobs so they are able to fail and cause the workflow as a whole to have a failing status, while still not blocking PR auto-merge. `continue-on-error` is removed from the job definition, but it is also no longer made a dependency of the required `tests-pass` check.
EliahKagan
changed the title
EliahKagan
added a commit
that referenced
this pull request
Nov 12, 2024
The problem seems to happen only on the upstream repository, or maybe only on PRs from forks. This initially makes no changes, to verify that the problem does not simply happen regardless of content. See GitoxideLabs#1668, and in particular GitoxideLabs#1668 (comment), as well as #3, and #4, for details.
That way, the runner hopefully doesn't run out of disk space while it is compiling the various targets used in a journey test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fork-internal PR is to investigate the failure described in GitoxideLabs#1668 (comment) that currently is blocking GitoxideLabs#1668 (and GitoxideLabs#1670). This PR should not be merged. See also #4.