github / gh-valet

Valet helps facilitate the migration of Azure DevOps, CircleCI, GitLab CI, Jenkins, and Travis CI pipelines to GitHub Actions.
MIT License
510 stars 35 forks source link

Output does not always have newline separators #79

Closed ethanis closed 2 years ago

ethanis commented 2 years ago

Description

There have been a handful of scenarios where the output from Valet didn't buffer correctly and output gets mangled. For example:

$ gh valet audit ...
...
$ ---=---=---=---=---=---=---=--|

and

$ gh valet update
WARNING! Your password will be stored unencLogin rypted in /home/codespace/.dockerSucceeded
luke-engle commented 2 years ago

Some updates here, @ethanis

I believe these are two separate problems.

The first, I can't seem to reproduce locally - only in a codespace. Interestingly, when you resize the browser window (changing the terminal size), the leftover progress bar disappears. In a codespace this happens very consistently, but I've tried many times and can't seem to reproduce the behavior in my local vscode terminal.

The second one I can produce locally if I remove the credentials store in ~/.docker/config.json, and run gh valet update. The output being mangled comes from the docker login command. When I run the docker login command directly: echo "$GH_PAT" | docker login ghcr.io --username luke-engle --password-stdin, there is consistently no output mangling.

This seems to be an issue that's coming from the gh cli and/or gh-valet. I'm also wondering if it's a similar problem to the issue where the docker update command lists every image download in a newline instead of replacing previous downloads like we see when the docker command is run by itself in a terminal

luke-engle commented 2 years ago

Do you think the second issue is related to this?

ReadStream(process.StandardOutput, output, cts.Token);
ReadStream(process.StandardError, output, cts.Token);

These run in parallel, correct?

ethanis commented 2 years ago

Do you think the second issue is related to this?

ReadStream(process.StandardOutput, output, cts.Token);
ReadStream(process.StandardError, output, cts.Token);

These run in parallel, correct?

@luke-engle yep, that means that we're reading the StdErr and StdOut streams in parallel. Maybe we need to put a lock around the bits where we write the output to the console? (or rather, use a thread safe lock like a semaphore 😄)

luke-engle commented 2 years ago

That's exactly what I was working on 😁