Merge lp:~jelmer/brz/ssh-config into lp:brz

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/ssh-config
Merge into: lp:brz
Diff against target: 258 lines (+73/-45)
4 files modified
breezy/config.py (+4/-0)
breezy/tests/test_ssh_transport.py (+61/-33)
breezy/transport/ssh.py (+5/-12)
doc/en/release-notes/brz-3.1.txt (+3/-0)
To merge this branch: bzr merge lp:~jelmer/brz/ssh-config
Reviewer Review Type Date Requested Status
Martin Packman Approve
Review via email: mp+369320@code.launchpad.net

Commit message

Allow the ssh vendor to be configured in the config, rather than just using the BRZ_SSH environment variable.

Description of the change

Allow the ssh vendor to be configured in the config, rather than just using the BRZ_SSH environment variable.

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

Makes sense to have in config.

review: Approve
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'breezy/config.py'
--- breezy/config.py 2019-07-07 18:18:20 +0000
+++ breezy/config.py 2019-07-07 18:58:24 +0000
@@ -2670,6 +2670,10 @@
2670 help="If we wait for a new request from a client for more than"2670 help="If we wait for a new request from a client for more than"
2671 " X seconds, consider the client idle, and hangup."))2671 " X seconds, consider the client idle, and hangup."))
2672option_registry.register(2672option_registry.register(
2673 Option('ssh',
2674 default=None, override_from_env=['BRZ_SSH'],
2675 help='SSH vendor to use.'))
2676option_registry.register(
2673 Option('stacked_on_location',2677 Option('stacked_on_location',
2674 default=None,2678 default=None,
2675 help="""The location where this branch is stacked on."""))2679 help="""The location where this branch is stacked on."""))
26762680
=== modified file 'breezy/tests/test_ssh_transport.py'
--- breezy/tests/test_ssh_transport.py 2018-11-11 04:08:32 +0000
+++ breezy/tests/test_ssh_transport.py 2019-07-07 18:58:24 +0000
@@ -14,7 +14,8 @@
14# along with this program; if not, write to the Free Software14# along with this program; if not, write to the Free Software
15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1616
17from breezy.tests import TestCase17from breezy import config
18from breezy.tests import TestCase, TestCaseWithTransport
18from breezy.errors import SSHVendorNotFound, UnknownSSH19from breezy.errors import SSHVendorNotFound, UnknownSSH
19from breezy.transport.ssh import (20from breezy.transport.ssh import (
20 OpenSSHSubprocessVendor,21 OpenSSHSubprocessVendor,
@@ -37,73 +38,95 @@
37 return self._ssh_version_string38 return self._ssh_version_string
3839
3940
40class SSHVendorManagerTests(TestCase):41class SSHVendorManagerTests(TestCaseWithTransport):
4142
42 def test_register_vendor(self):43 def test_register_vendor(self):
43 manager = TestSSHVendorManager()44 manager = TestSSHVendorManager()
44 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})45 self.overrideEnv('BRZ_SSH', None)
46 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
45 vendor = object()47 vendor = object()
46 manager.register_vendor("vendor", vendor)48 manager.register_vendor("vendor", vendor)
47 self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)49 self.overrideEnv('BRZ_SSH', 'vendor')
50 self.assertIs(manager.get_vendor(), vendor)
4851
49 def test_default_vendor(self):52 def test_default_vendor(self):
50 manager = TestSSHVendorManager()53 manager = TestSSHVendorManager()
51 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})54 self.overrideEnv('BRZ_SSH', None)
55 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
52 vendor = object()56 vendor = object()
53 manager.register_default_vendor(vendor)57 manager.register_default_vendor(vendor)
54 self.assertIs(manager.get_vendor({}), vendor)58 self.assertIs(manager.get_vendor(), vendor)
5559
56 def test_get_vendor_by_environment(self):60 def test_get_vendor_by_environment(self):
57 manager = TestSSHVendorManager()61 manager = TestSSHVendorManager()
58 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})62 self.overrideEnv('BRZ_SSH', None)
59 self.assertRaises(UnknownSSH,63 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
60 manager.get_vendor, {"BRZ_SSH": "vendor"})64 self.overrideEnv('BRZ_SSH', 'vendor')
61 vendor = object()65 self.assertRaises(UnknownSSH, manager.get_vendor)
62 manager.register_vendor("vendor", vendor)66 vendor = object()
63 self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)67 manager.register_vendor("vendor", vendor)
68 self.assertIs(manager.get_vendor(), vendor)
69
70 def test_get_vendor_by_config(self):
71 manager = TestSSHVendorManager()
72 self.overrideEnv('BRZ_SSH', None)
73 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
74 config.GlobalStack().set('ssh', 'vendor')
75 self.assertRaises(UnknownSSH, manager.get_vendor)
76 vendor = object()
77 manager.register_vendor("vendor", vendor)
78 self.assertIs(manager.get_vendor(), vendor)
6479
65 def test_get_vendor_by_inspection_openssh(self):80 def test_get_vendor_by_inspection_openssh(self):
66 manager = TestSSHVendorManager()81 manager = TestSSHVendorManager()
67 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})82 self.overrideEnv('BRZ_SSH', None)
83 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
68 manager.set_ssh_version_string("OpenSSH")84 manager.set_ssh_version_string("OpenSSH")
69 self.assertIsInstance(manager.get_vendor({}), OpenSSHSubprocessVendor)85 self.assertIsInstance(manager.get_vendor(), OpenSSHSubprocessVendor)
7086
71 def test_get_vendor_by_inspection_sshcorp(self):87 def test_get_vendor_by_inspection_sshcorp(self):
72 manager = TestSSHVendorManager()88 manager = TestSSHVendorManager()
73 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})89 self.overrideEnv('BRZ_SSH', None)
90 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
74 manager.set_ssh_version_string("SSH Secure Shell")91 manager.set_ssh_version_string("SSH Secure Shell")
75 self.assertIsInstance(manager.get_vendor({}), SSHCorpSubprocessVendor)92 self.assertIsInstance(manager.get_vendor(), SSHCorpSubprocessVendor)
7693
77 def test_get_vendor_by_inspection_lsh(self):94 def test_get_vendor_by_inspection_lsh(self):
78 manager = TestSSHVendorManager()95 manager = TestSSHVendorManager()
79 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})96 self.overrideEnv('BRZ_SSH', None)
97 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
80 manager.set_ssh_version_string("lsh")98 manager.set_ssh_version_string("lsh")
81 self.assertIsInstance(manager.get_vendor({}), LSHSubprocessVendor)99 self.assertIsInstance(manager.get_vendor(), LSHSubprocessVendor)
82100
83 def test_get_vendor_by_inspection_plink(self):101 def test_get_vendor_by_inspection_plink(self):
84 manager = TestSSHVendorManager()102 manager = TestSSHVendorManager()
85 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})103 self.overrideEnv('BRZ_SSH', None)
104 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
86 manager.set_ssh_version_string("plink")105 manager.set_ssh_version_string("plink")
87 # Auto-detect of plink vendor disabled, on Windows recommended106 # Auto-detect of plink vendor disabled, on Windows recommended
88 # default ssh-client is paramiko107 # default ssh-client is paramiko
89 # see https://bugs.launchpad.net/bugs/414743108 # see https://bugs.launchpad.net/bugs/414743
90 #~self.assertIsInstance(manager.get_vendor({}), PLinkSubprocessVendor)109 #~self.assertIsInstance(manager.get_vendor(), PLinkSubprocessVendor)
91 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})110 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
92111
93 def test_cached_vendor(self):112 def test_cached_vendor(self):
94 manager = TestSSHVendorManager()113 manager = TestSSHVendorManager()
95 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})114 self.overrideEnv('BRZ_SSH', None)
115 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
96 vendor = object()116 vendor = object()
97 manager.register_vendor("vendor", vendor)117 manager.register_vendor("vendor", vendor)
98 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})118 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
99 # Once the vendor is found the result is cached (mainly because of the119 # Once the vendor is found the result is cached (mainly because of the
100 # 'get_vendor' sometimes can be an expensive operation) and later120 # 'get_vendor' sometimes can be an expensive operation) and later
101 # invocations of the 'get_vendor' just returns the cached value.121 # invocations of the 'get_vendor' just returns the cached value.
102 self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)122 self.overrideEnv('BRZ_SSH', 'vendor')
103 self.assertIs(manager.get_vendor({}), vendor)123 self.assertIs(manager.get_vendor(), vendor)
124 self.overrideEnv('BRZ_SSH', None)
125 self.assertIs(manager.get_vendor(), vendor)
104 # The cache can be cleared by the 'clear_cache' method126 # The cache can be cleared by the 'clear_cache' method
105 manager.clear_cache()127 manager.clear_cache()
106 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})128 self.overrideEnv('BRZ_SSH', None)
129 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
107130
108 def test_get_vendor_search_order(self):131 def test_get_vendor_search_order(self):
109 # The 'get_vendor' method search for SSH vendors as following:132 # The 'get_vendor' method search for SSH vendors as following:
@@ -118,33 +141,37 @@
118141
119 manager = TestSSHVendorManager()142 manager = TestSSHVendorManager()
120 # At first no vendors are found143 # At first no vendors are found
121 self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})144 self.overrideEnv('BRZ_SSH', None)
145 self.assertRaises(SSHVendorNotFound, manager.get_vendor)
122146
123 # If the default vendor is registered it will be returned147 # If the default vendor is registered it will be returned
124 default_vendor = object()148 default_vendor = object()
125 manager.register_default_vendor(default_vendor)149 manager.register_default_vendor(default_vendor)
126 self.assertIs(manager.get_vendor({}), default_vendor)150 self.assertIs(manager.get_vendor(), default_vendor)
127151
128 # If the known vendor is found in the system it will be returned152 # If the known vendor is found in the system it will be returned
129 manager.clear_cache()153 manager.clear_cache()
130 manager.set_ssh_version_string("OpenSSH")154 manager.set_ssh_version_string("OpenSSH")
131 self.assertIsInstance(manager.get_vendor({}), OpenSSHSubprocessVendor)155 self.assertIsInstance(manager.get_vendor(), OpenSSHSubprocessVendor)
132156
133 # If the BRZ_SSH environment variable is found it will be treated as157 # If the BRZ_SSH environment variable is found it will be treated as
134 # the vendor name158 # the vendor name
135 manager.clear_cache()159 manager.clear_cache()
136 vendor = object()160 vendor = object()
137 manager.register_vendor("vendor", vendor)161 manager.register_vendor("vendor", vendor)
138 self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)162 self.overrideEnv('BRZ_SSH', 'vendor')
163 self.assertIs(manager.get_vendor(), vendor)
139164
140 # Last cached value always checked first165 # Last cached value always checked first
141 self.assertIs(manager.get_vendor({}), vendor)166 self.overrideEnv('BRZ_SSH', 'vendor')
167 self.assertIs(manager.get_vendor(), vendor)
142168
143 def test_get_vendor_from_path_win32_plink(self):169 def test_get_vendor_from_path_win32_plink(self):
144 manager = TestSSHVendorManager()170 manager = TestSSHVendorManager()
145 manager.set_ssh_version_string("plink: Release 0.60")171 manager.set_ssh_version_string("plink: Release 0.60")
146 plink_path = "C:/Program Files/PuTTY/plink.exe"172 plink_path = "C:/Program Files/PuTTY/plink.exe"
147 vendor = manager.get_vendor({"BRZ_SSH": plink_path})173 self.overrideEnv('BRZ_SSH', plink_path)
174 vendor = manager.get_vendor()
148 self.assertIsInstance(vendor, PLinkSubprocessVendor)175 self.assertIsInstance(vendor, PLinkSubprocessVendor)
149 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])176 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])
150 self.assertEqual(args[0], plink_path)177 self.assertEqual(args[0], plink_path)
@@ -154,7 +181,8 @@
154 manager.set_ssh_version_string(181 manager.set_ssh_version_string(
155 "OpenSSH_5.1p1 Debian-5, OpenSSL, 0.9.8g 19 Oct 2007")182 "OpenSSH_5.1p1 Debian-5, OpenSSL, 0.9.8g 19 Oct 2007")
156 openssh_path = "/usr/bin/ssh"183 openssh_path = "/usr/bin/ssh"
157 vendor = manager.get_vendor({"BRZ_SSH": openssh_path})184 self.overrideEnv('BRZ_SSH', openssh_path)
185 vendor = manager.get_vendor()
158 self.assertIsInstance(vendor, OpenSSHSubprocessVendor)186 self.assertIsInstance(vendor, OpenSSHSubprocessVendor)
159 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])187 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])
160 self.assertEqual(args[0], openssh_path)188 self.assertEqual(args[0], openssh_path)
161189
=== modified file 'breezy/transport/ssh.py'
--- breezy/transport/ssh.py 2019-06-16 01:52:23 +0000
+++ breezy/transport/ssh.py 2019-07-07 18:58:24 +0000
@@ -80,16 +80,9 @@
80 """Clear previously cached lookup result."""80 """Clear previously cached lookup result."""
81 self._cached_ssh_vendor = None81 self._cached_ssh_vendor = None
8282
83 def _get_vendor_by_environment(self, environment=None):83 def _get_vendor_by_config(self):
84 """Return the vendor or None based on BRZ_SSH environment variable.84 vendor_name = config.GlobalStack().get('ssh')
8585 if vendor_name is not None:
86 :raises UnknownSSH: if the BRZ_SSH environment variable contains
87 unknown vendor name
88 """
89 if environment is None:
90 environment = os.environ
91 if 'BRZ_SSH' in environment:
92 vendor_name = environment['BRZ_SSH']
93 try:86 try:
94 vendor = self._ssh_vendors[vendor_name]87 vendor = self._ssh_vendors[vendor_name]
95 except KeyError:88 except KeyError:
@@ -151,7 +144,7 @@
151 return self._get_vendor_by_version_string(version,144 return self._get_vendor_by_version_string(version,
152 os.path.splitext(os.path.basename(path))[0])145 os.path.splitext(os.path.basename(path))[0])
153146
154 def get_vendor(self, environment=None):147 def get_vendor(self):
155 """Find out what version of SSH is on the system.148 """Find out what version of SSH is on the system.
156149
157 :raises SSHVendorNotFound: if no any SSH vendor is found150 :raises SSHVendorNotFound: if no any SSH vendor is found
@@ -159,7 +152,7 @@
159 unknown vendor name152 unknown vendor name
160 """153 """
161 if self._cached_ssh_vendor is None:154 if self._cached_ssh_vendor is None:
162 vendor = self._get_vendor_by_environment(environment)155 vendor = self._get_vendor_by_config()
163 if vendor is None:156 if vendor is None:
164 vendor = self._get_vendor_by_inspection()157 vendor = self._get_vendor_by_inspection()
165 if vendor is None:158 if vendor is None:
166159
=== modified file 'doc/en/release-notes/brz-3.1.txt'
--- doc/en/release-notes/brz-3.1.txt 2019-07-07 18:18:20 +0000
+++ doc/en/release-notes/brz-3.1.txt 2019-07-07 18:58:24 +0000
@@ -54,6 +54,9 @@
54* Automatically upgrade to branch format 8 when setting branch references.54* Automatically upgrade to branch format 8 when setting branch references.
55 (Jelmer Vernooij)55 (Jelmer Vernooij)
5656
57* The ``ssh`` configuration variable can be used to set the default
58 SSH implementation. (Jelmer Vernooij, #650757)
59
57* ``locks.steal_dead`` is now enabled by default.60* ``locks.steal_dead`` is now enabled by default.
58 (Jelmer Vernooij, #220464)61 (Jelmer Vernooij, #220464)
5962

Subscribers

People subscribed via source and target branches