Mailing List Archive

[PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR
Update epytest to respect the modern NO_COLOR variable rather than
Portage's old NOCOLOR. Adjust it to correctly check whether it is set
at all rather than to a specific value, to match the behavior of pytest
itself.

Signed-off-by: Micha? Górny <mgorny@gentoo.org>
---
eclass/python-utils-r1.eclass | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 394f64a5d139..da9cb820840f 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1336,15 +1336,8 @@ epytest() {
_python_check_EPYTHON
_python_check_occluded_packages

- local color
- case ${NOCOLOR} in
- true|yes)
- color=no
- ;;
- *)
- color=yes
- ;;
- esac
+ local color=yes
+ [[ ${NO_COLOR} ]] && color=no

local args=(
# verbose progress reporting and tracebacks
--
2.43.0
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
On 12/2/23 7:44 AM, Micha? Górny wrote:
> Update epytest to respect the modern NO_COLOR variable rather than
> Portage's old NOCOLOR. Adjust it to correctly check whether it is set
> at all rather than to a specific value, to match the behavior of pytest
> itself.
>
> Signed-off-by: Micha? Górny <mgorny@gentoo.org>
> ---
> eclass/python-utils-r1.eclass | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index 394f64a5d139..da9cb820840f 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1336,15 +1336,8 @@ epytest() {
> _python_check_EPYTHON
> _python_check_occluded_packages
>
> - local color
> - case ${NOCOLOR} in
> - true|yes)
> - color=no
> - ;;
> - *)
> - color=yes
> - ;;
> - esac
> + local color=yes
> + [[ ${NO_COLOR} ]] && color=no


[[ -v NO_COLOR ]]

This is processed by the pytest code:

```
if "NO_COLOR" in os.environ:
return False
```


>
> local args=(
> # verbose progress reporting and tracebacks


--
Eli Schwartz
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
On Mon, 2023-12-11 at 13:57 -0500, Eli Schwartz wrote:
> On 12/2/23 7:44 AM, Micha? Górny wrote:
> > Update epytest to respect the modern NO_COLOR variable rather than
> > Portage's old NOCOLOR. Adjust it to correctly check whether it is set
> > at all rather than to a specific value, to match the behavior of pytest
> > itself.
> >
> > Signed-off-by: Micha? Górny <mgorny@gentoo.org>
> > ---
> > eclass/python-utils-r1.eclass | 11 ++---------
> > 1 file changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index 394f64a5d139..da9cb820840f 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1336,15 +1336,8 @@ epytest() {
> > _python_check_EPYTHON
> > _python_check_occluded_packages
> >
> > - local color
> > - case ${NOCOLOR} in
> > - true|yes)
> > - color=no
> > - ;;
> > - *)
> > - color=yes
> > - ;;
> > - esac
> > + local color=yes
> > + [[ ${NO_COLOR} ]] && color=no
>
>
> [[ -v NO_COLOR ]]
>
> This is processed by the pytest code:
>
> ```
> if "NO_COLOR" in os.environ:
> return False
> ```
>

That looks wrong. Per [1]:

> […] NO_COLOR environment variable that, when present and not an empty
string (regardless of its value), prevents the addition of ANSI color.

So hey, I'm actually fixing pytest ;-).

[1] https://no-color.org/

--
Best regards,
Micha? Górny
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:

>> + local color=yes
>> + [[ ${NO_COLOR} ]] && color=no

> [[ -v NO_COLOR ]]

No, this would give the wrong result if NO_COLOR is set to an empty
value. [[ ${NO_COLOR} ]] or [[ -n ${NO_COLOR} ]] is the correct test:

"Command-line software which adds ANSI color to its output by default
should check for a NO_COLOR environment variable that, when present
and not an empty string (regardless of its value), prevents the
addition of ANSI color." -- https://no-color.org/
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
On 12/11/23 2:30 PM, Ulrich Mueller wrote:
>>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:
>
>>> + local color=yes
>>> + [[ ${NO_COLOR} ]] && color=no
>
>> [[ -v NO_COLOR ]]
>
> No, this would give the wrong result if NO_COLOR is set to an empty
> value. [[ ${NO_COLOR} ]] or [[ -n ${NO_COLOR} ]] is the correct test:
>
> "Command-line software which adds ANSI color to its output by default
> should check for a NO_COLOR environment variable that, when present
> and not an empty string (regardless of its value), prevents the
> addition of ANSI color." -- https://no-color.org/


Again, not according to pytest itself. ;)

Given the commit message says:

"""
Adjust it to correctly check whether it is set at all rather than to a
specific value, to match the behavior of pytest itself.
"""


--
Eli Schwartz
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
On 12/11/23 2:27 PM, Micha? Górny wrote:
> That looks wrong. Per [1]:
>
>> […] NO_COLOR environment variable that, when present and not an empty
> string (regardless of its value), prevents the addition of ANSI color.
>
> So hey, I'm actually fixing pytest ;-).
>
> [1] https://no-color.org/


When you say you are fixing pytest, do you mean you are submitting a PR
to pytest to make its behavior align with the (tbh reasonable) behavior
you expected and want?


--
Eli Schwartz
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:

>> "Command-line software which adds ANSI color to its output by default
>> should check for a NO_COLOR environment variable that, when present
>> and not an empty string (regardless of its value), prevents the
>> addition of ANSI color." -- https://no-color.org/

> Again, not according to pytest itself. ;)

> Given the commit message says:

> """
> Adjust it to correctly check whether it is set at all rather than to a
> specific value, to match the behavior of pytest itself.
> """

The standard is defined by sno-color.org. If pytest does something
different then it is a bug.

Ulrich
Re: [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR [ In reply to ]
On 12/11/23 21:38, Ulrich Mueller wrote:

> The standard is defined by sno-color.org.

http://no-color.org

--
Toralf
PGP 23217DA7 9B888F45