main: disable `colorama` on Linux and flush output #494

abitrolly · 2022-06-30T19:14:35Z

Fixes #493

abitrolly · 2022-06-30T19:15:57Z

pre-commit.ci autofix

src/build/__main__.py

abitrolly · 2022-07-01T05:47:17Z

Got another problem. colored messages are piling at the end of the output.

Need to add more commits to turn off output buffering.

src/build/__main__.py

henryiii · 2022-07-01T13:00:26Z

The problems with the output are likely pypa/cibuildwheel#1161?

abitrolly · 2022-07-01T16:25:59Z

@henryiii without output buffering everything works as expected. But I need to properly test formatting.

src/build/__main__.py

abitrolly · 2022-07-01T19:58:50Z

Everything except codecov passes. I can rebase or squash if needed.

henryiii · 2022-07-01T21:04:58Z

I’ll see if I can easily add a test for missing colorama when I’m at a computer.

edit: code looks simpler now, I think one PR for the two fixes is likely fine.

henryiii · 2022-07-02T05:23:13Z

@gaborbernat or anyone else, is there an easy way to run an "uninstall colorama" only on windows in tox? Would keep from having to list the test deps again, which is what I'm trying here to see if that fixes the coverage check.

layday · 2022-07-02T08:59:09Z

Is there a probable downside to uninstalling colorama unconditionally?

henryiii · 2022-07-02T13:44:37Z

Ahh, it's a warning not an error code? Then no.

layday · 2022-07-02T14:17:44Z

Personally I would rather print with flush and format were separate functions but I don't think it matters that much.

abitrolly · 2022-07-08T09:06:14Z

Is there anything else that needs to be done to push this? I'd like to propose some semantic changes to the build output, and the diff will conflict with this one.

henryiii · 2022-07-08T11:52:26Z

We don’t need to make the coverage pass. I’m traveling till the 27th so can only try a little bit here and there. Currently it seems like the coverage info from the path job is not being reported properly.

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

`{` are present in tracebacks leading to errors

Fix type annotaion errors

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii

Sorry that took so long to fix!

henryiii · 2022-10-18T12:27:49Z

@gaborbernat I'd like your sign-off on this, especially since I touched the tox config quite a bit. We now produce coverage info from all the runs, and use the coverage job to combine it (it was unused before).

henryiii · 2022-10-21T15:05:18Z

src/build/__main__.py

@@ -46,6 +47,10 @@ def _init_colors() -> Dict[str, str]:
 _STYLES = _init_colors()


+def _cprint(fmt: str = '', msg: str = '') -> None:
+    print(fmt.format(msg, **_STYLES), flush=True)


Nice! I just noticed the flush=True. I was going to do a follow up PR since I noticed that build's output can get mangled with the build system's output if it goes to stderr. But there's a flush here already! :)

Yes, because Python default output buffering is evil )

#494 (comment)

Absolutely no argument there!!! Dealing with this in cibuildwheel, scikit-build, etc.

abitrolly · 2022-10-21T15:41:22Z

Finally. )

@henryiii thanks. :D

abitrolly requested review from FFY00, gaborbernat, layday and henryiii as code owners June 30, 2022 19:14

henryiii reviewed Jul 1, 2022

View reviewed changes

src/build/__main__.py Show resolved Hide resolved

layday linked an issue Jul 1, 2022 that may be closed by this pull request

colorama disables colors in GitHub Actions #493

Closed

layday reviewed Jul 1, 2022

View reviewed changes

src/build/__main__.py Outdated Show resolved Hide resolved

FFY00 reviewed Jul 1, 2022

View reviewed changes

src/build/__main__.py Outdated Show resolved Hide resolved

layday reviewed Jul 1, 2022

View reviewed changes

src/build/__main__.py Outdated Show resolved Hide resolved

