Run fix_bss.py in Jenkins and generate a patch (#2168)

* fix_bss.py: Disable colors if stdout is not a tty

* Run fix_bss.py in CI and output a patch

* Wording tweaks

Jenkinsfile

@@ -20,8 +20,10 @@ pipeline {
                 }
             }
             steps {
-                echo 'Checking formatting on modified files...'
-                sh 'python3 tools/check_format.py --verbose --compare-to origin/main'
+                catchError(buildResult: 'FAILURE', stageResult: 'FAILURE') {
+                    echo 'Checking formatting on modified files...'
+                    sh 'python3 tools/check_format.py --verbose --compare-to origin/main'
+                }
             }
         }
         stage('Build ntsc-1.2, check disasm metadata') {
@@ -38,66 +40,70 @@ pipeline {
         // NTSC/PAL/MQ/DEBUG as quickly as possible.
         stage('Build gc-jp') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-jp.z64 baseroms/gc-jp/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-jp'
-                sh 'make -j$(nproc) VERSION=gc-jp'
-                sh 'make clean assetclean VERSION=gc-jp'
-            }
+                script {
+                    build('gc-jp')
+                }
+           }
         }
         stage('Build gc-eu-mq') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-eu-mq.z64 baseroms/gc-eu-mq/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-eu-mq'
-                sh 'make -j$(nproc) VERSION=gc-eu-mq'
-                sh 'make clean assetclean VERSION=gc-eu-mq'
+                script {
+                    build('gc-eu-mq')
+                }
             }
         }
         stage('Build gc-eu-mq-dbg') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-eu-mq-dbg.z64 baseroms/gc-eu-mq-dbg/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-eu-mq-dbg'
-                sh 'make -j$(nproc) VERSION=gc-eu-mq-dbg'
-                sh 'make clean assetclean VERSION=gc-eu-mq-dbg'
+                script {
+                    build('gc-eu-mq-dbg')
+                }
             }
         }
         stage('Build gc-us') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-us.z64 baseroms/gc-us/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-us'
-                sh 'make -j$(nproc) VERSION=gc-us'
-                sh 'make clean assetclean VERSION=gc-us'
+                script {
+                    build('gc-us')
+                }
             }
         }
         stage('Build gc-jp-ce') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-jp-ce.z64 baseroms/gc-jp-ce/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-jp-ce'
-                sh 'make -j$(nproc) VERSION=gc-jp-ce'
-                sh 'make clean assetclean VERSION=gc-jp-ce'
+                script {
+                    build('gc-jp-ce')
+                }
             }
         }
         stage('Build gc-eu') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-eu.z64 baseroms/gc-eu/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-eu'
-                sh 'make -j$(nproc) VERSION=gc-eu'
-                sh 'make clean assetclean VERSION=gc-eu'
+                script {
+                    build('gc-eu')
+                }
             }
         }
         stage('Build gc-jp-mq') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-jp-mq.z64 baseroms/gc-jp-mq/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-jp-mq'
-                sh 'make -j$(nproc) VERSION=gc-jp-mq'
-                sh 'make clean assetclean VERSION=gc-jp-mq'
+                script {
+                    build('gc-jp-mq')
+                }
             }
         }
         stage('Build gc-us-mq') {
             steps {
-                sh 'ln -s /usr/local/etc/roms/oot-gc-us-mq.z64 baseroms/gc-us-mq/baserom.z64'
-                sh 'make -j$(nproc) setup VERSION=gc-us-mq'
-                sh 'make -j$(nproc) VERSION=gc-us-mq'
-                sh 'make clean assetclean VERSION=gc-us-mq'
+                script {
+                    build('gc-us-mq')
+                }
+            }
+        }
+        stage('Generate patch') {
+            when {
+                not {
+                    branch 'main'
+                }
+            }
+            steps {
+                sh 'git diff'
+                echo 'Generating patch...'
+                sh 'tools/generate_patch_from_jenkins.sh'
             }
         }
     }
@@ -115,3 +121,20 @@ pipeline {
         }
     }
 }
