actions / upload-release-asset

An Action to upload a release asset via the GitHub Release API
MIT License
688 stars 189 forks source link

ci: forced npm to colorize its output #19

Closed claudeleveille closed 4 years ago

claudeleveille commented 4 years ago

This is particularly helpful at small hours of the morning when your eyes are barely open.

eine commented 4 years ago

Ref actions/starter-workflows#114

claudeleveille commented 4 years ago

Ref actions/starter-workflows#114

It's my experience that programs run in CI systems need a little bit of a nudge to output colors (whether that means passing a parameter or running them in a pty). I do agree that GitHub might be able to make this easier on users by providing a different terminal setup for our scripts to run in, but the workarounds I've implemented over the last couple of years aren't that hard to put in place most of the time (like this single env var for npm). I've never had to hack nearly as hard as you say (running everything in nested containers) to get mere colors out of my programs. I also have some scripts that use the invoke Python package to run shell scripts in ptys and that works fine. I'd encourage you to take a look at that if you have gnarly scripts/programs that are fussy about the "specific serial number of the glass teletype they're running in".

eine commented 4 years ago

I've never had to hack nearly as hard as you say (running everything in nested containers) to get mere colors out of my programs.

Running tasks inside docker containers is not a hack to get colored output, but decided for other reasons. The purpose of mentioning it in actions/starter-workflows#114 was to argue that it is possible to have a tty, even if there is neither a keyboard nor an interactive session. Moreover, given the fact that containers with option -i are a currently valid workaround, I ask rhetorically whether it is worth to use ubuntu-latest instead of ubuntu:latest for GNU/Linux jobs.

the workarounds I've implemented over the last couple of years aren't that hard to put in place most of the time (like this single env var for npm)

Well, I guess that it depends on how many different tools you need to fix and how well-suited they are. Overall, I agree, it is not much effort. Nevertheless, the point is that it is some effort that needs to be done for GHA only, since tools already work as expected locally, in docker containers and in other CI services. After introducing fixes for GHA, additional modifications might be required to switch them off selectively. For example, in https://github.com/1138-4EB/vunit/commit/441bcb04f670c4a1c98670329d9977a7f3ae38a0, environment variables added to the workflow are harmless, but modifications to tox.ini are not.

I also have some scripts that use the invoke Python package to run shell scripts in ptys and that works fine. I'd encourage you to take a look at that if you have gnarly scripts/programs that are fussy about the "specific serial number of the glass teletype they're running in".

Thanks! I will have a look!

Note that I don't have any problem with this issue at all. I added the reference just to gather cases where having a pty/tty can be useful.

claudeleveille commented 4 years ago

Note that I don't have any problem with this issue at all. I added the reference just to gather cases where having a pty/tty can be useful.

Thanks for that link to your issue, BTW. You and and @ioquatix made a very interesting and solid case for GHA changing their design to provide TTYs. I agree that a more Travis-esque setup would make sense here because this status quo only leads to users having to implement more complex workarounds to solve this problem than GitHub can.

I do wonder if our voices have been heard by the right people though... The parts of GHA we're talking about are probably proprietary (at least in part).

ioquatix commented 4 years ago

We can all email support@github.com.

eine commented 4 years ago

I do wonder if our voices have been heard by the right people though... The parts of GHA we're talking about are probably proprietary (at least in part).

We can all email support@github.com.

I did already contact them for a similar issue (actions/starter-workflows#96) and I got the same reply twice several weeks later. The suggestion being https://github.com/actions/starter-workflows/issues/96#issuecomment-527995978, which is (and was) already replied in the issue. As commented in https://github.com/actions/toolkit/issues/214#issuecomment-553711592, I think that engineers are doing their best, but we have to deal with this annoying lack of better planification. Even most sensible requests (say actions/upload-artifact#7, actions/upload-artifact#3) are almost unattended. Furthermore, unlike other requests, AFAIK no employee has reported having added pty/tty support to their internal tracker. Unfortunately, if secure docker login (which is critical for 'container Actions') is still broken, I don't expect pty/tty to be fixed soon. Actually, they must be busy with other critical bugs from a product perspective: https://github.com/actions/upload-artifact/issues/5#issuecomment-559543364, https://github.community/t5/GitHub-Actions/Performance-on-Windows/m-p/39195#M3708...

ioquatix commented 4 years ago

Apparently the runner is now open source so perhaps we can make a PR directly: https://github.com/actions/runner