Replace 'push-url' and 'pull-url' options with just 'url'sam/canonical-push-urls

This is possible now that the SSH protocol can redirect to the
appropriate pull URL.

Note that this commit makes no attempt at backwards compatibility.
Everthing will break!

11 files changed, 73 insertions, 123 deletions

diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py
index 7c43b69e2..98aefc00c 100644
--- a/buildstream/_artifactcache/artifactcache.py
+++ b/buildstream/_artifactcache/artifactcache.py
@@ -40,39 +40,26 @@ class ArtifactCache():
         os.makedirs(context.artifactdir, exist_ok=True)
         self.extractdir = os.path.join(context.artifactdir, 'extract')
 
-        self._pull_local = False
-        self._push_local = False
+        self._local = False
 
         project_overrides = context._get_overrides(project.name)
         artifact_overrides = _yaml.node_get(project_overrides, Mapping, 'artifacts', default_value={})
-        override_pull = _yaml.node_get(artifact_overrides, str, 'pull-url', default_value='') or None
-        override_push = _yaml.node_get(artifact_overrides, str, 'push-url', default_value='') or None
+        override_url = _yaml.node_get(artifact_overrides, str, 'url', default_value='') or None
 
-        _yaml.node_validate(artifact_overrides, ['pull-url', 'push-url'])
-
-        if override_pull or override_push:
-            self.artifact_pull = override_pull
-            self.artifact_push = override_push
-
-        elif any((project.artifact_pull, project.artifact_push)):
-            self.artifact_pull = project.artifact_pull
-            self.artifact_push = project.artifact_push
+        _yaml.node_validate(artifact_overrides, ['url'])
 
+        if override_url:
+            self.url = override_url
+        elif project.artifact_url:
+            self.url = project.artifact_url
         else:
-            self.artifact_pull = context.artifact_pull
-            self.artifact_push = context.artifact_push
-
-        if self.artifact_push:
-            if self.artifact_push.startswith("/") or \
-               self.artifact_push.startswith("file://"):
-                self._push_local = True
+            self.url = context.artifact_url
 
-        if self.artifact_pull:
-            if self.artifact_pull.startswith("/") or \
-               self.artifact_pull.startswith("file://"):
-                self._pull_local = True
+        if self.url:
+            if self.url.startswith('/') or self.url.startswith('file://'):
+                self._local = True
 
-            self.remote = utils.url_directory_name(self.artifact_pull)
+            self.remote = utils.url_directory_name(self.url)
         else:
             self.remote = None
 
@@ -139,7 +126,7 @@ class ArtifactCache():
     # Returns: True if remote repository is available, False otherwise
     #
     def can_fetch(self):
-        return (not self._offline or self._pull_local) and \
+        return (not self._offline or self._local) and \
             self.remote is not None
 
     # can_push():
@@ -149,8 +136,8 @@ class ArtifactCache():
     # Returns: True if remote repository is available, False otherwise
     #
     def can_push(self):
-        return (not self._offline or self._push_local) and \
-            self.artifact_push is not None
+        return (not self._offline or self._local) and \
+            self.url is not None
 
     # remote_contains_key():
     #
diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py
index af1b27b9e..e11be09fe 100644
--- a/buildstream/_artifactcache/ostreecache.py
+++ b/buildstream/_artifactcache/ostreecache.py
@@ -69,28 +69,35 @@ class OSTreeCache(ArtifactCache):
         ostreedir = os.path.join(context.artifactdir, 'ostree')
         self.repo = _ostree.ensure(ostreedir, False)
 
-        if self.artifact_pull:
-            _ostree.configure_remote(self.repo, self.remote, self.artifact_pull)
+        if self.url is None:
+            self.push_url = None
+            self.pull_url = None
+        else:
+            if self.url.startswith('ssh://'):
+                self.push_url = self.url
+                try:
+                    self.pull_url = initialize_push_connection(self.push_url)
+                except PushException as e:
+                    raise ArtifactError("BuildStream did not connect succesfully "
+                                        "to the shared cache: {}".format(e))
+            elif self.url.startswith('http://') or self.url.startswith('https://'):
+                self.push_url = None
+                self.pull_url = self.url
+            elif self._local:
+                self.push_url = self.url
+                self.pull_url = self.url
+            else:
+                raise ArtifactError("Unsupported URL scheme: {}".format(self.url))
+
+            _ostree.configure_remote(self.repo, self.remote, self.pull_url)
 
         self._remote_refs = None
 
     def can_push(self):
         if self.enable_push:
