Remove the push-port config option

Ports can and should be specified by using proper ssh:// URL forms,
e.g:

    ssh://artifacts@example.com:22200/artifacts

The alternate form of artifacts@example.com:artifacts isn't a valid
URL, and doesn't let you specify a different port. People are used
to this form due to Git continuing to use it but we should encourage
people to use proper URLs.

9 files changed, 16 insertions, 40 deletions

diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py
index ac4f6080d..7c43b69e2 100644
--- a/buildstream/_artifactcache/artifactcache.py
+++ b/buildstream/_artifactcache/artifactcache.py
@@ -47,24 +47,20 @@ class ArtifactCache():
         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_push_port = _yaml.node_get(artifact_overrides, int, 'push-port', default_value=22)
 
-        _yaml.node_validate(artifact_overrides, ['pull-url', 'push-url', 'push-port'])
+        _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
-            self.artifact_push_port = override_push_port
 
         elif any((project.artifact_pull, project.artifact_push)):
             self.artifact_pull = project.artifact_pull
             self.artifact_push = project.artifact_push
-            self.artifact_push_port = project.artifact_push_port
 
         else:
             self.artifact_pull = context.artifact_pull
             self.artifact_push = context.artifact_push
-            self.artifact_push_port = context.artifact_push_port
 
         if self.artifact_push:
             if self.artifact_push.startswith("/") or \
diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py
index 6b2411e7f..af1b27b9e 100644
--- a/buildstream/_artifactcache/ostreecache.py
+++ b/buildstream/_artifactcache/ostreecache.py
@@ -82,8 +82,7 @@ class OSTreeCache(ArtifactCache):
     def preflight(self):
         if self.can_push() and not self.artifact_push.startswith("/"):
             try:
-                pull_url = initialize_push_connection(self.artifact_push,
-                                                      self.artifact_push_port)
+                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 "
@@ -365,7 +364,6 @@ class OSTreeCache(ArtifactCache):
                     try:
                         pushed = push_artifact(temp_repo.get_path().get_path(),
                                                self.artifact_push,
-                                               self.artifact_push_port,
                                                [ref, weak_ref], output_file)
                     except PushException as e:
                         raise ArtifactError("Failed to push artifact {}: {}".format(ref, e)) from e
diff --git a/buildstream/_artifactcache/pushreceive.py b/buildstream/_artifactcache/pushreceive.py
index 18905fed3..39c92dfdf 100644
--- a/buildstream/_artifactcache/pushreceive.py
+++ b/buildstream/_artifactcache/pushreceive.py
@@ -315,7 +315,7 @@ class PushMessageReader(object):
         return args
 
 
-def parse_remote_location(remotepath, remote_port):
+def parse_remote_location(remotepath):
     """Parse remote artifact cache URL that's been specified in our config."""
     remote_host = remote_user = remote_repo = None
 
@@ -327,7 +327,7 @@ def parse_remote_location(remotepath, remote_port):
         remote_host = url.hostname
         remote_user = url.username
         remote_repo = url.path
-        remote_port = url.port
+        remote_port = url.port or 22
     else:
         # Scp/git style remote (user@hostname:path)
         parts = remotepath.split('@', 1)
@@ -343,6 +343,8 @@ def parse_remote_location(remotepath, remote_port):
                                 'contain a hostname and path separated '
                                 'by ":"'.format(remotepath))
         remote_host, remote_repo = parts
+        # This form doesn't make it possible to specify a non-standard port.
+        remote_port = 22
 
     return remote_host, remote_user, remote_repo, remote_port
 
@@ -634,15 +636,14 @@ class OSTreeReceiver(object):
 #
 # Args:
 #   remote: The ssh remote url to push to
-#   remote_port: The ssh port at the remote url
 #
 # Returns:
 #   (str): The URL that should be used for pushing to this cache.
 #
 # Raises:
 #   PushException if there was an issue connecting to the remote.
-def initialize_push_connection(remote, remote_port):
-    remote_host, remote_user, remote_repo, remote_port = parse_remote_location(remote, remote_port)
+def initialize_push_connection(remote):
+    remote_host, remote_user, remote_repo, remote_port = parse_remote_location(remote)
     ssh_cmd = ssh_commandline(remote_host, remote_user, remote_port)
 
     # We need a short timeout here because if 'remote' isn't reachable at
@@ -684,7 +685,6 @@ def initialize_push_connection(remote, remote_port):
 # Args:
 #   repo: The local repository path
 #   remote: The ssh remote url to push to
