Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color output from CI jobs #1090

Closed
ericfrederich opened this issue Aug 13, 2018 · 10 comments
Closed

Color output from CI jobs #1090

ericfrederich opened this issue Aug 13, 2018 · 10 comments

Comments

@ericfrederich
Copy link
Contributor

GitLab supports color escape sequences when viewing a job's output. Perhaps the same is true of GitHub, Jenkins, or other CI tools.

I'm guessing that the server running these commands is piping stdout and stderr to other processes or files which later get shipped over the wire. Because of this it is likely failing some isatty() call in _compat.py.

While it isn't a tty, it will eventually be displayed by something capable of handling colors. My current workaround is to detect if it's being ran as a GitLab CI job by the presence of an environment variable and explicitly setting ctx.color = True

I think this is something that could/should be handled internally by click so that I don't have to add this workaround to every utility I write which may end up being used within some CI. Does anyone else agree?

workaround

#!/usr/bin/env python

import os

import click


@click.command()
@click.option('--name', '-n', default='Anonymous', envvar='LOGNAME')
@click.pass_context
def cli_main(ctx, name):
    if 'GITLAB_USER_LOGIN' in os.environ:
        ctx.color = True
    click.secho(f'Hello {name}', fg='magenta', bold=True)


if __name__ == '__main__':
    import sys
    cli_main(sys.argv[1:])
@davidism
Copy link
Member

I don't want to hard code things like this, it ends up being an arbitrary list that we have to maintain. An environment variable, such as CLICK_COLOR=1 would be better for this.

@ericfrederich
Copy link
Contributor Author

@davidism good point.

I agree with you, making click have knowledge of various environments which may run a click program is probably the wrong way to go.
Likewise though, having a variable such as CLICK_COLOR would then shift the responsibility to those environments to have knowledge of click and have to maintain their own arbitrary list.

Is there a generic solution adopted by other programs, like ANSI_COLOR=1 or something?

Perhaps implemented as a default click option (like --help is default) could be done like --raw-control-chars set as a flag with envvar='ANSI_COLOR'
The effect of this would be to just set ctx.color=True

@davidism
Copy link
Member

GitLab, Travis, etc. don't need to know about Click. You would add an env var for the test job.

@ericfrederich
Copy link
Contributor Author

I would think that it would be the responsibility of GitLab, Travis, etc to know that they are able to properly handle color escape sequences. Unfortunately there doesn't seem to be a common environment variable to express this.

For example, a non-empty DISPLAY variable tells programs that graphics are available. There is no generic variable to tell programs that ANSI colors should be used.

@flying-sheep
Copy link
Contributor

Transcluding my comments from #558:

the chalk JS package has those 13 000 depending packages. this (transitively) includes nearly all node.js command line applications.

there’s no real standard for this. just check this mess

i think we have FORCE_COLOR, CLICOLOR, CLICOLOR_FORCE, LS_COLORS, GREP_COLORS, and various --color options, e.g. --color=always|auto|off in GNU tools. CLICOLOR[_FORCE] seems to be in cmake and OSX tools, as well as the most clear and intuitive way, but none of the bug reports for other libs had any impact. i’m sad for @jhasse for putting in so much work in PRs being interested, despite it being the best idea and already integrated in some tools.

convenient and expected would IMHO be to have a method that automatically adds a GNU-like --color option, as well as support for the whole env variable mess like supports-color does.

@flying-sheep
Copy link
Contributor

for semantics:

  • FORCE_COLOR ≠ 0, CLICOLOR_FORCE ≠ 0, --color=always force it to be on
  • CLICOLOR ≠ 0, --color, --color=auto means autodetection
  • CLICOLOR=0, FORCE_COLOR=0, --color=off force it to be off
  • CLICOLOR_FORCE=0 just means no forcing, just like it being unset

@davidism
Copy link
Member

davidism commented May 8, 2019

I feel like this is already easy enough to implement if a project wants it.

if __name__ == "__main__":
    color = os.environ.get("CLICK_COLOR")
    if color is not None: color = color in {"1", "true"}
    cli(color=color)

@flying-sheep
Copy link
Contributor

CLICK_COLOR is too specific for my taste already. And if we let people do it themselves too (in incompatible ways), it becomes entirely useless. We should instead support one (or more) of the other variables I named.

@davidism
Copy link
Member

davidism commented May 8, 2019

The point is it's now up to you exactly how you want to implement it. There's already a way to control color when main executes, as shown.

@eplodn
Copy link

eplodn commented Feb 20, 2020

I'm running aws-sam-cli that happens to use click to format its output, in Jenkins.

My Jenkins is perfectly capable of displaying colors, but click (correctly) discovers it's not running with tty, so no colors would be printed.

My options are: a) forking aws-sam-cli and implementing the fix as above, while maintaining my fork, or b) respectfully ask the click maintainers to reconsider respecting an environment variable e.g. as per @flying-sheep 's proposal. It would still be up to me to run e.g. CLICOLOR_FORCE=1 aws-sam-cli ... in Jenkins.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants