commit: e5e79c093c50bfaf7ec667fe0ded72efe972d1c3
Author: Brian Harring <ferringb <AT> gmail <DOT> com>
AuthorDate: Tue Jan 10 09:42:00 2023 +0000
Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
CommitDate: Tue Jan 17 20:46:03 2023 +0000
URL:
https://gitweb.gentoo.org/proj/pkgcore/pkgcore.git/commit/?id=e5e79c09
refactor(sync): hide FD passing as an internal thing.
Effectively, add '_spawn' and '_spawn_interactive' to the base class,
and expect consumers to invoke the appropriate one.
This addition doesn't fully fix the fundamental lack of an observer (or log)
based approach to getting info out of syncers, but it at least hides some of
the internals so the syncer classes can do things like knowing what the
correct encoding is for stdout, thus being able to write their own messages
to it.
TL;DR: This subsystem's interaction w/ CLI (and used as a library) needs
rewriting,
this just reduces that problem when we visit it.
Signed-off-by: Brian Harring <ferringb <AT> gmail.com>
Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org>
src/pkgcore/sync/base.py | 24 ++++++++++++++++--------
src/pkgcore/sync/http.py | 2 +-
src/pkgcore/sync/rsync.py | 11 +++++------
src/pkgcore/sync/svn.py | 10 ++++------
tests/scripts/test_pmaint.py | 7 ++++---
5 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/src/pkgcore/sync/base.py b/src/pkgcore/sync/base.py
index 2a63545e3..1114532fb 100644
--- a/src/pkgcore/sync/base.py
+++ b/src/pkgcore/sync/base.py
@@ -14,6 +14,7 @@ __all__ = (
import os
import pwd
import stat
+import typing
from importlib import import_module
from snakeoil import process
@@ -123,7 +124,7 @@ class Syncer:
except KeyError as exc:
raise MissingLocalUser(raw_uri, str(exc))
- def sync(self, verbosity=None, force=False):
+ def sync(self, verbosity: typing.Optional[int] = None, force=False):
if self.disabled:
return False
kwds = {}
@@ -131,10 +132,9 @@ class Syncer:
kwds["force"] = True
if verbosity is None:
verbosity = self.verbosity
- # output_fd is harded coded as stdout atm.
- return self._sync(verbosity, 1, **kwds)
+ return self._sync(verbosity, **kwds)
- def _sync(self, verbosity, output_fd, **kwds):
+ def _sync(self, verbosity: int, **kwds):
raise NotImplementedError(self, "_sync")
def __str__(self):
@@ -191,11 +191,19 @@ class ExternalSyncer(Syncer):
disabled = cls._disabled = os.path.exists(path)
return disabled
- def _spawn(self, command, pipes, **kwargs):
+ def _spawn(self, command, **kwargs):
+ # Note: stderr is explicitly forced to stdout since that's how it was
originally done.
+ # This can be changed w/ a discussion.
+ kwargs.setdefault("fd_pipes", {1: 1, 2: 1})
return process.spawn.spawn(
- command, fd_pipes=pipes, uid=self.uid, gid=self.gid, env=self.env,
**kwargs
+ command, uid=self.uid, gid=self.gid, env=self.env, **kwargs
)
+ def _spawn_interactive(self, command, **kwargs):
+ # Note: stderr is explicitly forced to stdout since that's how it was
originally done.
+ # This can be changed w/ a discussion.
+ return self._spawn(command, fd_pipes={0: 0, 1: 1, 2: 1}, **kwargs)
+
@staticmethod
def _rewrite_uri_from_stat(path, uri):
chunks = uri.split("//", 1)
@@ -209,7 +217,7 @@ class ExternalSyncer(Syncer):
class VcsSyncer(ExternalSyncer):
- def _sync(self, verbosity, output_fd):
+ def _sync(self, verbosity):
try:
st = os.stat(self.basedir)
except FileNotFoundError:
@@ -229,7 +237,7 @@ class VcsSyncer(ExternalSyncer):
elif verbosity > 0:
command.append("-" + "v" * verbosity)
- ret = self._spawn(command, pipes={1: output_fd, 2: output_fd, 0: 0},
cwd=chdir)
+ ret = self._spawn_interactive(command, cwd=chdir)
return ret == 0
def _initial_pull(self):
diff --git a/src/pkgcore/sync/http.py b/src/pkgcore/sync/http.py
index b422d94ad..a3ec1898a 100644
--- a/src/pkgcore/sync/http.py
+++ b/src/pkgcore/sync/http.py
@@ -22,7 +22,7 @@ class http_syncer(base.Syncer):
self.basename = os.path.basename(uri)
super().__init__(basedir, uri, **kwargs)
- def _sync(self, verbosity, output_fd, force=False, **kwargs):
+ def _sync(self, verbosity, force=False, **kwargs):
dest = self._pre_download()
if self.uri.lower().startswith("https://";):
diff --git a/src/pkgcore/sync/rsync.py b/src/pkgcore/sync/rsync.py
index c6b95c70f..b0f032731 100644
--- a/src/pkgcore/sync/rsync.py
+++ b/src/pkgcore/sync/rsync.py
@@ -128,8 +128,7 @@ class rsync_syncer(base.ExternalSyncer):
f"DNS resolution failed for {self.hostname!r}: {e.strerror}"
)
- def _sync(self, verbosity, output_fd):
- fd_pipes = {1: output_fd, 2: output_fd}
+ def _sync(self, verbosity):
opts = list(self.opts)
if self.rsh:
opts.append("-e")
@@ -150,7 +149,7 @@ class rsync_syncer(base.ExternalSyncer):
self.basedir,
] + opts
- ret = self._spawn(cmd, fd_pipes)
+ ret = self._spawn(cmd)
if ret == 0:
return True
elif ret == 1:
@@ -203,7 +202,7 @@ class rsync_timestamp_syncer(rsync_syncer):
# malformed timestamp
return None
- def _sync(self, verbosity, output_fd, force=False):
+ def _sync(self, verbosity, force=False):
doit = force or self.last_timestamp is None
ret = None
try:
@@ -213,7 +212,7 @@ class rsync_timestamp_syncer(rsync_syncer):
timestamp_uri = pjoin(self.uri, "metadata",
"timestamp.chk")
timestamp_path = new_timestamp.name
timestamp_syncer = _RsyncFileSyncer(timestamp_path,
timestamp_uri)
- ret = timestamp_syncer._sync(verbosity, output_fd)
+ ret = timestamp_syncer._sync(verbosity)
if not ret:
doit = True
else:
@@ -226,7 +225,7 @@ class rsync_timestamp_syncer(rsync_syncer):
doit = delta > self.negative_sync_delay
if not doit:
return True
- ret = super()._sync(verbosity, output_fd)
+ ret = super()._sync(verbosity)
# force a reset of the timestamp
self.last_timestamp = self.current_timestamp()
finally:
diff --git a/src/pkgcore/sync/svn.py b/src/pkgcore/sync/svn.py
index e8cd60593..6b9e29a52 100644
--- a/src/pkgcore/sync/svn.py
+++ b/src/pkgcore/sync/svn.py
@@ -50,19 +50,17 @@ class svn_syncer(base.ExternalSyncer):
raise base.UriError(raw_uri, "protocol unknown")
return raw_uri
- def _sync(self, verbosity, output_fd):
+ def _sync(self, verbosity):
uri = self.uri
if uri.startswith("svn+http://";):
uri = uri.replace("svn+http://";, "http://";)
elif uri.startswith("svn+https://";):
uri = uri.replace("svn+https://";, "https://";)
if not os.path.exists(self.basedir):
- return 0 == self._spawn(
- [self.binary_path, "co", uri, self.basedir],
- {1: output_fd, 2: output_fd, 0: 0},
+ return 0 == self._spawn_interactive(
+ [self.binary_path, "co", uri, self.basedir]
)
- return 0 == self._spawn(
+ return 0 == self._spawn_interactive(
[self.binary_path, "update"],
- {1: output_fd, 2: output_fd, 0: 0},
cwd=self.basedir,
)
diff --git a/tests/scripts/test_pmaint.py b/tests/scripts/test_pmaint.py
index 63846456d..c9af122f2 100644
--- a/tests/scripts/test_pmaint.py
+++ b/tests/scripts/test_pmaint.py
@@ -1,6 +1,9 @@
from functools import partial
from io import BytesIO
+from snakeoil.formatters import PlainTextFormatter
+from snakeoil.mappings import AttrAccessible
+
from pkgcore.config import basics
from pkgcore.config.hint import ConfigHint
from pkgcore.ebuild.cpv import CPV
@@ -9,8 +12,6 @@ from pkgcore.repository import syncable, util
from pkgcore.scripts import pmaint
from pkgcore.sync import base
from pkgcore.test.scripts.helpers import ArgParseMixin
-from snakeoil.formatters import PlainTextFormatter
-from snakeoil.mappings import AttrAccessible
Options = AttrAccessible
@@ -96,7 +97,7 @@ class FakeSyncer(base.Syncer):
super().__init__(*args, **kwargs)
self.synced = False
- def _sync(self, verbosity, output_fd, **kwds):
+ def _sync(self, verbosity, **kwds):
self.synced = True
return self.succeed
