Handle invalid data in COLUMNS env var when determining console width

[scollinson]

Description

Building on the main branch gives me the following error:

[👉]    Summary: u-boot patching: 8 total patches; 8 applied; 0 with problems
[👉]    Traceback (most recent call last):
[👉]      File "/opt/broker/firmware/armbian/lib/tools/patching.py", line 447, in <module>
[👉]        console_width = (int(os.environ.get("COLUMNS", 160)) - 12) if os.environ.get("GITHUB_ACTIONS", "") == "" else 160
[👉]                         ~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[👉]    ValueError: invalid literal for int() with base 10: ''
[💥]     👆👆👆 Showing logfile above 👆👆👆 [ /opt/broker/firmware/armbian/.tmp/logs-no-uuidgen-yet-28734-9577/023.000.compile_uboot.log ]
[💥] Error 1 occurred in main shell [ at /opt/broker/firmware/armbian/lib/functions/logging/runners.sh:247
    run_host_command_logged_raw() --> lib/functions/logging/runners.sh:247
        run_host_command_logged() --> lib/functions/logging/runners.sh:229
     uboot_main_patching_python() --> lib/functions/compilation/uboot-patching.sh:56
                  do_with_hooks() --> lib/functions/general/extensions.sh:617
             patch_uboot_target() --> lib/functions/compilation/uboot.sh:42
           compile_uboot_target() --> lib/functions/compilation/uboot.sh:69
   loop_over_uboot_targets_and_do() --> lib/functions/compilation/uboot.sh:364
                  compile_uboot() --> lib/functions/compilation/uboot.sh:455
                do_with_logging() --> lib/functions/logging/section-logging.sh:85
   artifact_uboot_build_from_sources() --> lib/functions/artifacts/artifact-uboot.sh:178
    artifact_build_from_sources() --> lib/functions/artifacts/artifacts-obtain.sh:34
       obtain_complete_artifact() --> lib/functions/artifacts/artifacts-obtain.sh:280
       build_artifact_for_image() --> lib/functions/artifacts/artifacts-obtain.sh:392
    main_default_build_packages() --> lib/functions/main/build-packages.sh:102
   full_build_packages_rootfs_and_image() --> lib/functions/main/default-build.sh:31
          do_with_default_build() --> lib/functions/main/default-build.sh:42
         cli_standard_build_run() --> lib/functions/cli/cli-build.sh:25
        armbian_cli_run_command() --> lib/functions/cli/utils-cli.sh:136
                 cli_entrypoint() --> lib/functions/cli/entrypoint.sh:208
                           main() --> ./compile.sh:50
 ]

How Has This Been Tested?

My build now completes, both locally and in CI.

Checklist:

Please delete options that are not relevant.

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• My changes generate no new warnings

Summary by CodeRabbit

• Bug Fixes
  • Prevented crashes from malformed console settings by safely parsing width input, falling back to sensible defaults, and enforcing a minimum usable width.
  • Ensures a consistent console width in CI environments and applies the corrected width when initializing the display renderer, improving layout stability and readability.
  • Improved robustness against non-numeric terminal configurations to avoid layout regressions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 94506a60-1b09-4888-9928-4f98379199c5

📥 Commits

Reviewing files that changed from the base of the PR and between 9600e5b and 79610ea.

📒 Files selected for processing (1)

• lib/tools/patching.py

🚧 Files skipped from review as they are similar to previous changes (1)

• lib/tools/patching.py

---

📝 Walkthrough

Walkthrough

Refactored console width computation in lib/tools/patching.py: when GITHUB_ACTIONS is unset, COLUMNS is now parsed with try/except to fall back to 160 on non-integer values, clamped to a minimum of 120, then reduced by 12. When GITHUB_ACTIONS is set, width remains 160.

Changes

Cohort / File(s)	Summary
Console Width Calculation lib/tools/patching.py	Wrap COLUMNS parsing in try/except ValueError; fallback to 160 on parse failure; clamp parsed value to >=120 before subtracting 12; preserve fixed 160 when GITHUB_ACTIONS is set; Rich Console constructed with updated width.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped by the terminal breeze,
Found COLUMNS giving me unease,
A try and a catch,
A clamp and a patch,
Now the console stretches with ease 🥕✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding error handling for invalid COLUMNS environment variable values when calculating console width.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/tools/patching.py`:
- Around line 447-453: The computed console_width assignment can still be <= 0
when COLUMNS is numeric but small; update the block that sets console_width (the
variable computed in the if os.environ.get("GITHUB_ACTIONS", "") == "": branch)
to validate the parsed value after subtracting 12 and clamp it to a sensible
minimum (e.g., 40) or fall back to 160 if result <= 0; ensure you handle
ValueError as before and apply the same clamp/fallback in the GitHub Actions
else branch if desired.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fa951d06-fcc1-4e2f-a2a6-0df8d2e8e383

📥 Commits

Reviewing files that changed from the base of the PR and between ee43699 and 1d1e24d.

📒 Files selected for processing (1)

• lib/tools/patching.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/tools/patching.py`:
- Around line 447-455: The indentation in the if-block checking
os.environ.get("GITHUB_ACTIONS") is mixed (tabs/spaces) and must be normalized
(use consistent spaces), and the logic around console_width needs adjusting: in
the block where you read COLUMNS (via os.environ.get("COLUMNS", "160")) parse it
inside a try/except to set console_width = int(...), fall back to 160 on
ValueError, then perform console_width -= 12 outside the try/except but still
inside the if that checks GITHUB_ACTIONS; reference the symbols console_width,
os.environ.get("COLUMNS", ...), and the GITHUB_ACTIONS check to locate and fix
the code.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c0d4f627-fe34-4a8d-a677-7f97bef51df6

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1e24d and 9600e5b.

📒 Files selected for processing (1)

• lib/tools/patching.py

[rpardini]

This reads as "force COLUMNS to at least 120" and "tolerate invalid COLUMNS values" (which will later be choked upon by unrelated utils).

What is the underlying cause here?

[scollinson]

The underlying cause seems rather obvious from the backtrace, which is that somewhere in my buikd environment the variable COLUMNS is an empty string, causing an exception. I've not set this myself, and it also seems to happen in CI and my local host. The fix was pretty simple, but the AI review noted that should the COLUMNS value be less than 12 then the column width would be negative, which the current implementation does not account for either. If you can think of another sane default then I can replace 120. I've now spent more time writing this reponse than I did making the change, which seems like an obvious improvement and has meant my build pipeline now succeeds.