-#   remote_port: The ssh port at the remote url
 #   branches: The refs to push
 #   output: The output where logging should go
 #
@@ -695,12 +695,12 @@ def initialize_push_connection(remote, remote_port):
 # Raises:
 #   PushException if there was an error
 #
-def push(repo, remote, remote_port, branches, output):
+def push(repo, remote, branches, output):
 
     logging.basicConfig(format='%(module)s: %(levelname)s: %(message)s',
                         level=logging.INFO, stream=output)
 
-    pusher = OSTreePusher(repo, remote, remote_port, branches, True, False, output=output)
+    pusher = OSTreePusher(repo, remote, branches, True, False, output=output)
 
     def terminate_push():
         pusher.close()
diff --git a/buildstream/_context.py b/buildstream/_context.py
index f85771bac..4a597524d 100644
--- a/buildstream/_context.py
+++ b/buildstream/_context.py
@@ -65,9 +65,6 @@ class Context():
         # The URL to upload built artifacts to
         self.artifact_push = None
 
-        # The port number for pushing artifacts over ssh
-        self.artifact_push_port = 22
-
         # The directory to store build logs
         self.logdir = None
 
@@ -166,10 +163,9 @@ class Context():
 
         # Load artifact share configuration
         artifacts = _yaml.node_get(defaults, Mapping, 'artifacts')
-        _yaml.node_validate(artifacts, ['pull-url', 'push-url', 'push-port'])
+        _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
-        self.artifact_push_port = _yaml.node_get(artifacts, int, 'push-port', default_value=22)
 
         # Load logging config
         logging = _yaml.node_get(defaults, Mapping, 'logging')
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 46f271f5b..8348ca9f1 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -171,10 +171,9 @@ 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', 'push-port'])
+        _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
-        self.artifact_push_port = _yaml.node_get(artifacts, int, 'push-port', default_value=22)
 
         # Workspace configurations
         self._workspaces = self._load_workspace_config()
diff --git a/buildstream/data/userconfig.yaml b/buildstream/data/userconfig.yaml
index 8d066ab52..17c855b1f 100644
--- a/buildstream/data/userconfig.yaml
+++ b/buildstream/data/userconfig.yaml
@@ -60,10 +60,6 @@ artifacts:
   # (must point to the same repository as pull-url)
   push-url: ''
 
-  # Specify the port number for pushing artifacts, if it's
-  # not the default port 22
-  push-port: 22
-
 #
 #    Logging
 #
diff --git a/doc/source/artifacts.rst b/doc/source/artifacts.rst
index 575f51a51..9547c98d2 100644
--- a/doc/source/artifacts.rst
+++ b/doc/source/artifacts.rst
@@ -162,12 +162,8 @@ then a user can use the following user configuration:
      # A url from which to download prebuilt artifacts
      pull-url: https://artifacts.com
 
-     # A url to upload built artifacts to
-     push-url: artifacts@artifacts.com:artifacts
-
-     # If the artifact server uses a custom port for sshd
-     # then you can specify it here
-     push-port: 666
+     # A url to upload built artifacts to. Note we specify a custom port.
+     push-url: ssh://artifacts@artifacts.com:22200/artifacts
 
 
 Authenticating Users
diff --git a/doc/source/config.rst b/doc/source/config.rst
index 2b86156bd..ef3e92a6b 100644
--- a/doc/source/config.rst
+++ b/doc/source/config.rst
@@ -42,8 +42,7 @@ it can be overridden on a per project basis using the same format
      project-name:
        artifacts:
          pull-url: https://artifacts.com
-         push-url: artifacts@artifacts.com:artifacts
-         push-port: 443
+         push-url: ssh://artifacts@artifacts.com:22200/artifacts
 
 
 Strict Build Plan
diff --git a/doc/source/projectconf.rst b/doc/source/projectconf.rst
index cb0a2911b..7c53f3721 100644
--- a/doc/source/projectconf.rst
+++ b/doc/source/projectconf.rst
@@ -78,11 +78,7 @@ with an artifact share.
 
     # A url to upload built artifacts to
     # (must point to the same repository as pull-url)
-    push-url: artifacts@foo.com:artifacts
-
-    # Specify the port number for pushing artifacts, if it's
-    # not the default port 22
-    push-port: 10000
+    push-url: ssh://artifacts@foo.com:22200/artifacts
 
 
 Plugin Paths