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

False-negative on GitHub Actions #106

Closed
ai opened this issue Dec 31, 2019 · 22 comments
Closed

False-negative on GitHub Actions #106

ai opened this issue Dec 31, 2019 · 22 comments

Comments

@ai
Copy link

ai commented Dec 31, 2019

GitHub Actions CI supports color output. But chalk use black/white output there.

Seems like we have a false-negative detection at support-color.

I am fixing the problem with FORCE_COLOR=2 right now.

@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

Hey @ai :) Do you have an example I could take a look at? Chalk behaves differently based on how it's used (e.g. with pipes). Can't confirm if this is a bug or not without seeing if there's another issue.

@ai
Copy link
Author

ai commented Dec 31, 2019

Sure, here is an example with FORCE_COLOR=2.

I have Node.js script with simple code like:

#!/usr/bin/env node

import chalk from 'chalk'

process.stdout.write(chalk.red('Test'))

When I run this script in CI with:

      - name: Deploy files
        run: ./script.js

I have no colors and I start to have colors only with:

      - name: Deploy files
        run: ./script.js
        env:
          FORCE_COLOR: 2

@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

Which version of chalk? chalk@3 uses supports-color@7.1.0 which includes #102, so this should be working.

Can you verify you're at chalk >= 3.0?

@ai
Copy link
Author

ai commented Dec 31, 2019

"chalk": "^3.0.0",

I checked that I have GITHUB_ACTIONS env.

Maybe some other check disable it?

@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

Very strange. Could you see if process.stdout.isTTY is truthey?

@ai
Copy link
Author

ai commented Dec 31, 2019

@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

That's why. For whatever reason, Node doesn't seem to think it's outputting to a TTY. This is probably on Github not setting up a proper pseudo terminal environment for actions to run in.

Unfortunately, there's no great way to properly solve this without Github's help. The environment checks have to come after the TTY checks, otherwise we'd break people piping output to other programs (which would get polluted with escape sequences).

Unless we think Github can fix this with PTY support, unfortunately this is a nofix - FORCE_COLOR=1/=2 will have to be used instead.

Sorry about that :/ Wish I had a better answer.

If a member of the Github staff wants to answer, I'll happily re-open this.

@Qix- Qix- closed this as completed Dec 31, 2019
@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

Took the liberty of contacting Github about it. If they respond positively, I'll open this back up and track it ^^

Thanks for bringing it to our attention!

@ai
Copy link
Author

ai commented Dec 31, 2019

Seems like we should call @chrispat here

@ai
Copy link
Author

ai commented Dec 31, 2019

I also created an issue (hope I used right place for it) actions/runner-images#218

Thanks for finding the actual reason.

@Qix- Qix- reopened this Dec 31, 2019
@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

Re-opening for visibility, then :)

@ai
Copy link
Author

ai commented Dec 31, 2019

Can we just force colors on GITHUB_ACTIONS even without isTTY since we know that GH supports colors?

@Qix-
Copy link
Member

Qix- commented Dec 31, 2019

You can, sure. But as a general rule, no. It would break anyone using pipes.

@ai
Copy link
Author

ai commented Dec 31, 2019

Oh, I forgot about pipes

regevbr added a commit to PruvoNet/node-upgrade-checker that referenced this issue Mar 21, 2020
@SimenB
Copy link

SimenB commented Apr 23, 2020

Solved with https://github.blog/changelog/2020-04-15-github-actions-sets-the-ci-environment-variable-to-true/? Maybe not if it's tty detection that's off 😀

@mshima
Copy link

mshima commented May 21, 2020

This should be fixed in 651576a

A new release should be great.

@viceice
Copy link

viceice commented Oct 22, 2020

I think this can be closed, as it's fixed with v7.2

@Qix-
Copy link
Member

Qix- commented Oct 22, 2020

Yep, thanks for the reminder. Let me know if this isn't solved by upgrading.

@rethab
Copy link

rethab commented Feb 16, 2022

@Qix- from what I can tell, this not solved yet:

In the code, this check:

	if (haveStream && !streamIsTTY && forceColor === undefined) {
		return 0;
	}

comes before this part:

	if ('CI' in env) {
      // etc...

When run on GitHub Actions, the first condition already returns 0. This is because of this open issue in the runner: actions/runner#241

As a result of this, chalk does use color in my action. I'd be happy to work out a reproducing example if anyone's interested, but it looks kind of obvious to me given the evidence above.

seb-cr added a commit to seb-cr/ts-package-template that referenced this issue Apr 24, 2023
Work around an ongoing issue with detecting colour support in GitHub
Actions. See chalk/supports-color#106
peternewman added a commit to peternewman/iD that referenced this issue Jun 6, 2023
egasimus added a commit to hackbg/fadroma that referenced this issue Jul 5, 2023
@mrmckeb
Copy link

mrmckeb commented Feb 28, 2024

Does anyone have this working? We're also seeing level 0 returned due to isTTY being undefined.

As @rethab said, this seems to be an issue on GitHub's side - but given it has been so long, perhaps a workaround can be implemented here?

@peternewman
Copy link

As mentioned at the top @mrmckeb FORCE_COLOR=2 works. See for example my PR here:
https://github.com/openstreetmap/iD/pull/9692/files

Or any of the others referencing this issue and linked to it.

@mrmckeb
Copy link

mrmckeb commented Feb 29, 2024

That's a great solution, thanks. I was wondering if anyone had a solution that end-users wouldn't need to implement though.

For now, I've implemented this:

if (env.GITHUB_ACTIONS) {
  const supportsColor = createSupportsColor({ isTTY: true });
  if (supportsColor) {
    chalk.level = supportsColor.level;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants