From 4cff2bd3d5224629415a82786606dcc75af39729 Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 16 Oct 2023 10:49:11 -0400 Subject: [PATCH 1/3] analytics: fix mac not sending reports --- dvc/daemon.py | 52 +-------------------------------------------------- 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/dvc/daemon.py b/dvc/daemon.py index 7e57fadea7..444f5e3fab 100644 --- a/dvc/daemon.py +++ b/dvc/daemon.py @@ -3,7 +3,6 @@ import inspect import logging import os -import platform import sys from subprocess import Popen # nosec B404 from typing import List @@ -55,62 +54,13 @@ def _spawn_windows(cmd, env): _suppress_resource_warning(popen) -def _spawn_posix(cmd, env): - from dvc.cli import main - - # `fork` will copy buffers, so we need to flush them before forking. - # Otherwise, we will get duplicated outputs. - if sys.stdout and not sys.stdout.closed: - sys.stdout.flush() - if sys.stderr and not sys.stderr.closed: - sys.stderr.flush() - - # NOTE: using os._exit instead of sys.exit, because dvc built - # with PyInstaller has trouble with SystemExit exception and throws - # errors such as "[26338] Failed to execute script __main__" - try: - # pylint: disable-next=no-member - pid = os.fork() # type: ignore[attr-defined] - if pid > 0: - return - except OSError: - logger.exception("failed at first fork") - os._exit(1) # pylint: disable=protected-access - - os.setsid() # type: ignore[attr-defined] # pylint: disable=no-member - - try: - # pylint: disable-next=no-member - pid = os.fork() # type: ignore[attr-defined] - if pid > 0: - os._exit(0) # pylint: disable=protected-access - except OSError: - logger.exception("failed at second fork") - os._exit(1) # pylint: disable=protected-access - - sys.stdin.close() - sys.stdout.close() - sys.stderr.close() - os.closerange(0, 3) - - if platform.system() == "Darwin": - # workaround for MacOS bug - # https://github.com/iterative/dvc/issues/4294 - _popen(cmd, env=env).communicate() - else: - os.environ.update(env) - main(cmd) - - os._exit(0) # pylint: disable=protected-access - - def _spawn(cmd, env): logger.debug("Trying to spawn '%s'", cmd) if os.name == "nt": _spawn_windows(cmd, env) elif os.name == "posix": - _spawn_posix(cmd, env) + _popen(cmd, env=env) else: raise NotImplementedError From 59ad988eea9a8462c0c149e72579db48ea6cf44d Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 16 Oct 2023 11:52:01 -0400 Subject: [PATCH 2/3] limit analytics change to mac --- dvc/daemon.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/dvc/daemon.py b/dvc/daemon.py index 444f5e3fab..6941b20607 100644 --- a/dvc/daemon.py +++ b/dvc/daemon.py @@ -3,6 +3,7 @@ import inspect import logging import os +import platform import sys from subprocess import Popen # nosec B404 from typing import List @@ -54,13 +55,62 @@ def _spawn_windows(cmd, env): _suppress_resource_warning(popen) +def _spawn_posix(cmd, env): + from dvc.cli import main + + if platform.system() == "Darwin": + # workaround for MacOS bug + # https://github.com/iterative/dvc/issues/4294 + _popen(cmd, env=env) + else: + # `fork` will copy buffers, so we need to flush them before forking. + # Otherwise, we will get duplicated outputs. + if sys.stdout and not sys.stdout.closed: + sys.stdout.flush() + if sys.stderr and not sys.stderr.closed: + sys.stderr.flush() + + # NOTE: using os._exit instead of sys.exit, because dvc built + # with PyInstaller has trouble with SystemExit exception and throws + # errors such as "[26338] Failed to execute script __main__" + try: + # pylint: disable-next=no-member + pid = os.fork() # type: ignore[attr-defined] + if pid > 0: + return + except OSError: + logger.exception("failed at first fork") + os._exit(1) # pylint: disable=protected-access + + os.setsid() # type: ignore[attr-defined] # pylint: disable=no-member + + try: + # pylint: disable-next=no-member + pid = os.fork() # type: ignore[attr-defined] + if pid > 0: + os._exit(0) # pylint: disable=protected-access + except OSError: + logger.exception("failed at second fork") + os._exit(1) # pylint: disable=protected-access + + sys.stdin.close() + sys.stdout.close() + sys.stderr.close() + os.closerange(0, 3) + + os.environ.update(env) + main(cmd) + + os._exit(0) # pylint: disable=protected-access + + def _spawn(cmd, env): logger.debug("Trying to spawn '%s'", cmd) if os.name == "nt": _spawn_windows(cmd, env) elif os.name == "posix": - _popen(cmd, env=env) + _spawn_posix(cmd, env) else: raise NotImplementedError From 9ab6e4ebecc0c28d4aee77f01943e0a8187b45aa Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Mon, 16 Oct 2023 13:04:31 -0400 Subject: [PATCH 3/3] analytics: stop using popen on mac --- dvc/daemon.py | 84 ++++++++++++++++++++++++--------------------------- 1 file changed, 39 insertions(+), 45 deletions(-) diff --git a/dvc/daemon.py b/dvc/daemon.py index 6941b20607..ffaa308697 100644 --- a/dvc/daemon.py +++ b/dvc/daemon.py @@ -3,7 +3,6 @@ import inspect import logging import os -import platform import sys from subprocess import Popen # nosec B404 from typing import List @@ -58,50 +57,45 @@ def _spawn_windows(cmd, env): def _spawn_posix(cmd, env): from dvc.cli import main - if platform.system() == "Darwin": - # workaround for MacOS bug - # https://github.com/iterative/dvc/issues/4294 - _popen(cmd, env=env) - else: - # `fork` will copy buffers, so we need to flush them before forking. - # Otherwise, we will get duplicated outputs. - if sys.stdout and not sys.stdout.closed: - sys.stdout.flush() - if sys.stderr and not sys.stderr.closed: - sys.stderr.flush() - - # NOTE: using os._exit instead of sys.exit, because dvc built - # with PyInstaller has trouble with SystemExit exception and throws - # errors such as "[26338] Failed to execute script __main__" - try: - # pylint: disable-next=no-member - pid = os.fork() # type: ignore[attr-defined] - if pid > 0: - return - except OSError: - logger.exception("failed at first fork") - os._exit(1) # pylint: disable=protected-access - - os.setsid() # type: ignore[attr-defined] # pylint: disable=no-member - - try: - # pylint: disable-next=no-member - pid = os.fork() # type: ignore[attr-defined] - if pid > 0: - os._exit(0) # pylint: disable=protected-access - except OSError: - logger.exception("failed at second fork") - os._exit(1) # pylint: disable=protected-access - - sys.stdin.close() - sys.stdout.close() - sys.stderr.close() - os.closerange(0, 3) - - os.environ.update(env) - main(cmd) - - os._exit(0) # pylint: disable=protected-access + # `fork` will copy buffers, so we need to flush them before forking. + # Otherwise, we will get duplicated outputs. + if sys.stdout and not sys.stdout.closed: + sys.stdout.flush() + if sys.stderr and not sys.stderr.closed: + sys.stderr.flush() + + # NOTE: using os._exit instead of sys.exit, because dvc built + # with PyInstaller has trouble with SystemExit exception and throws + # errors such as "[26338] Failed to execute script __main__" + try: + # pylint: disable-next=no-member + pid = os.fork() # type: ignore[attr-defined] + if pid > 0: + return + except OSError: + logger.exception("failed at first fork") + os._exit(1) # pylint: disable=protected-access + + os.setsid() # type: ignore[attr-defined] # pylint: disable=no-member + + try: + # pylint: disable-next=no-member + pid = os.fork() # type: ignore[attr-defined] + if pid > 0: + os._exit(0) # pylint: disable=protected-access + except OSError: + logger.exception("failed at second fork") + os._exit(1) # pylint: disable=protected-access + + sys.stdin.close() + sys.stdout.close() + sys.stderr.close() + os.closerange(0, 3) + + os.environ.update(env) + main(cmd) + + os._exit(0) # pylint: disable=protected-access def _spawn(cmd, env):