godot icon indicating copy to clipboard operation
godot copied to clipboard

SCons: Colorize warnings/errors during generation

Open Repiteo opened this issue 1 year ago • 10 comments

This change adds the ability to substitute print with methods.print_warning or methods.print_error, prepending the messages with WARNING: /ERROR: and colorizing them as yellow/red respectively. The messages will also be routed to stderr instead of stdout, allowing users to silence stdout without missing on any crucial information if a build fails or otherwise stops. non-atty environments won't benefit from colorization, but will still enjoy prefix & stderr benefits. 24-04-26 13-22-39 Code

EDIT: Supersedes #88074

Repiteo avatar Apr 26 '24 18:04 Repiteo

Does this supersede https://github.com/godotengine/godot/pull/88074? It includes some not changes not made in this PR.

Calinou avatar Apr 26 '24 22:04 Calinou

I don't think it supercedes that PR entirety; though, it probably would need a different title if left as-is

Repiteo avatar Apr 26 '24 22:04 Repiteo

Oops, I forgot about the older PR, I'm sorry @Calinou.

This is a bit of an unfortunate situation. How should we handle it?

They're both great PRs, with the same main change but different details.

Riteo avatar Apr 26 '24 22:04 Riteo

Did some cleanup work; now it supersedes #88074! The only bits I didn't bring over were print_info because it felt a little redundant & the "Enable ANSI escape code support on Windows 10 and later" section as that issue appears to have been fixed in Python 3.7 which is good enough for me.

Repiteo avatar Apr 27 '24 15:04 Repiteo

Sounds good to me. We should make sure to update the Compiling on Windows documentation to require Python 3.7 though, if we start requiring it de facto for proper display of errors/warnings on Windows.

Calinou avatar Apr 28 '24 16:04 Calinou

Ehhh, if we gotta go through that trouble I'll just add that bit of code after all; I'll throw in a note that it should be removed when the minimum version is >=3.7

Repiteo avatar Apr 28 '24 16:04 Repiteo

Hmm, I might've misread the original post; I don't think it's necessarily limited to 3.7 after all. And yet, I've never had any issues with displaying colored text & I use Windows 11, so I honestly have no clue what circumstances cause that bug to occur. Either way, no real harm done in including it.

Repiteo avatar Apr 28 '24 16:04 Repiteo

Sounds good to me. We should make sure to update the Compiling on Windows documentation to require Python 3.7 though, if we start requiring it de facto for proper display of errors/warnings on Windows.

Python 3.7 reached end-of-life 10 months ago. 3.8 and later still get security fixes.

tiloc avatar Apr 28 '24 19:04 tiloc

Compilation output on CI looks strange with lots of [Initial build], which I don't see on other PRs' CI logs: https://github.com/godotengine/godot/actions/runs/8868984410/job/24349194236?pr=91220

Perhaps we should use progress=no on CI either way, as I don't think logging progress on CI is that useful.

Calinou avatar Apr 28 '24 21:04 Calinou

I double-checked the PR this supersedes, and that error was happening there too. Seems like enabling progress reporting for CI is what caused it, as reverting that fixed the issue.

Repiteo avatar Apr 28 '24 21:04 Repiteo

Thanks!

akien-mga avatar Apr 29 '24 08:04 akien-mga

This MR broke use_mingw flag on windows:

PS D:\Repo\godot> scons use_mingw=yes
scons: Reading SConscript files ...
Automatically detected platform: windows
Auto-detected 24 CPU cores available for build parallelism. Using 23 cores by default. You can override it with the -j argument.
Using MinGW, arch x86_64
Building for platform "windows", architecture "x86_64", target "editor".
AttributeError: 'str' object has no attribute 'insert':
  File "D:\Repo\godot\SConstruct", line 971:
    methods.no_verbose(env)
  File "D:\Repo\godot\methods.py", line 671:
    env.Append(SHLINKCOMSTR=link_shared_library_message)
  File "C:\Users\<username>\AppData\Roaming\Python\Python312\site-packages\SCons\Environment.py", line 1458:
    val.insert(0, orig)

Note: Compilation proceeds when verbose flag is given.

Anutrix avatar Apr 29 '24 16:04 Anutrix