Re: [gentoo-portage-dev] [PATCH] Stop implicitly manipulating `NO_COLOR`/`NOCOLOR`

2023-11-26 Thread Michał Górny
On Sun, 2023-11-26 at 18:33 +0100, Ulrich Mueller wrote:
> > > > > > On Sun, 26 Nov 2023, Michał Górny wrote:
> 
> > 3. Forcing `NO_COLOR=1` turns out to cause random test failures,
> >    and while fixing them is commendable, it is a pain for arch testing
> >    and it is currently blocking stabilization requests.
> 
> I'd argue that test suites that fail because of NO_COLOR are broken.
> IIUC, these failures will show up for users who set NO_COLOR=1 in their
> make.conf or in the environment?

Sure, they are broken.  However, as I said I'd rather fix them in my
spare time than have to fix them to unblock a security stablereq because
implicit Portage logic blocks all AT work on a minor issue that's non-
trivial to fix.

> 
> > With the new approach, the color output in programs is consistent
> > between using ``--jobs`` or ``--quiet-build``, and not.  Therefore,
> > both cases generate uniform logs.  In order to obtain logs free of color
> > codes, one can either filter them via `ansifilter(1)` (as the manpages
> > already recommend) or explicitly set `NO_COLOR`.
> 
> Will this still guarantee that NO_COLOR=1 from the environment is always
> passed on to the build system?
> 
> Otherwise, it would break ebuild-mode in some configurations.
> 

Yes, I've tested it and it worked.  Both for passing from external env
and via make.conf.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-portage-dev] [PATCH] Stop implicitly manipulating `NO_COLOR`/`NOCOLOR`

2023-11-26 Thread Ulrich Mueller
> On Sun, 26 Nov 2023, Michał Górny wrote:

> 3. Forcing `NO_COLOR=1` turns out to cause random test failures,
>and while fixing them is commendable, it is a pain for arch testing
>and it is currently blocking stabilization requests.

I'd argue that test suites that fail because of NO_COLOR are broken.
IIUC, these failures will show up for users who set NO_COLOR=1 in their
make.conf or in the environment?

> With the new approach, the color output in programs is consistent
> between using ``--jobs`` or ``--quiet-build``, and not.  Therefore,
> both cases generate uniform logs.  In order to obtain logs free of color
> codes, one can either filter them via `ansifilter(1)` (as the manpages
> already recommend) or explicitly set `NO_COLOR`.

Will this still guarantee that NO_COLOR=1 from the environment is always
passed on to the build system?

Otherwise, it would break ebuild-mode in some configurations.

Ulrich


signature.asc
Description: PGP signature


[gentoo-portage-dev] [PATCH] Stop implicitly manipulating `NO_COLOR`/`NOCOLOR`

2023-11-26 Thread Michał Górny
Stop implicitly forcing `NO_COLOR` and `NOCOLOR` in ebuild environment.
This is undesired for several reasons:

1. It makes it impossible to control color for emerge output
   independently of command output, e.g. when one goes to a pty while
   the other goes to logs.

2. It makes it impossible to get color output in logs when running
   `emerge --jobs ...`.

3. Forcing `NO_COLOR=1` turns out to cause random test failures,
   and while fixing them is commendable, it is a pain for arch testing
   and it is currently blocking stabilization requests.

With the new approach, the color output in programs is consistent
between using ``--jobs`` or ``--quiet-build``, and not.  Therefore,
both cases generate uniform logs.  In order to obtain logs free of color
codes, one can either filter them via `ansifilter(1)` (as the manpages
already recommend) or explicitly set `NO_COLOR`.

Furthermore, one can combine color-free build output (for clean logs)
with colorful emerge output by using:

NO_COLOR=true emerge --color y ...

Bug: https://bugs.gentoo.org/918515
---
 bin/ebuild   | 4 
 lib/_emerge/AbstractEbuildProcess.py | 6 --
 lib/_emerge/actions.py   | 5 -
 lib/portage/tests/resolver/ResolverPlayground.py | 3 ---
 4 files changed, 18 deletions(-)

diff --git a/bin/ebuild b/bin/ebuild
index cbefb5816..69db474c0 100755
--- a/bin/ebuild
+++ b/bin/ebuild
@@ -132,10 +132,6 @@ def main():
 or not sys.stdout.isatty()
 ):
 portage.output.nocolor()
-portage.settings.unlock()
-portage.settings["NO_COLOR"] = "true"
-portage.settings.backup_changes("NO_COLOR")
-portage.settings.lock()
 
 apply_priorities(portage.settings)
 
diff --git a/lib/_emerge/AbstractEbuildProcess.py 
b/lib/_emerge/AbstractEbuildProcess.py
index 96d91b5da..3a8a18b5f 100644
--- a/lib/_emerge/AbstractEbuildProcess.py
+++ b/lib/_emerge/AbstractEbuildProcess.py
@@ -80,12 +80,6 @@ class AbstractEbuildProcess(SpawnProcess):
 self._async_wait()
 return
 
-if self.background:
-# Automatically prevent color codes from showing up in logs,
-# since we're not displaying to a terminal anyway.
-self.settings["NOCOLOR"] = "true"
-self.settings["NO_COLOR"] = "true"
-
 start_ipc_daemon = False
 if self._enable_ipc_daemon:
 self.settings.pop("PORTAGE_EBUILD_EXIT_FILE", None)
diff --git a/lib/_emerge/actions.py b/lib/_emerge/actions.py
index 37264114e..dbd9707a8 100644
--- a/lib/_emerge/actions.py
+++ b/lib/_emerge/actions.py
@@ -2749,15 +2749,10 @@ def adjust_config(myopts, settings):
 if "--color" in myopts:
 if "y" == myopts["--color"]:
 portage.output.havecolor = 1
-settings["NO_COLOR"] = ""
 else:
 portage.output.havecolor = 0
-settings["NO_COLOR"] = "true"
-settings.backup_changes("NO_COLOR")
 elif settings.get("TERM") == "dumb" or not sys.stdout.isatty():
 portage.output.havecolor = 0
-settings["NO_COLOR"] = "true"
-settings.backup_changes("NO_COLOR")
 
 if "--pkg-format" in myopts:
 settings["PORTAGE_BINPKG_FORMAT"] = myopts["--pkg-format"]
diff --git a/lib/portage/tests/resolver/ResolverPlayground.py 
b/lib/portage/tests/resolver/ResolverPlayground.py
index 475c4aaac..115800a60 100644
--- a/lib/portage/tests/resolver/ResolverPlayground.py
+++ b/lib/portage/tests/resolver/ResolverPlayground.py
@@ -591,9 +591,6 @@ class ResolverPlayground:
 "PORTAGE_TMPDIR": os.path.join(self.eroot, "var/tmp"),
 }
 
-if portage.util.no_color(os.environ):
-make_conf["NO_COLOR"] = os.environ["NO_COLOR"]
-
 # Pass along PORTAGE_USERNAME and PORTAGE_GRPNAME since they
 # need to be inherited by ebuild subprocesses.
 if "PORTAGE_USERNAME" in os.environ:
-- 
2.43.0