henryiii changed the title ~~Disable colorama on Linux (fixes #493)~~ Disable colorama on Linux Jul 2, 2022

henryiii force-pushed the patch-2 branch from ecb5c6d to b25e047 Compare July 2, 2022 13:45

layday self-requested a review July 2, 2022 14:18

layday approved these changes Jul 2, 2022

View reviewed changes

abitrolly mentioned this pull request Jul 8, 2022

Tracking DB schema changes yakshaveinc/tasks#80

Open

10 tasks

henryiii self-assigned this Aug 1, 2022

abitrolly added 3 commits October 17, 2022 16:45

Disable colorama on Linux (fixes pypa#493)

6231afc

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

colorama is a required dependency on Windows

3c2e668

Prevent output buffering

f75a29c

abitrolly and others added 8 commits October 17, 2022 16:46

Fix cprint()

753e8ff

Make cprint() private

c2ad8c1

Get back ModuleNotFoundError

97dd2aa

cprint: don't try to format message

31bc4f1

`{` are present in tracebacks leading to errors

Add type annotation

b589716

Fix type annotaion errors

tests: remove colorama on path test

ec960b2

tests: move path test up to normal tests

c6dc62f

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

tox: include coverage on path job

4fa3086

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii force-pushed the patch-2 branch 6 times, most recently from 95056ba to 7c66f64 Compare October 18, 2022 03:55

ci: combine coverage from the run

a1ccc27

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii force-pushed the patch-2 branch from 7c66f64 to a1ccc27 Compare October 18, 2022 04:47

henryiii approved these changes Oct 18, 2022

View reviewed changes

henryiii reviewed Oct 21, 2022

View reviewed changes

henryiii changed the title ~~Disable colorama on Linux~~ main: disable colorama on Linux and flush output Oct 21, 2022

henryiii merged commit b1acadc into pypa:main Oct 21, 2022

abitrolly deleted the patch-2 branch October 21, 2022 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

main: disable `colorama` on Linux and flush output #494

main: disable `colorama` on Linux and flush output #494

abitrolly commented Jun 30, 2022 •

edited by henryiii

abitrolly commented Jun 30, 2022

abitrolly commented Jul 1, 2022

henryiii commented Jul 1, 2022

abitrolly commented Jul 1, 2022

abitrolly commented Jul 1, 2022

henryiii commented Jul 1, 2022 •

edited

henryiii commented Jul 2, 2022

layday commented Jul 2, 2022

henryiii commented Jul 2, 2022

layday commented Jul 2, 2022 •

edited

abitrolly commented Jul 8, 2022

henryiii commented Jul 8, 2022

henryiii left a comment

henryiii commented Oct 18, 2022

henryiii Oct 21, 2022

abitrolly Oct 21, 2022

henryiii Oct 21, 2022

abitrolly commented Oct 21, 2022

main: disable colorama on Linux and flush output #494

main: disable colorama on Linux and flush output #494

Conversation

abitrolly commented Jun 30, 2022 • edited by henryiii

abitrolly commented Jun 30, 2022

abitrolly commented Jul 1, 2022

henryiii commented Jul 1, 2022

abitrolly commented Jul 1, 2022

abitrolly commented Jul 1, 2022

henryiii commented Jul 1, 2022 • edited

henryiii commented Jul 2, 2022

layday commented Jul 2, 2022

henryiii commented Jul 2, 2022

layday commented Jul 2, 2022 • edited

abitrolly commented Jul 8, 2022

henryiii commented Jul 8, 2022

henryiii left a comment

Choose a reason for hiding this comment

henryiii commented Oct 18, 2022

henryiii Oct 21, 2022

Choose a reason for hiding this comment

abitrolly Oct 21, 2022

Choose a reason for hiding this comment

henryiii Oct 21, 2022

Choose a reason for hiding this comment

abitrolly commented Oct 21, 2022

main: disable `colorama` on Linux and flush output #494

main: disable `colorama` on Linux and flush output #494

abitrolly commented Jun 30, 2022 •

edited by henryiii

henryiii commented Jul 1, 2022 •

edited

layday commented Jul 2, 2022 •

edited