Only initialize remote artifact cache connections if needed

This fixes a regression from the canonical-pull-urls branch that was
just merged. The `OSTreeCache.__init__()` function was connecting to
the cache, which is bad because execution would randomly freeze for
several seconds when the connection was slow.

We now only initialize remote connections where needed; this was
already introduced in 5c2ef6d076921bc0121e61efaa7a719c34ea1912 but
had regressed. I renamed the keyword arg from `fetch_remote_refs`
to `use_remote_cache` because it needs to be set for any interaction
with the remote cache, doesn't matter if they are fetches or pushes.

The initialization stage is also moved later so that we print a message
telling the user what we are up to before trying the network access.

4 files changed, 26 insertions, 15 deletions

diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py
index 98aefc00c..1283b37ab 100644
--- a/buildstream/_artifactcache/artifactcache.py
+++ b/buildstream/_artifactcache/artifactcache.py
@@ -65,6 +65,14 @@ class ArtifactCache():
 
         self._offline = False
 
+    # initialize_remote():
+    #
+    # Initialize any remote artifact cache, if needed. This may require network
+    # access and could block for several seconds.
+    #
+    def initialize_remote(self):
+        pass
+
     # contains():
     #
     # Check whether the artifact for the specified Element is already available
diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py
index e11be09fe..78796dc6c 100644
--- a/buildstream/_artifactcache/ostreecache.py
+++ b/buildstream/_artifactcache/ostreecache.py
@@ -69,13 +69,17 @@ class OSTreeCache(ArtifactCache):
         ostreedir = os.path.join(context.artifactdir, 'ostree')
         self.repo = _ostree.ensure(ostreedir, False)
 
-        if self.url is None:
-            self.push_url = None
-            self.pull_url = None
-        else:
+        self.push_url = None
+        self.pull_url = None
+
+        self._remote_refs = None
+
+    def initialize_remote(self):
+        if self.url is not None:
             if self.url.startswith('ssh://'):
                 self.push_url = self.url
                 try:
+                    # Contact the remote cache.
                     self.pull_url = initialize_push_connection(self.push_url)
                 except PushException as e:
                     raise ArtifactError("BuildStream did not connect succesfully "
@@ -91,8 +95,6 @@ class OSTreeCache(ArtifactCache):
 
             _ostree.configure_remote(self.repo, self.remote, self.pull_url)
 
-        self._remote_refs = None
-
     def can_push(self):
         if self.enable_push:
             return (not self._offline or self._local) and self.push_url is not None
diff --git a/buildstream/_frontend/main.py b/buildstream/_frontend/main.py
index e02582799..d70645887 100644
--- a/buildstream/_frontend/main.py
+++ b/buildstream/_frontend/main.py
@@ -201,7 +201,7 @@ def cli(context, **kwargs):
 def build(app, elements, all, track):
     """Build elements in a pipeline"""
 
-    app.initialize(elements, rewritable=track, inconsistent=track, fetch_remote_refs=True)
+    app.initialize(elements, rewritable=track, inconsistent=track, use_remote_cache=True)
     app.print_heading()
     try:
         app.pipeline.build(app.scheduler, all, track)
@@ -316,7 +316,7 @@ def pull(app, elements, deps):
         none:  No dependencies, just the element itself
         all:   All dependencies
     """
-    app.initialize(elements, fetch_remote_refs=True)
+    app.initialize(elements, use_remote_cache=True)
     try:
         to_pull = app.pipeline.deps_elements(deps)
         app.pipeline.pull(app.scheduler, to_pull)
@@ -346,7 +346,7 @@ def push(app, elements, deps):
         none:  No dependencies, just the element itself
         all:   All dependencies
     """
-    app.initialize(elements, fetch_remote_refs=True)
+    app.initialize(elements, use_remote_cache=True)
     try:
         to_push = app.pipeline.deps_elements(deps)
         app.pipeline.push(app.scheduler, to_push)
@@ -425,7 +425,7 @@ def show(app, elements, deps, except_, order, format, downloadable):
         bst show target.bst --format \\
             $'---------- %{name} ----------\\n%{vars}'
     """
-    app.initialize(elements, except_=except_, fetch_remote_refs=downloadable)
+    app.initialize(elements, except_=except_, use_remote_cache=downloadable)
     try:
         dependencies = app.pipeline.deps_elements(deps)
     except PipelineError as e:
@@ -766,7 +766,7 @@ class App():
     # Initialize the main pipeline
     #
     def initialize(self, elements, except_=tuple(), rewritable=False,
-                   inconsistent=False, fetch_remote_refs=False):
+                   inconsistent=False, use_remote_cache=False):
 
         profile_start(Topics.LOAD_PIPELINE, "_".join(t.replace(os.sep, '-') for t in elements))
 
@@ -842,7 +842,7 @@ class App():
             self.pipeline = Pipeline(self.context, self.project, elements, except_,
                                      inconsistent=inconsistent,
                                      rewritable=rewritable,
-                                     fetch_remote_refs=fetch_remote_refs,
+                                     use_remote_cache=use_remote_cache,
                                      load_ticker=self.load_ticker,
                                      resolve_ticker=self.resolve_ticker,
                                      remote_ticker=self.remote_ticker,
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 8245b31ca..2a64e2f99 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -94,7 +94,7 @@ class Planner():
 #                         current source refs will not be the effective refs.
 #    rewritable (bool): Whether the loaded files should be rewritable
 #                       this is a bit more expensive due to deep copies
-#    fetch_remote_refs (bool): Whether to attempt to check remote artifact server for new refs
+#    use_remote_cache (bool): Whether to connect with remote artifact cache
 #    load_ticker (callable): A function which will be called for each loaded element
 #    resolve_ticker (callable): A function which will be called for each resolved element
 #    cache_ticker (callable): A function which will be called for each element
@@ -116,7 +116,7 @@ class Pipeline():
     def __init__(self, context, project, targets, except_,
                  inconsistent=False,
                  rewritable=False,
-                 fetch_remote_refs=False,
+                 use_remote_cache=False,
                  load_ticker=None,
                  resolve_ticker=None,
                  remote_ticker=None,
@@ -171,10 +171,11 @@ class Pipeline():
 
                 self.project._set_workspace(element, source, workspace)
 
-        if fetch_remote_refs and self.artifacts.can_fetch():
+        if use_remote_cache and self.artifacts.can_fetch():
             try:
                 if remote_ticker:
                     remote_ticker(self.artifacts.url)
+                self.artifacts.initialize_remote()
                 self.artifacts.fetch_remote_refs()
             except ArtifactError:
                 self.message(MessageType.WARN, "Failed to fetch remote refs")