Skip to content

Commit

Permalink
Improves OpenDream Linting (Annotations and More) (tgstation#82029)
Browse files Browse the repository at this point in the history
## About The Pull Request

This PR does a few things.

The main goal was to have it annotate the code like we do for `Run
Dreamchecker`, which it now does. The code for this was mostly
shamelessly ripped from the very same process I just mentioned, but
stripped down a lot more for the task. Since I had to write a lot more
code to handle the way we get warnings and the like from OpenDream, I
didn't fold both into one program.


![image](https://github.com/tgstation/tgstation/assets/34697715/f4a05b59-5407-412e-a73f-5c069fdb4b02)


![image](https://github.com/tgstation/tgstation/assets/34697715/0d3335aa-0da1-45ed-af19-1d17a7fc4174)

It works! Ta-da.

Anyways, in order to facilitate the code annotation, we need to use a
Python script. This Python script means that we need to use the
bootstrap, and the bootstrap is best used with the bootstrap cache. I
then realized that we should just really fold this special CI runner
into the Run Linters workflow anyways, instead of having it on its own
runner (since it'll cut into the number of runners we have available at
a time anyways).

I think it's a good way to save precious CPU cycles on our dying Earth,
and the issues are now visible enough to the average coder (in the
`Files Changed` tab) to where I feel pretty good about doing this.

There's a bit of weirdness and jank in the Python program, but it's all
done for a reason I assure you (it's always silly when stuff breaks
silently and no one knows to fix it until years later). You can look an
example of a failing run here:
https://github.com/san7890/bruhstation/actions/runs/8303759645/job/22728427623#step:17:14
  • Loading branch information
san7890 authored Mar 16, 2024
1 parent 57c4e6e commit 49a53b4
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 19 deletions.
29 changes: 10 additions & 19 deletions .github/workflows/ci_suite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ jobs:
with:
path: tools/icon_cutter/cache
key: ${{ runner.os }}-cutter-${{ hashFiles('dependencies.sh') }}
- name: Install OpenDream
uses: robinraju/[email protected]
with:
repository: "OpenDreamProject/OpenDream"
tag: "latest"
fileName: "DMCompiler_linux-x64.tar.gz"
extract: true
- name: Install Tools
run: |
pip3 install setuptools
Expand Down Expand Up @@ -91,6 +98,9 @@ jobs:
if: steps.linter-setup.conclusion == 'success' && !cancelled()
shell: bash
run: ~/dreamchecker 2>&1 | bash tools/ci/annotate_dm.sh
- name: Run OpenDream
if: steps.linter-setup.conclusion == 'success' && !cancelled()
run: ./DMCompiler_linux-x64/DMCompiler tgstation.dme --suppress-unimplemented --define=CIBUILDING | bash tools/ci/annotate_od.sh
- name: Run Map Checks
if: steps.linter-setup.conclusion == 'success' && !cancelled()
run: |
Expand All @@ -115,25 +125,6 @@ jobs:
if: steps.linter-setup.conclusion == 'success' && !cancelled()
run: tools/build/build --ci lint tgui-test

odlint:
if: ( !contains(github.event.head_commit.message, '[ci skip]') )
name: "Lint with OpenDream"
runs-on: ubuntu-22.04
concurrency:
group: odlint-${{ github.head_ref || github.run_id }}
cancel-in-progress: true
steps:
- uses: actions/checkout@v4
- uses: robinraju/[email protected]
with:
repository: "OpenDreamProject/OpenDream"
tag: "latest"
fileName: "DMCompiler_linux-x64.tar.gz"
extract: true
- name: Run OpenDream
run: |
./DMCompiler_linux-x64/DMCompiler tgstation.dme --suppress-unimplemented --define=CIBUILDING
compile_all_maps:
if: ( !contains(github.event.head_commit.message, '[ci skip]') )
name: Compile Maps
Expand Down
4 changes: 4 additions & 0 deletions tools/ci/annotate_od.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash

set -euo pipefail
tools/bootstrap/python -m od_annotator "$@"
50 changes: 50 additions & 0 deletions tools/od_annotator/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import sys
import re

def green(text):
return "\033[32m" + str(text) + "\033[0m"

def red(text):
return "\033[31m" + str(text) + "\033[0m"

def annotate(raw_output):
# Remove ANSI escape codes
raw_output = re.sub(r'(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]', '', raw_output)

print("::group::OpenDream Output")
print(raw_output)
print("::endgroup::")

annotation_regex = r'((?P<type>Error|Warning) (?P<errorcode>OD(?P<errornumber>\d{4})) at (?P<location>(?P<filename>.+):(?P<line>\d+):(?P<column>\d+)|<internal>): (?P<message>.+))'
failures_detected = False
expected_failure_case_detected = False # this is just here so this script breaks if we forget to set it to True when we expect a failure. remove this when we have handled the expected failure

print("OpenDream Code Annotations:")
for annotation in re.finditer(annotation_regex, raw_output):
message = annotation['message']
if message == "Unimplemented proc & var warnings are currently suppressed": # this happens every single run, it's important to know about it but we don't need to throw an error
message += " (This is expected and can be ignored)" # also there's no location for it to annotate to since it's an <internal> failure.
expected_failure_case_detected = True

if annotation['type'] == "Error":
failures_detected = True

error_string = f"{annotation['errorcode']}: {message}"

if annotation['location'] == "<internal>":
print(f"::{annotation['type']} file=,line=,col=::{error_string}")
else:
print(f"::{annotation['type']} file={annotation['filename']},line={annotation['line']},col={annotation['column']}::{error_string}")

if failures_detected:
sys.exit(1)
return

if not expected_failure_case_detected:
print(red("Failed to detect the expected failure case! If you have recently changed how we work with OpenDream Pragmas, please fix the od_annotator script!"))
sys.exit(1)
return

print(green("No OpenDream issues found!"))

annotate(sys.stdin.read())

0 comments on commit 49a53b4

Please sign in to comment.