-            return super().can_push()
+            return (not self._offline or self._local) and self.push_url is not None
         return False
 
-    def preflight(self):
-        if self.can_push() and not self.artifact_push.startswith("/"):
-            try:
-                pull_url = initialize_push_connection(self.artifact_push)
-                if pull_url != self.artifact_pull:
-                    raise ArtifactError(
-                        "This cache reports its pull URL as {}, but user "
-                        "configuration specifies {}.".format(pull_url, self.artifact_pull))
-            except PushException as e:
-                raise ArtifactError("BuildStream will be unable to push artifacts "
-                                    "to the shared cache: {}".format(e))
-
     # contains():
     #
     # Check whether the artifact for the specified Element is already available
@@ -240,15 +247,13 @@ class OSTreeCache(ArtifactCache):
     #
     def pull(self, element, progress=None):
 
-        if self._offline and not self._pull_local:
+        if self._offline and not self._local:
             raise ArtifactError("Attempt to pull artifact while offline")
 
-        if self.artifact_pull.startswith("/"):
-            remote = "file://" + self.artifact_pull
-        elif self.remote is not None:
-            remote = self.remote
+        if self.pull_url.startswith("/"):
+            remote = "file://" + self.pull_url
         else:
-            raise ArtifactError("Attempt to pull artifact without any pull URL")
+            remote = self.remote
 
         weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK))
 
@@ -290,8 +295,8 @@ class OSTreeCache(ArtifactCache):
     # Fetch list of artifacts from remote repository.
     #
     def fetch_remote_refs(self):
-        if self.artifact_pull.startswith("/"):
-            remote = "file://" + self.artifact_pull
+        if self.pull_url.startswith("/"):
+            remote = "file://" + self.pull_url
         elif self.remote is not None:
             remote = self.remote
         else:
@@ -329,17 +334,17 @@ class OSTreeCache(ArtifactCache):
     #   (ArtifactError): if there was an error
     def push(self, element):
 
-        if self._offline and not self._push_local:
+        if self._offline and not self._local:
             raise ArtifactError("Attempt to push artifact while offline")
 
-        if self.artifact_push is None:
-            raise ArtifactError("Attempt to push artifact without any push URL")
+        if self.push_url is None:
+            raise ArtifactError("The protocol in use does not support pushing.")
 
         ref = buildref(element, element._get_cache_key_from_artifact())
         weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK))
-        if self.artifact_push.startswith("/"):
+        if self.push_url.startswith("/"):
             # local repository
-            push_repo = _ostree.ensure(self.artifact_push, True)
+            push_repo = _ostree.ensure(self.push_url, True)
             _ostree.fetch(push_repo, remote=self.repo.get_path().get_uri(), ref=ref)
             _ostree.fetch(push_repo, remote=self.repo.get_path().get_uri(), ref=weak_ref)
 
@@ -363,7 +368,7 @@ class OSTreeCache(ArtifactCache):
                     element._output_file() as output_file:
                     try:
                         pushed = push_artifact(temp_repo.get_path().get_path(),
-                                               self.artifact_push,
+                                               self.push_url,
                                                [ref, weak_ref], output_file)
                     except PushException as e:
                         raise ArtifactError("Failed to push artifact {}: {}".format(ref, e)) from e
diff --git a/buildstream/_context.py b/buildstream/_context.py
index 4a597524d..5fd4a428e 100644
--- a/buildstream/_context.py
+++ b/buildstream/_context.py
@@ -59,11 +59,8 @@ class Context():
         # The local binary artifact cache directory
         self.artifactdir = None
 
-        # The URL from which to download prebuilt artifacts
-        self.artifact_pull = None
-
-        # The URL to upload built artifacts to
-        self.artifact_push = None
+        # The URL from which to push and pull prebuilt artifacts
+        self.artifact_url = None
 
         # The directory to store build logs
         self.logdir = None
@@ -163,9 +160,8 @@ class Context():
 
         # Load artifact share configuration
         artifacts = _yaml.node_get(defaults, Mapping, 'artifacts')
-        _yaml.node_validate(artifacts, ['pull-url', 'push-url'])
-        self.artifact_pull = _yaml.node_get(artifacts, str, 'pull-url', default_value='') or None
-        self.artifact_push = _yaml.node_get(artifacts, str, 'push-url', default_value='') or None
+        _yaml.node_validate(artifacts, ['url'])
+        self.artifact_url = _yaml.node_get(artifacts, str, 'url', default_value='') or None
 
         # Load logging config
         logging = _yaml.node_get(defaults, Mapping, 'logging')
diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py
index 75e5b0a50..8245b31ca 100644
--- a/buildstream/_pipeline.py
+++ b/buildstream/_pipeline.py
@@ -174,7 +174,7 @@ class Pipeline():
         if fetch_remote_refs and self.artifacts.can_fetch():
             try:
                 if remote_ticker:
-                    remote_ticker(self.artifacts.artifact_pull)
+                    remote_ticker(self.artifacts.url)
                 self.artifacts.fetch_remote_refs()
             except ArtifactError:
                 self.message(MessageType.WARN, "Failed to fetch remote refs")
@@ -284,24 +284,6 @@ class Pipeline():
 
         return element
 
-    # Internal: If a remote artifact cache is configured for pushing, check
-    # that it actually works. Returns True if it works, False otherwise.
-    def can_push_remote_artifact_cache(self):
-        if self.artifacts.can_push():
-            starttime = datetime.datetime.now()
-            self.message(MessageType.START, "Checking connectivity to remote artifact cache")
-            try:
-                self.artifacts.preflight()
-            except ArtifactError as e:
-                self.message(MessageType.WARN, str(e),
-                             elapsed=datetime.datetime.now() - starttime)
-                return False
-            self.message(MessageType.SUCCESS, "Connectivity OK",
-                         elapsed=datetime.datetime.now() - starttime)
-            return True
-        else:
-            return False
-
     #############################################################
     #                         Commands                          #
     #############################################################
@@ -433,7 +415,7 @@ class Pipeline():
             queues.append(pull)
         queues.append(fetch)
         queues.append(build)
-        if self.can_push_remote_artifact_cache():
+        if self.artifacts.can_push():
             push = PushQueue()
             queues.append(push)
         queues[0].enqueue(plan)
@@ -683,8 +665,6 @@ class Pipeline():
 
         if not self.artifacts.can_push():
             raise PipelineError("Not configured for pushing artifacts")
-        if not self.can_push_remote_artifact_cache():
-            raise PipelineError("Unable to push to the configured remote cache")
 
         plan = elements
         self.assert_consistent(plan)
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 8348ca9f1..3a4bf97f3 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -171,9 +171,8 @@ class Project():
 
         # Load artifacts pull/push configuration for this project
         artifacts = _yaml.node_get(config, Mapping, 'artifacts', default_value={})
-        _yaml.node_validate(artifacts, ['pull-url', 'push-url'])
-        self.artifact_pull = _yaml.node_get(artifacts, str, 'pull-url', default_value='') or None
-        self.artifact_push = _yaml.node_get(artifacts, str, 'push-url', default_value='') or None
+        _yaml.node_validate(artifacts, ['url'])
+        self.artifact_url = _yaml.node_get(artifacts, str, 'url', default_value='') or None
 
         # Workspace configurations
         self._workspaces = self._load_workspace_config()
diff --git a/buildstream/data/userconfig.yaml b/buildstream/data/userconfig.yaml
index 17c855b1f..f43989dcc 100644
--- a/buildstream/data/userconfig.yaml
+++ b/buildstream/data/userconfig.yaml
@@ -53,12 +53,9 @@ scheduler:
 #
 artifacts:
 
-  # A url from which to download prebuilt artifacts
-  pull-url: ''
-
-  # A url to upload built artifacts to
-  # (must point to the same repository as pull-url)
-  push-url: ''
+  # A url from which to push and pull prebuilt artifacts.
+  # Some protocols only support pushing.
+  url: ''
 
 #
 #    Logging
diff --git a/doc/source/artifacts.rst b/doc/source/artifacts.rst
index 9547c98d2..330de818c 100644
--- a/doc/source/artifacts.rst
+++ b/doc/source/artifacts.rst
@@ -159,11 +159,10 @@ then a user can use the following user configuration:
    #
    artifacts:
 
-     # A url from which to download prebuilt artifacts
-     pull-url: https://artifacts.com
+     url: https://artifacts.com/artifacts
 
-     # A url to upload built artifacts to. Note we specify a custom port.
-     push-url: ssh://artifacts@artifacts.com:22200/artifacts
+     # Alternative form if you have push access to the cache
+     #url: ssh://artifacts@artifacts.com:22200/artifacts
 
 
 Authenticating Users
diff --git a/doc/source/config.rst b/doc/source/config.rst
index ef3e92a6b..2b9883f93 100644
--- a/doc/source/config.rst
+++ b/doc/source/config.rst
@@ -41,8 +41,7 @@ it can be overridden on a per project basis using the same format
    projects:
      project-name:
        artifacts:
-         pull-url: https://artifacts.com
-         push-url: ssh://artifacts@artifacts.com:22200/artifacts
+         url: https://artifacts.com/artifacts
 
 
 Strict Build Plan
diff --git a/doc/source/projectconf.rst b/doc/source/projectconf.rst
index 7c53f3721..c8bfdeefc 100644
--- a/doc/source/projectconf.rst
+++ b/doc/source/projectconf.rst
@@ -74,11 +74,7 @@ with an artifact share.
   artifacts:
 
     # A url from which to download prebuilt artifacts
-    pull-url: https://foo.com/artifacts
-
-    # A url to upload built artifacts to
-    # (must point to the same repository as pull-url)
-    push-url: ssh://artifacts@foo.com:22200/artifacts
+    url: https://foo.com/artifacts
 
 
 Plugin Paths
diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py
index 07d664c5f..116bc9e49 100644
--- a/tests/frontend/pull.py
+++ b/tests/frontend/pull.py
@@ -53,14 +53,12 @@ def test_push_pull(cli, tmpdir, datafiles, user_url, project_url, override_url):
     # Configure artifact share
     cli.configure({
         'artifacts': {
-            'pull-url': user_url,
-            'push-url': user_url,
+            'url': user_url,
         },
         'projects': {
             'test': {
                 'artifacts': {
-                    'pull-url': override_url,
-                    'push-url': override_url,
+                    'url': override_url,
                 }
             }
         }
@@ -71,8 +69,7 @@ def test_push_pull(cli, tmpdir, datafiles, user_url, project_url, override_url):
         project_config = _yaml.load(project_conf_file)
         project_config.update({
             'artifacts': {
-                'pull-url': project_url,
-                'push-url': project_url,
+                'url': project_url,
             }
         })
         _yaml.dump(_yaml.node_sanitize(project_config), filename=project_conf_file)
@@ -137,8 +134,7 @@ def test_push_pull_all(cli, tmpdir, datafiles):
             'pushers': 1
         },
         'artifacts': {
-            'pull-url': share.repo,
-            'push-url': share.repo,
+            'url': share.repo,
         }
     })
 
diff --git a/tests/frontend/push.py b/tests/frontend/push.py
index 89a864d16..d4ae6c6dc 100644
--- a/tests/frontend/push.py
+++ b/tests/frontend/push.py
@@ -52,14 +52,12 @@ def test_push(cli, tmpdir, datafiles, user_url, project_url, override_url):
     # Configure artifact share
     cli.configure({
         'artifacts': {
-            'pull-url': user_url,
-            'push-url': user_url,
+            'url': user_url,
         },
         'projects': {
             'test': {
                 'artifacts': {
-                    'pull-url': override_url,
-                    'push-url': override_url,
+                    'url': override_url,
                 }
             }
         }
@@ -70,8 +68,7 @@ def test_push(cli, tmpdir, datafiles, user_url, project_url, override_url):
         project_config = _yaml.load(project_conf_file)
         project_config.update({
             'artifacts': {
-                'pull-url': project_url,
-                'push-url': project_url,
+                'url': project_url,
             }
         })
         _yaml.dump(_yaml.node_sanitize(project_config), filename=project_conf_file)
@@ -112,8 +109,7 @@ def test_push_all(cli, tmpdir, datafiles):
             'pushers': 1
         },
         'artifacts': {
-            'pull-url': share.repo,
-            'push-url': share.repo,
+            'url': share.repo,
         }
     })