Skip to content

Conversation

@rindeal
Copy link
Contributor

@rindeal rindeal commented Aug 10, 2024

Most important of the fixes is the first one, which sets TRUNK_LAUNCHER_QUIET=false.

The launcher currently completely suppresses all output (including all errors),
unless TRUNK_LAUNCHER_QUIET env var is set to false.

if [[ ${TRUNK_LAUNCHER_QUIET} != false ]]; then
  exec 3>&1 4>&2 &>/dev/null
fi

This prevents each and every one from debugging failed runs.
Redirecting file descriptors like that also disables all set -vx output,
resulting in a complete blackout and painful debugging.

This PR will always enable launcher output, which is what I prefer, however if you don't like that, it may be changed, so that it's set only if RUNNER_DEBUG == 1.

Also, you can migrate to actions/download-artifact@v4 and rewrite this step:

trunk-action/action.yaml

Lines 355 to 378 in 03cb46f

- name: Download annotations artifact
if: inputs.post-annotations == 'true'
uses: actions/github-script@v6
with:
# TODO(chris): We can't use the official download artifact action yet: https://github.com/actions/download-artifact/issues/172
script: |
let artifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: ${{ github.event.workflow_run.id }},
});
let matchArtifact = artifacts.data.artifacts.filter((artifact) => {
return artifact.name == "trunk-annotations"
})[0];
if (matchArtifact) {
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
let fs = require('fs');
fs.writeFileSync('${{ env.TRUNK_TMPDIR }}/annotations.zip', Buffer.from(download.data));
}

rindeal and others added 5 commits August 11, 2024 00:23
The output contains critical information in case of launcher errors.

The launcher currently completely suppresses all output (including all errors),
unless `TRUNK_LAUNCHER_QUIET` env var is set to `false`.

```bash
if [[ ${TRUNK_LAUNCHER_QUIET} != false ]]; then
  exec 3>&1 4>&2 &>/dev/null
fi
```

This prevents each and every one from debugging failed runs.
Redirecting file descriptors like that also disables all `set -vx` output,
resulting in a complete blackout and painful debugging.
Do not use POSIX test expressions.
The progress spinners cause a bunch of "blank"-looking lines
in the log like this:
```
2024-08-10T22:47:58.8716066Z �[2K�[0G
2024-08-10T22:47:59.0718541Z �[2K�[0G
2024-08-10T22:47:59.2719909Z �[2K�[0G
2024-08-10T22:47:59.4720633Z �[2K�[0G
2024-08-10T22:47:59.6722099Z �[2K�[0G
2024-08-10T22:47:59.8722445Z �[2K�[0G
2024-08-10T22:47:59.9751114Z �[2K�[0G
2024-08-10T22:48:00.1794815Z �[2K�[0G�[0G
2024-08-10T22:48:00.3794984Z �[2K�[0G
2024-08-10T22:48:00.5797120Z �[2K�[0G
2024-08-10T22:48:00.7798352Z �[2K�[0G
2024-08-10T22:48:00.9799600Z �[2K�[0G
2024-08-10T22:48:01.1800856Z �[2K�[0G
2024-08-10T22:48:01.3802270Z �[2K�[0G
2024-08-10T22:48:01.5803572Z �[2K�[0G
2024-08-10T22:48:01.7804948Z �[2K�[0G
2024-08-10T22:48:01.9805564Z �[2K�[0G
2024-08-10T22:48:02.1806545Z �[2K�[0G
2024-08-10T22:48:02.2442270Z �[2K�[0G
2024-08-10T22:48:03.1623273Z �[2K�[0G�[1m�[32m
2024-08-10T22:48:03.1625751Z ✔ �[0m�[1m10 linters were enabled �[0m�[90m(�[0m�[4m�[90m.trunk/trunk.yaml�[0m�[90m)
```

`--ci` options reduces their refresh rate to 30s, effectively making it a non-issue.
@EliSchleifer EliSchleifer requested review from det and puzzler7 October 10, 2024 17:23
Copy link
Collaborator

@puzzler7 puzzler7 left a comment

Choose a reason for hiding this comment

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

LGTM

@puzzler7 puzzler7 merged commit 2eaee16 into trunk-io:main Oct 10, 2024
@rindeal rindeal deleted the fixes-2 branch October 14, 2024 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants