[gentoo-commits] proj/portage:master commit in: lib/portage/util/, lib/portage/, lib/_emerge/

[gentoo-commits] proj/portage:master commit in: lib/portage/util/, lib/portage/, lib/_emerge/

[Zac Medico]

commit:     0ff7a3b28e0ec63d68d32e01145db8962d53774d
Author:     Zac Medico <zmedico <AT> gentoo <DOT> org>
AuthorDate: Mon Feb 26 05:09:21 2024 +0000
Commit:     Zac Medico <zmedico <AT> gentoo <DOT> org>
CommitDate: Mon Feb 26 05:09:21 2024 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=0ff7a3b2

SpawnProcess: Wait for async set_term_size

Use the SpawnProcess _unregister method to handle async
set_term_size results, avoiding possible messages like
bug 925456 triggered:

    [ERROR] Task was destroyed but it is pending!

Also update _BinpkgFetcherProcess and _EbuildFetcherProcess
which inherit the _pty_ready attribute from SpawnProcess.

Fixes: f97e414ce980 ("set_term_size: Wait asynchronously if event loop is running")
Bug: https://bugs.gentoo.org/923750
Bug: https://bugs.gentoo.org/925456
Signed-off-by: Zac Medico <zmedico <AT> gentoo.org>

 lib/_emerge/BinpkgFetcher.py |  6 ++++--
 lib/_emerge/EbuildFetcher.py |  6 ++++--
 lib/_emerge/SpawnProcess.py  | 14 +++++++++++++-
 lib/portage/output.py        | 13 +++++++++----
 lib/portage/util/_pty.py     | 23 ++++++++++++++++-------
 5 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/lib/_emerge/BinpkgFetcher.py b/lib/_emerge/BinpkgFetcher.py
index a1524dc009..587e4a57a3 100644
--- a/lib/_emerge/BinpkgFetcher.py
+++ b/lib/_emerge/BinpkgFetcher.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2023 Gentoo Authors
+# Copyright 1999-2024 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from _emerge.AsynchronousLock import AsynchronousLock
@@ -233,7 +233,9 @@ class _BinpkgFetcherProcess(SpawnProcess):
         stdout_pipe = None
         if not self.background:
             stdout_pipe = fd_pipes.get(1)
-        got_pty, master_fd, slave_fd = _create_pty_or_pipe(copy_term_size=stdout_pipe)
+        self._pty_ready, master_fd, slave_fd = _create_pty_or_pipe(
+            copy_term_size=stdout_pipe
+        )
         return (master_fd, slave_fd)
 
     def sync_timestamp(self):

diff --git a/lib/_emerge/EbuildFetcher.py b/lib/_emerge/EbuildFetcher.py
index 7a45d95172..81d4b1054b 100644
--- a/lib/_emerge/EbuildFetcher.py
+++ b/lib/_emerge/EbuildFetcher.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2023 Gentoo Authors
+# Copyright 1999-2024 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import copy
@@ -394,7 +394,9 @@ class _EbuildFetcherProcess(ForkProcess):
         stdout_pipe = None
         if not self.background:
             stdout_pipe = fd_pipes.get(1)
-        got_pty, master_fd, slave_fd = _create_pty_or_pipe(copy_term_size=stdout_pipe)
+        self._pty_ready, master_fd, slave_fd = _create_pty_or_pipe(
+            copy_term_size=stdout_pipe
+        )
         return (master_fd, slave_fd)
 
     def _eerror(self, lines):

diff --git a/lib/_emerge/SpawnProcess.py b/lib/_emerge/SpawnProcess.py
index b63afae01c..9fc12c42e5 100644
--- a/lib/_emerge/SpawnProcess.py
+++ b/lib/_emerge/SpawnProcess.py
@@ -46,6 +46,7 @@ class SpawnProcess(SubProcess):
         + (
             "_main_task",
             "_main_task_cancel",
+            "_pty_ready",
             "_selinux_type",
         )
     )
@@ -193,6 +194,9 @@ class SpawnProcess(SubProcess):
         self._main_task.add_done_callback(self._main_exit)
 
     async def _main(self, build_logger, pipe_logger, loop=None):
+        if isinstance(self._pty_ready, asyncio.Future):
+            await self._pty_ready
+            self._pty_ready = None
         try:
             if pipe_logger.poll() is None:
                 await pipe_logger.async_wait()
@@ -238,7 +242,9 @@ class SpawnProcess(SubProcess):
         stdout_pipe = None
         if not self.background:
             stdout_pipe = fd_pipes.get(1)
-        got_pty, master_fd, slave_fd = _create_pty_or_pipe(copy_term_size=stdout_pipe)
+        self._pty_ready, master_fd, slave_fd = _create_pty_or_pipe(
+            copy_term_size=stdout_pipe
+        )
         return (master_fd, slave_fd)
 
     def _spawn(
@@ -258,6 +264,12 @@ class SpawnProcess(SubProcess):
         SubProcess._unregister(self)
         if self._main_task is not None:
             self._main_task.done() or self._main_task.cancel()
+        if isinstance(self._pty_ready, asyncio.Future):
+            (
+                self._pty_ready.done()
+                and (self._pty_ready.cancelled() or self._pty_ready.result() or True)
+            ) or self._pty_ready.cancel()
+            self._pty_ready = None
 
     def _cancel(self):
         if self._main_task is not None:

diff --git a/lib/portage/output.py b/lib/portage/output.py
index 7d3a6278f3..4408705c45 100644
--- a/lib/portage/output.py
+++ b/lib/portage/output.py
@@ -8,6 +8,7 @@ import itertools
 import re
 import subprocess
 import sys
+from typing import Optional
 
 import portage
 
@@ -554,10 +555,16 @@ def get_term_size(fd=None):
     return (0, 0)
 
 
-def set_term_size(lines, columns, fd):
+def set_term_size(lines: int, columns: int, fd: int) -> Optional[asyncio.Future]:
     """
     Set the number of lines and columns for the tty that is connected to fd.
     For portability, this simply calls `stty rows $lines columns $columns`.
+
+    If spawn succeeds and the event loop is running then an instance of
+    asyncio.Future is returned and the caller should wait for it in order
+    to prevent possible error messages like this:
+
+    [ERROR] Task was destroyed but it is pending!
     """
 
     cmd = ["stty", "rows", str(lines), "columns", str(columns)]
@@ -568,9 +575,7 @@ def set_term_size(lines, columns, fd):
     else:
         loop = asyncio.get_event_loop()
         if loop.is_running():
-            asyncio.ensure_future(proc.wait(), loop).add_done_callback(
-                lambda future: future.result()
-            )
+            return asyncio.ensure_future(proc.wait(), loop=loop)
         else:
             loop.run_until_complete(proc.wait())
 

diff --git a/lib/portage/util/_pty.py b/lib/portage/util/_pty.py
index 9372e556ca..9d090b711a 100644
--- a/lib/portage/util/_pty.py
+++ b/lib/portage/util/_pty.py
@@ -1,9 +1,11 @@
-# Copyright 2010-2011 Gentoo Foundation
+# Copyright 2010-2024 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
+import asyncio
 import platform
 import pty
 import termios
+from typing import Optional, Union
 
 from portage import os
 from portage.output import get_term_size, set_term_size
@@ -19,17 +21,21 @@ _disable_openpty = platform.system() in ("SunOS",)
 _fbsd_test_pty = platform.system() == "FreeBSD"
 
 
-def _create_pty_or_pipe(copy_term_size=None):
+def _create_pty_or_pipe(
+    copy_term_size: Optional[int] = None,
+) -> tuple[Union[asyncio.Future, bool], int, int]:
     """
     Try to create a pty and if then fails then create a normal
-    pipe instead.
+    pipe instead. If a Future is returned for pty_ready, then the
+    caller should wait for it (which comes from set_term_size
+    because it spawns stty).
 
     @param copy_term_size: If a tty file descriptor is given
             then the term size will be copied to the pty.
     @type copy_term_size: int
     @rtype: tuple
-    @return: A tuple of (is_pty, master_fd, slave_fd) where
-            is_pty is True if a pty was successfully allocated, and
+    @return: A tuple of (pty_ready, master_fd, slave_fd) where
+            pty_ready is asyncio.Future or True if a pty was successfully allocated, and
             False if a normal pipe was allocated.
     """
 
@@ -69,8 +75,11 @@ def _create_pty_or_pipe(copy_term_size=None):
         mode[1] &= ~termios.OPOST
         termios.tcsetattr(slave_fd, termios.TCSANOW, mode)
 
+    pty_ready = None
     if got_pty and copy_term_size is not None and os.isatty(copy_term_size):
         rows, columns = get_term_size()
-        set_term_size(rows, columns, slave_fd)
+        pty_ready = set_term_size(rows, columns, slave_fd)
 
-    return (got_pty, master_fd, slave_fd)
+    # The future only exists when got_pty is True, so we can
+    # return the future in lieu of got_pty when it exists.
+    return (got_pty if pty_ready is None else pty_ready, master_fd, slave_fd)