From 57505893bf65a3acfdb585d92cfbabb70fb4a8da Mon Sep 17 00:00:00 2001
From: Seppo Yli-Olli <seppo.yliolli@gmail.com>
Date: Sun, 29 Jan 2023 17:48:00 +0200
Subject: [PATCH] Stop passing URL opener across process boundaries

The object is not picklable. This cannot be supported. Instead create
URL opener when needed in child process. Drops URL opener caching
which may result in minor performance impact.
---
 src/buildstream/_stream.py                |  7 ---
 src/buildstream/downloadablefilesource.py | 61 ++++++++++++-----------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 57c19ee1a..9372347e3 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -50,7 +50,6 @@
 from .types import _KeyStrength, _PipelineSelection, _Scope, _HostMount
 from .plugin import Plugin
 from . import utils, node, _yaml, _site, _pipeline
-from .downloadablefilesource import DownloadableFileSource
 
 
 # Stream()
@@ -118,12 +117,6 @@ def cleanup(self):
         # test isolation.
         node._reset_global_state()
 
-        # Ensure that any global state loaded by the downloadablefilesource
-        # is discarded in between sessions (different invocations of the CLI
-        # may come with different local state such as .netrc files, so we need
-        # a reset here).
-        DownloadableFileSource._reset_url_opener()
-
     # set_project()
     #
     # Set the top-level project.
diff --git a/src/buildstream/downloadablefilesource.py b/src/buildstream/downloadablefilesource.py
index d36da6749..6fec2bf33 100644
--- a/src/buildstream/downloadablefilesource.py
+++ b/src/buildstream/downloadablefilesource.py
@@ -88,7 +88,8 @@ def find_user_password(self, realm, authuri):
             return login, password
 
 
-def _download_file(opener, url, etag, directory):
+def _download_file(opener_creator, url, etag, directory):
+    opener = opener_creator.get_url_opener()
     default_name = os.path.basename(url)
     request = urllib.request.Request(url)
     request.add_header("Accept", "*/*")
@@ -120,7 +121,6 @@ class DownloadableFileSource(Source):
 
     COMMON_CONFIG_KEYS = Source.COMMON_CONFIG_KEYS + ["url", "ref", "etag"]
 
-    __urlopener = None
     __default_mirror_file = None
 
     def configure(self, node):
@@ -217,8 +217,10 @@ def _ensure_mirror(self, activity_name: str):
                 else:
                     etag = None
 
+                url_opener_creator = _UrlOpenerCreator(self._parse_netrc())
+
                 local_file, new_etag = self.blocking_activity(
-                    _download_file, (self.__get_urlopener(), self.url, etag, td), activity_name
+                    _download_file, (url_opener_creator, self.url, etag, td), activity_name
                 )
 
                 if local_file is None:
@@ -251,6 +253,21 @@ def _ensure_mirror(self, activity_name: str):
             # ValueError for unknown url types, so we handle it here.
             raise SourceError("{}: Error mirroring {}: {}".format(self, self.url, e), temporary=True) from e
 
+    def _parse_netrc(self):
+        netrc_config = None
+        try:
+            netrc_config = netrc.netrc()
+        except OSError:
+            # If the .netrc file was not found, FileNotFoundError will be
+            # raised, but OSError will be raised directly by the netrc package
+            # in the case that $HOME is not set.
+            #
+            # This will catch both cases.
+            pass
+        except netrc.NetrcParseError as e:
+            self.warn("{}: While reading .netrc: {}".format(self, e))
+        return netrc_config
+
     def _get_mirror_file(self, sha=None):
         if sha is not None:
             return os.path.join(self._mirror_dir, sha)
@@ -260,29 +277,15 @@ def _get_mirror_file(self, sha=None):
 
         return self.__default_mirror_file
 
-    @classmethod
-    def _reset_url_opener(cls):
-        # Needed for tests, in order to cleanup the `netrc` configuration.
-        cls.__urlopener = None  # pylint: disable=unused-private-member
-
-    def __get_urlopener(self):
-        if not DownloadableFileSource.__urlopener:
-            try:
-                netrc_config = netrc.netrc()
-            except OSError:
-                # If the .netrc file was not found, FileNotFoundError will be
-                # raised, but OSError will be raised directly by the netrc package
-                # in the case that $HOME is not set.
-                #
-                # This will catch both cases.
-                #
-                DownloadableFileSource.__urlopener = urllib.request.build_opener()
-            except netrc.NetrcParseError as e:
-                self.warn("{}: While reading .netrc: {}".format(self, e))
-                return urllib.request.build_opener()
-            else:
-                netrc_pw_mgr = _NetrcPasswordManager(netrc_config)
-                http_auth = urllib.request.HTTPBasicAuthHandler(netrc_pw_mgr)
-                ftp_handler = _NetrcFTPOpener(netrc_config)
-                DownloadableFileSource.__urlopener = urllib.request.build_opener(http_auth, ftp_handler)
-        return DownloadableFileSource.__urlopener
+
+class _UrlOpenerCreator:
+    def __init__(self, netrc_config):
+        self.netrc_config = netrc_config
+
+    def get_url_opener(self):
+        if self.netrc_config:
+            netrc_pw_mgr = _NetrcPasswordManager(self.netrc_config)
+            http_auth = urllib.request.HTTPBasicAuthHandler(netrc_pw_mgr)
+            ftp_handler = _NetrcFTPOpener(self.netrc_config)
+            return urllib.request.build_opener(http_auth, ftp_handler)
+        return urllib.request.build_opener()