+
+def build(String version) {
+    sh "ln -s /usr/local/etc/roms/oot-${version}.z64 baseroms/${version}/baserom.z64"
+    sh "make -j\$(nproc) setup VERSION=${version}"
+    try {
+        sh "make -j\$(nproc) VERSION=${version}"
+    } catch (e) {
+        echo "Build failed, attempting to fix BSS ordering..."
+        sh ".venv/bin/python3 tools/fix_bss.py -v ${version}"
+        // If fix_bss.py succeeds, continue the build, but ensure both the build and current stage are marked as failed
+        catchError(buildResult: 'FAILURE', stageResult: 'FAILURE') {
+            sh 'exit 1'
+        }
+    } finally {
+        sh "make clean assetclean VERSION=${version}"
+    }
+}

tools/fix_bss.py

@@ -19,7 +19,7 @@ import shlex
 import sys
 import time
 import traceback
-from typing import BinaryIO, Iterator, Tuple
+from typing import BinaryIO, Iterator, Optional, Tuple
 
 from ido_block_numbers import (
     generate_make_log,
@@ -33,6 +33,17 @@ import elftools.elf.elffile
 import mapfile_parser.mapfile
 
 
+# Set on program start since we replace sys.stdout in worker processes
+stdout_isatty = sys.stdout.isatty()
+
+
+def output(message: str = "", color: Optional[str] = None, end: str = "\n"):
+    if color and stdout_isatty:
+        print(f"{color}{message}{colorama.Fore.RESET}", end=end)
+    else:
+        print(message, end=end)
+
+
 def read_u32(f: BinaryIO, offset: int) -> int:
     f.seek(offset)
     return int.from_bytes(f.read(4), "big")
@@ -208,8 +219,8 @@ def compare_pointers(version: str) -> dict[Path, BssSection]:
         source_code_segments.append(mapfile_segment)
 
     # Find all pointers with different values
-    if not sys.stdout.isatty():
-        print(f"Comparing pointers between baserom and build ...")
+    if not stdout_isatty:
+        output(f"Comparing pointers between baserom and build ...")
     pointers = []
     file_results = []
     with multiprocessing.Pool(
@@ -230,22 +241,22 @@ def compare_pointers(version: str) -> dict[Path, BssSection]:
         while True:
             time.sleep(0.010)
             num_files_done = sum(file_result.ready() for file_result in file_results)
-            if sys.stdout.isatty():
-                print(
+            if stdout_isatty:
+                output(
                     f"Comparing pointers between baserom and build ... {num_files_done:>{len(f'{num_files}')}}/{num_files}",
                     end="\r",
                 )
             if num_files_done == num_files:
                 break
-        if sys.stdout.isatty():
-            print("")
+        if stdout_isatty:
+            output("")
 
         # Collect results and check for errors
         for file_result in file_results:
             try:
                 pointers.extend(file_result.get())
             except FixBssException as e:
-                print(f"{colorama.Fore.RED}Error: {str(e)}{colorama.Fore.RESET}")
+                output(f"Error: {str(e)}", color=colorama.Fore.RED)
                 sys.exit(1)
 
     # Remove duplicates and sort by baserom address
@@ -674,27 +685,27 @@ def process_file(
     dry_run: bool,
     version: str,
 ):
-    print(f"{colorama.Fore.CYAN}Processing {file} ...{colorama.Fore.RESET}")
+    output(f"Processing {file} ...", color=colorama.Fore.CYAN)
 
     command_line = find_compiler_command_line(make_log, file)
     if command_line is None:
         raise FixBssException(f"Could not determine compiler command line for {file}")
 
-    print(f"Compiler command: {shlex.join(command_line)}")
+    output(f"Compiler command: {shlex.join(command_line)}")
     symbol_table, ucode = run_cfe(command_line, keep_files=False)
 
     bss_variables = find_bss_variables(symbol_table, ucode)
-    print("BSS variables:")
+    output("BSS variables:")
     for var in bss_variables:
         i = var.block_number
-        print(
+        output(
             f"  {i:>6} [{i%256:>3}]: size=0x{var.size:04X} align=0x{var.align:X} referenced_in_data={str(var.referenced_in_data):<5} {var.name}"
         )
 
     build_bss_symbols = predict_bss_ordering(bss_variables)
-    print("Current build BSS ordering:")
+    output("Current build BSS ordering:")
     for symbol in build_bss_symbols:
-        print(
+        output(
             f"  offset=0x{symbol.offset:04X} size=0x{symbol.size:04X} align=0x{symbol.align:X} referenced_in_data={str(symbol.referenced_in_data):<5} {symbol.name}"
         )
 
@@ -702,9 +713,9 @@ def process_file(
         raise FixBssException(f"No pointers to BSS found in ROM for {file}")
 
     base_bss_symbols = determine_base_bss_ordering(build_bss_symbols, bss_section)
-    print("Baserom BSS ordering:")
+    output("Baserom BSS ordering:")
     for symbol in base_bss_symbols:
-        print(
+        output(
             f"  offset=0x{symbol.offset:04X} size=0x{symbol.size:04X} align=0x{symbol.align:X} referenced_in_data={str(symbol.referenced_in_data):<5} {symbol.name}"
         )
 
@@ -717,15 +728,15 @@ def process_file(
             f"Too many increment_block_number pragmas found in {file} (found {len(pragmas)}, max {max_pragmas})"
         )
 
-    print("Solving BSS ordering ...")
+    output("Solving BSS ordering ...")
     new_pragmas = solve_bss_ordering(pragmas, bss_variables, base_bss_symbols)
-    print("New increment_block_number amounts:")
+    output("New increment_block_number amounts:")
     for pragma in new_pragmas:
-        print(f"  line {pragma.line_number}: {pragma.amount}")
+        output(f"  line {pragma.line_number}: {pragma.amount}")
 
     if not dry_run:
         update_source_file(version, file, new_pragmas)
-        print(f"{colorama.Fore.GREEN}Updated {file}{colorama.Fore.RESET}")
+        output(f"Updated {file}", color=colorama.Fore.GREEN)
 
 
 def process_file_worker(*x):
@@ -737,17 +748,17 @@ def process_file_worker(*x):
         process_file(*x)
     except FixBssException as e:
         # exception with a message for the user
-        print(f"{colorama.Fore.RED}Error: {str(e)}{colorama.Fore.RESET}")
+        output(f"Error: {str(e)}", color=colorama.Fore.RED)
         raise
     except Exception as e:
         # "unexpected" exception, also print a trace for devs
-        print(f"{colorama.Fore.RED}Error: {str(e)}{colorama.Fore.RESET}")
+        output(f"Error: {str(e)}", color=colorama.Fore.RED)
         traceback.print_exc(file=sys.stdout)
         raise
     finally:
         sys.stdout = old_stdout
-        print()
-        print(fake_stdout.getvalue(), end="")
+        output()
+        output(fake_stdout.getvalue(), end="")
 
 
 def main():
@@ -797,11 +808,11 @@ def main():
             files_with_reordering.append(file)
 
     if files_with_reordering:
-        print("Files with BSS reordering:")
+        output("Files with BSS reordering:")
         for file in files_with_reordering:
-            print(f"  {file}")
+            output(f"  {file}")
     else:
-        print("No BSS reordering found.")
+        output("No BSS reordering found.")
 
     if args.files:
         # Ignore files that don't have a BSS section in the ROM
@@ -811,7 +822,7 @@ def main():
     if not files_to_fix:
         return
 
-    print(f"Running make to find compiler command line ...")
+    output(f"Running make to find compiler command line ...")
     make_log = generate_make_log(version)
 
     with multiprocessing.Pool() as p:
@@ -836,12 +847,13 @@ def main():
         # Collect results and check for errors
         num_successes = sum(file_result.successful() for file_result in file_results)
         if num_successes == len(file_results):
-            print()
-            print(f"Processed {num_successes}/{len(file_results)} files.")
+            output()
+            output(f"Processed {num_successes}/{len(file_results)} files.")
         else:
-            print()
-            print(
-                f"{colorama.Fore.RED}Processed {num_successes}/{len(file_results)} files.{colorama.Fore.RESET}"
+            output()
+            output(
+                f"Processed {num_successes}/{len(file_results)} files.",
+                color=colorama.Fore.RED,
             )
             sys.exit(1)
 

tools/generate_patch_from_jenkins.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+set -euo pipefail
+
+PATCH=$(git diff | base64 -w 0)
+if [ -n "$PATCH" ]; then
+    echo "Jenkins made some fixes to your PR. To apply these changes to your working directory,"
+    echo "copy and run the following command (triple-click to select the entire line):"
+    echo
+    echo "echo -n $PATCH | base64 -d | git apply -"
+    echo
+fi