Breezy

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

  1. ssh-config
  2. Merge into trunk
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 :

Merging failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/441/

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

Merging failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/444/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/config.py'
2--- breezy/config.py 2019-07-07 18:18:20 +0000
3+++ breezy/config.py 2019-07-07 18:58:24 +0000
4@@ -2670,6 +2670,10 @@
5 help="If we wait for a new request from a client for more than"
6 " X seconds, consider the client idle, and hangup."))
7 option_registry.register(
8+ Option('ssh',
9+ default=None, override_from_env=['BRZ_SSH'],
10+ help='SSH vendor to use.'))
11+option_registry.register(
12 Option('stacked_on_location',
13 default=None,
14 help="""The location where this branch is stacked on."""))
15
16=== modified file 'breezy/tests/test_ssh_transport.py'
17--- breezy/tests/test_ssh_transport.py 2018-11-11 04:08:32 +0000
18+++ breezy/tests/test_ssh_transport.py 2019-07-07 18:58:24 +0000
19@@ -14,7 +14,8 @@
20 # along with this program; if not, write to the Free Software
21 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
22
23-from breezy.tests import TestCase
24+from breezy import config
25+from breezy.tests import TestCase, TestCaseWithTransport
26 from breezy.errors import SSHVendorNotFound, UnknownSSH
27 from breezy.transport.ssh import (
28 OpenSSHSubprocessVendor,
29@@ -37,73 +38,95 @@
30 return self._ssh_version_string
31
32
33-class SSHVendorManagerTests(TestCase):
34+class SSHVendorManagerTests(TestCaseWithTransport):
35
36 def test_register_vendor(self):
37 manager = TestSSHVendorManager()
38- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
39+ self.overrideEnv('BRZ_SSH', None)
40+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
41 vendor = object()
42 manager.register_vendor("vendor", vendor)
43- self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)
44+ self.overrideEnv('BRZ_SSH', 'vendor')
45+ self.assertIs(manager.get_vendor(), vendor)
46
47 def test_default_vendor(self):
48 manager = TestSSHVendorManager()
49- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
50+ self.overrideEnv('BRZ_SSH', None)
51+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
52 vendor = object()
53 manager.register_default_vendor(vendor)
54- self.assertIs(manager.get_vendor({}), vendor)
55+ self.assertIs(manager.get_vendor(), vendor)
56
57 def test_get_vendor_by_environment(self):
58 manager = TestSSHVendorManager()
59- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
60- self.assertRaises(UnknownSSH,
61- manager.get_vendor, {"BRZ_SSH": "vendor"})
62- vendor = object()
63- manager.register_vendor("vendor", vendor)
64- self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)
65+ self.overrideEnv('BRZ_SSH', None)
66+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
67+ self.overrideEnv('BRZ_SSH', 'vendor')
68+ self.assertRaises(UnknownSSH, manager.get_vendor)
69+ vendor = object()
70+ manager.register_vendor("vendor", vendor)
71+ self.assertIs(manager.get_vendor(), vendor)
72+
73+ def test_get_vendor_by_config(self):
74+ manager = TestSSHVendorManager()
75+ self.overrideEnv('BRZ_SSH', None)
76+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
77+ config.GlobalStack().set('ssh', 'vendor')
78+ self.assertRaises(UnknownSSH, manager.get_vendor)
79+ vendor = object()
80+ manager.register_vendor("vendor", vendor)
81+ self.assertIs(manager.get_vendor(), vendor)
82
83 def test_get_vendor_by_inspection_openssh(self):
84 manager = TestSSHVendorManager()
85- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
86+ self.overrideEnv('BRZ_SSH', None)
87+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
88 manager.set_ssh_version_string("OpenSSH")
89- self.assertIsInstance(manager.get_vendor({}), OpenSSHSubprocessVendor)
90+ self.assertIsInstance(manager.get_vendor(), OpenSSHSubprocessVendor)
91
92 def test_get_vendor_by_inspection_sshcorp(self):
93 manager = TestSSHVendorManager()
94- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
95+ self.overrideEnv('BRZ_SSH', None)
96+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
97 manager.set_ssh_version_string("SSH Secure Shell")
98- self.assertIsInstance(manager.get_vendor({}), SSHCorpSubprocessVendor)
99+ self.assertIsInstance(manager.get_vendor(), SSHCorpSubprocessVendor)
100
101 def test_get_vendor_by_inspection_lsh(self):
102 manager = TestSSHVendorManager()
103- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
104+ self.overrideEnv('BRZ_SSH', None)
105+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
106 manager.set_ssh_version_string("lsh")
107- self.assertIsInstance(manager.get_vendor({}), LSHSubprocessVendor)
108+ self.assertIsInstance(manager.get_vendor(), LSHSubprocessVendor)
109
110 def test_get_vendor_by_inspection_plink(self):
111 manager = TestSSHVendorManager()
112- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
113+ self.overrideEnv('BRZ_SSH', None)
114+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
115 manager.set_ssh_version_string("plink")
116 # Auto-detect of plink vendor disabled, on Windows recommended
117 # default ssh-client is paramiko
118 # see https://bugs.launchpad.net/bugs/414743
119- #~self.assertIsInstance(manager.get_vendor({}), PLinkSubprocessVendor)
120- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
121+ #~self.assertIsInstance(manager.get_vendor(), PLinkSubprocessVendor)
122+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
123
124 def test_cached_vendor(self):
125 manager = TestSSHVendorManager()
126- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
127+ self.overrideEnv('BRZ_SSH', None)
128+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
129 vendor = object()
130 manager.register_vendor("vendor", vendor)
131- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
132+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
133 # Once the vendor is found the result is cached (mainly because of the
134 # 'get_vendor' sometimes can be an expensive operation) and later
135 # invocations of the 'get_vendor' just returns the cached value.
136- self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)
137- self.assertIs(manager.get_vendor({}), vendor)
138+ self.overrideEnv('BRZ_SSH', 'vendor')
139+ self.assertIs(manager.get_vendor(), vendor)
140+ self.overrideEnv('BRZ_SSH', None)
141+ self.assertIs(manager.get_vendor(), vendor)
142 # The cache can be cleared by the 'clear_cache' method
143 manager.clear_cache()
144- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
145+ self.overrideEnv('BRZ_SSH', None)
146+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
147
148 def test_get_vendor_search_order(self):
149 # The 'get_vendor' method search for SSH vendors as following:
150@@ -118,33 +141,37 @@
151
152 manager = TestSSHVendorManager()
153 # At first no vendors are found
154- self.assertRaises(SSHVendorNotFound, manager.get_vendor, {})
155+ self.overrideEnv('BRZ_SSH', None)
156+ self.assertRaises(SSHVendorNotFound, manager.get_vendor)
157
158 # If the default vendor is registered it will be returned
159 default_vendor = object()
160 manager.register_default_vendor(default_vendor)
161- self.assertIs(manager.get_vendor({}), default_vendor)
162+ self.assertIs(manager.get_vendor(), default_vendor)
163
164 # If the known vendor is found in the system it will be returned
165 manager.clear_cache()
166 manager.set_ssh_version_string("OpenSSH")
167- self.assertIsInstance(manager.get_vendor({}), OpenSSHSubprocessVendor)
168+ self.assertIsInstance(manager.get_vendor(), OpenSSHSubprocessVendor)
169
170 # If the BRZ_SSH environment variable is found it will be treated as
171 # the vendor name
172 manager.clear_cache()
173 vendor = object()
174 manager.register_vendor("vendor", vendor)
175- self.assertIs(manager.get_vendor({"BRZ_SSH": "vendor"}), vendor)
176+ self.overrideEnv('BRZ_SSH', 'vendor')
177+ self.assertIs(manager.get_vendor(), vendor)
178
179 # Last cached value always checked first
180- self.assertIs(manager.get_vendor({}), vendor)
181+ self.overrideEnv('BRZ_SSH', 'vendor')
182+ self.assertIs(manager.get_vendor(), vendor)
183
184 def test_get_vendor_from_path_win32_plink(self):
185 manager = TestSSHVendorManager()
186 manager.set_ssh_version_string("plink: Release 0.60")
187 plink_path = "C:/Program Files/PuTTY/plink.exe"
188- vendor = manager.get_vendor({"BRZ_SSH": plink_path})
189+ self.overrideEnv('BRZ_SSH', plink_path)
190+ vendor = manager.get_vendor()
191 self.assertIsInstance(vendor, PLinkSubprocessVendor)
192 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])
193 self.assertEqual(args[0], plink_path)
194@@ -154,7 +181,8 @@
195 manager.set_ssh_version_string(
196 "OpenSSH_5.1p1 Debian-5, OpenSSL, 0.9.8g 19 Oct 2007")
197 openssh_path = "/usr/bin/ssh"
198- vendor = manager.get_vendor({"BRZ_SSH": openssh_path})
199+ self.overrideEnv('BRZ_SSH', openssh_path)
200+ vendor = manager.get_vendor()
201 self.assertIsInstance(vendor, OpenSSHSubprocessVendor)
202 args = vendor._get_vendor_specific_argv("user", "host", 22, ["bzr"])
203 self.assertEqual(args[0], openssh_path)
204
205=== modified file 'breezy/transport/ssh.py'
206--- breezy/transport/ssh.py 2019-06-16 01:52:23 +0000
207+++ breezy/transport/ssh.py 2019-07-07 18:58:24 +0000
208@@ -80,16 +80,9 @@
209 """Clear previously cached lookup result."""
210 self._cached_ssh_vendor = None
211
212- def _get_vendor_by_environment(self, environment=None):
213- """Return the vendor or None based on BRZ_SSH environment variable.
214-
215- :raises UnknownSSH: if the BRZ_SSH environment variable contains
216- unknown vendor name
217- """
218- if environment is None:
219- environment = os.environ
220- if 'BRZ_SSH' in environment:
221- vendor_name = environment['BRZ_SSH']
222+ def _get_vendor_by_config(self):
223+ vendor_name = config.GlobalStack().get('ssh')
224+ if vendor_name is not None:
225 try:
226 vendor = self._ssh_vendors[vendor_name]
227 except KeyError:
228@@ -151,7 +144,7 @@
229 return self._get_vendor_by_version_string(version,
230 os.path.splitext(os.path.basename(path))[0])
231
232- def get_vendor(self, environment=None):
233+ def get_vendor(self):
234 """Find out what version of SSH is on the system.
235
236 :raises SSHVendorNotFound: if no any SSH vendor is found
237@@ -159,7 +152,7 @@
238 unknown vendor name
239 """
240 if self._cached_ssh_vendor is None:
241- vendor = self._get_vendor_by_environment(environment)
242+ vendor = self._get_vendor_by_config()
243 if vendor is None:
244 vendor = self._get_vendor_by_inspection()
245 if vendor is None:
246
247=== modified file 'doc/en/release-notes/brz-3.1.txt'
248--- doc/en/release-notes/brz-3.1.txt 2019-07-07 18:18:20 +0000
249+++ doc/en/release-notes/brz-3.1.txt 2019-07-07 18:58:24 +0000
250@@ -54,6 +54,9 @@
251 * Automatically upgrade to branch format 8 when setting branch references.
252 (Jelmer Vernooij)
253
254+* The ``ssh`` configuration variable can be used to set the default
255+ SSH implementation. (Jelmer Vernooij, #650757)
256+
257 * ``locks.steal_dead`` is now enabled by default.
258 (Jelmer Vernooij, #220464)
259

Subscribers

People subscribed via source and target branches