epics-base / ci-scripts

Continuous Integration Scripts for EPICS Modules
Other
8 stars 18 forks source link

Add color output for gcc errors and warnings #74

Closed dirk-zimoch closed 1 year ago

dirk-zimoch commented 1 year ago

At least since gcc version 4.8, the compiler has the flag -fdiagnostics-color which enables highlighting of warnings and errors. This is useful for the gcc output in the github action as well because it makes warnings stand out.

The compiler would do so automatically when running in a terminal, but not in a github action, probably because its output is not a tty.

Versions 4.4 and earlier do not know this flag and fails if it is used, thus the usage needs to be version dependent.

One may consider to tweak the color settings for better readability. The default color setting is equivalent to:

GCC_COLORS=error=01;31:warning=01;35:note=01;36:range1=32:range2=34:locus=01:\
                   quote=01:fixit-insert=32:fixit-delete=31:\
                   diff-filename=01:diff-hunk=32:diff-delete=31:diff-insert=32:\
                   type-diff=01;32
mdavidsaver commented 1 year ago

Versions 4.4 and earlier do not know this flag and fails if it is used

One possible way to add conditional flags is via. configure/toolchain.c, which can test GCC version, and append to eg. USR_CFLAGS.

ralphlange commented 1 year ago

Isn't this issue fixed with #79?

dirk-zimoch commented 1 year ago

It would be if the current .ci module version was used in base. But base 7.0 still uses commit 1e0e326, as far as I can see.

ralphlange commented 1 year ago

??! This issue and Pull Request #79 are in ci-scripts, not in base.

dirk-zimoch commented 1 year ago

Maybe I misunderstood what you meant. This issue is fixed by #79. I close it. The original problem that base builds do not show color is not fixed because this fix is not yet in use by base. This is a problem with submodules. One needs to modify the submodule and then additionally change the repo that uses it. Is the solution to always create two issues and two pull requests, one each for the submodule and the repo using it?

ralphlange commented 1 year ago

Is the solution to always create two issues and two pull requests, one each for the submodule and the repo using it?

At this point, ci-scripts is used by >20 other modules. We shouldn't track problems of ci-script clients here.

dirk-zimoch commented 1 year ago

My only concern was base. Should I create an issue and PR for base to get this included?

ralphlange commented 1 year ago

If it's urgent enough to get it done immediately, sure. Otherwise, it will be updated at some point and the world will be colorful.

I don't plan to make a new release of ci-scripts as long as the color support spams the build logs (#82).