Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-19573

Allow color outputs from compilers under scons

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: sconsUtils
    • Labels:
      None
    • Team:
      External

      Description

      Compilers (g++, clang++) often produce color outputs (e.g., the word error is in red, and important paths are boldfaced) when run on the command-line, but do not when run under scons. I've just discovered how to enable color output under scons, which I propose to implement. It merely involves propagating the TERM envvar through scons.

      I know that scons deliberately doesn't propagate envvars, but we propagate some (e.g., TEMP, EUPS_DIR); I propose to add TERM to this list. I don't believe it should affect the compilation of anything, but only the format of the outputs.

        Attachments

          Activity

          Hide
          price Paul Price added a comment -

          Not implementing here, but recording in case I want to come back to in the future: https://github.com/SCons/scons/wiki/ColorBuildMessages

          Show
          price Paul Price added a comment - Not implementing here, but recording in case I want to come back to in the future: https://github.com/SCons/scons/wiki/ColorBuildMessages
          Hide
          price Paul Price added a comment - - edited

          Example Jenkins run.
          No funny business in the log.

          Show
          price Paul Price added a comment - - edited Example Jenkins run . No funny business in the log .
          Hide
          price Paul Price added a comment -

          Robert Lupton: you've raised an objection on the GitHub PR. Would you like this to go to an RFC?

          I think this is an important feature that should have been implemented a long time ago, since having color compiler outputs makes it so much easier to identify errors and warnings hidden in the flood of compiler command-lines.

          Show
          price Paul Price added a comment - Robert Lupton : you've raised an objection on the GitHub PR. Would you like this to go to an RFC? I think this is an important feature that should have been implemented a long time ago, since having color compiler outputs makes it so much easier to identify errors and warnings hidden in the flood of compiler command-lines.
          Hide
          tjenness Tim Jenness added a comment -

          Looking at Jenkins it does indeed seem that the jenkins build system is not defining a colored TERM variable so it has no impact there (Which is good).

          Show
          tjenness Tim Jenness added a comment - Looking at Jenkins it does indeed seem that the jenkins build system is not defining a colored TERM variable so it has no impact there (Which is good).
          Hide
          price Paul Price added a comment -

          Ah, as suspected by Robert Lupton, you can do this on the command-line without the proposed fix: scons export=TERM=$TERM. However, it appears that this is a sconsUtils feature so it's not something you can put in SCONSFLAGS. I suggest that this feature is sufficiently important that it should be the default behaviour, so users shouldn't have to use the additional command-line arguments to get it.

          Show
          price Paul Price added a comment - Ah, as suspected by Robert Lupton , you can do this on the command-line without the proposed fix: scons export=TERM=$TERM . However, it appears that this is a sconsUtils feature so it's not something you can put in SCONSFLAGS . I suggest that this feature is sufficiently important that it should be the default behaviour, so users shouldn't have to use the additional command-line arguments to get it.
          Hide
          price Paul Price added a comment -

          RHL has removed his objection, writing to me:

          I thought that they were the minimal set, and we don't need `TERM` for `scons` to work. Still, if you can't set this in `SCONS*FLAGS` I guess I won't protest more

          Tim Jenness, you've signed off on this in GitHub, but I thought I should get the official permission to merge here.

          Show
          price Paul Price added a comment - RHL has removed his objection, writing to me: I thought that they were the minimal set, and we don't need `TERM` for `scons` to work. Still, if you can't set this in `SCONS*FLAGS` I guess I won't protest more Tim Jenness , you've signed off on this in GitHub, but I thought I should get the official permission to merge here.
          Hide
          price Paul Price added a comment -

          Thanks!

          Merged to master.

          Show
          price Paul Price added a comment - Thanks! Merged to master.

            People

            • Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              Tim Jenness
              Watchers:
              Jim Bosch, Kian-Tat Lim, Paul Price, Tim Jenness
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel