Bazaar

Merge lp:~vila/bzr/376582-auth-prompt into lp:~bzr/bzr/trunk-old

  1. 376582-auth-prompt
  2. Merge into trunk-old
Proposed by Vincent Ladeuil
Status: Merged
Approved by: Ian Clatworthy
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~vila/bzr/376582-auth-prompt
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 277 lines
To merge this branch: bzr merge lp:~vila/bzr/376582-auth-prompt
Reviewer Review Type Date Requested Status
Ian Clatworthy Approve
Review via email: mp+6609@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

There is no good reason to use stdout for prompting the user. Bug #376582 raised the issue that doing that forbid the redirection of command outputs.

This patch use stderr instead and should address the issue.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

I suspect this change ought to be mentioned as a Compatibility break, not just a bug fix, in NEWS. When landing, make sure you tweak NEWS to put this in 1.16 (not 1.15) as well. :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-06-02 01:37:30 +0000
3+++ NEWS 2009-06-02 14:35:20 +0000
4@@ -12,6 +12,11 @@
5 Compatibility Breaks
6 ********************
7
8+* Display prompt on stderr (instead of stdout) when querying users so
9+ that the output of commands can be safely redirected.
10+ (Vincent Ladeuil, #376582)
11+
12+
13 New Features
14 ************
15
16
17=== modified file 'bzrlib/plugins/launchpad/test_register.py'
18--- bzrlib/plugins/launchpad/test_register.py 2009-03-23 14:59:43 +0000
19+++ bzrlib/plugins/launchpad/test_register.py 2009-06-02 14:35:20 +0000
20@@ -335,12 +335,14 @@
21 g_conf = config.GlobalConfig()
22 g_conf.set_user_option('email', 'Test User <test@user.com>')
23 stdout = tests.StringIOWrapper()
24+ stderr = tests.StringIOWrapper()
25 ui.ui_factory = tests.TestUIFactory(stdin='userpass\n',
26- stdout=stdout)
27+ stdout=stdout, stderr=stderr)
28 self.assertIs(None, service.registrant_password)
29 service.gather_user_credentials()
30 self.assertEqual('test@user.com', service.registrant_email)
31 self.assertEqual('userpass', service.registrant_password)
32- self.assertContainsRe(stdout.getvalue(),
33+ self.assertEquals('', stdout.getvalue())
34+ self.assertContainsRe(stderr.getvalue(),
35 'launchpad.net password for test@user\\.com')
36
37
38=== modified file 'bzrlib/tests/test_config.py'
39--- bzrlib/tests/test_config.py 2009-04-27 16:10:10 +0000
40+++ bzrlib/tests/test_config.py 2009-06-02 14:35:20 +0000
41@@ -1461,7 +1461,8 @@
42 """Test AuthenticationConfig behaviour"""
43
44 def _check_default_password_prompt(self, expected_prompt_format, scheme,
45- host=None, port=None, realm=None, path=None):
46+ host=None, port=None, realm=None,
47+ path=None):
48 if host is None:
49 host = 'bar.org'
50 user, password = 'jim', 'precious'
51@@ -1470,17 +1471,20 @@
52 'user': user, 'realm': realm}
53
54 stdout = tests.StringIOWrapper()
55+ stderr = tests.StringIOWrapper()
56 ui.ui_factory = tests.TestUIFactory(stdin=password + '\n',
57- stdout=stdout)
58+ stdout=stdout, stderr=stderr)
59 # We use an empty conf so that the user is always prompted
60 conf = config.AuthenticationConfig()
61 self.assertEquals(password,
62 conf.get_password(scheme, host, user, port=port,
63 realm=realm, path=path))
64- self.assertEquals(stdout.getvalue(), expected_prompt)
65+ self.assertEquals(expected_prompt, stderr.getvalue())
66+ self.assertEquals('', stdout.getvalue())
67
68 def _check_default_username_prompt(self, expected_prompt_format, scheme,
69- host=None, port=None, realm=None, path=None):
70+ host=None, port=None, realm=None,
71+ path=None):
72 if host is None:
73 host = 'bar.org'
74 username = 'jim'
75@@ -1488,13 +1492,15 @@
76 'scheme': scheme, 'host': host, 'port': port,
77 'realm': realm}
78 stdout = tests.StringIOWrapper()
79+ stderr = tests.StringIOWrapper()
80 ui.ui_factory = tests.TestUIFactory(stdin=username+ '\n',
81- stdout=stdout)
82+ stdout=stdout, stderr=stderr)
83 # We use an empty conf so that the user is always prompted
84 conf = config.AuthenticationConfig()
85 self.assertEquals(username, conf.get_user(scheme, host, port=port,
86 realm=realm, path=path, ask=True))
87- self.assertEquals(stdout.getvalue(), expected_prompt)
88+ self.assertEquals(expected_prompt, stderr.getvalue())
89+ self.assertEquals('', stdout.getvalue())
90
91 def test_username_defaults_prompts(self):
92 # HTTP prompts can't be tested here, see test_http.py
93
94=== modified file 'bzrlib/tests/test_http.py'
95--- bzrlib/tests/test_http.py 2009-05-27 13:59:47 +0000
96+++ bzrlib/tests/test_http.py 2009-06-02 14:35:20 +0000
97@@ -1569,15 +1569,18 @@
98 self.server.add_user('joe', 'foo')
99 t = self.get_user_transport(None, None)
100 stdout = tests.StringIOWrapper()
101- ui.ui_factory = tests.TestUIFactory(stdin='joe\nfoo\n', stdout=stdout)
102+ stderr = tests.StringIOWrapper()
103+ ui.ui_factory = tests.TestUIFactory(stdin='joe\nfoo\n',
104+ stdout=stdout, stderr=stderr)
105 self.assertEqual('contents of a\n',t.get('a').read())
106 # stdin should be empty
107 self.assertEqual('', ui.ui_factory.stdin.readline())
108- stdout.seek(0)
109+ stderr.seek(0)
110 expected_prompt = self._expected_username_prompt(t._unqualified_scheme)
111- self.assertEquals(expected_prompt, stdout.read(len(expected_prompt)))
112+ self.assertEquals(expected_prompt, stderr.read(len(expected_prompt)))
113+ self.assertEquals('', stdout.getvalue())
114 self._check_password_prompt(t._unqualified_scheme, 'joe',
115- stdout.readline())
116+ stderr.readline())
117
118 def test_prompt_for_password(self):
119 if self._testing_pycurl():
120@@ -1588,12 +1591,15 @@
121 self.server.add_user('joe', 'foo')
122 t = self.get_user_transport('joe', None)
123 stdout = tests.StringIOWrapper()
124- ui.ui_factory = tests.TestUIFactory(stdin='foo\n', stdout=stdout)
125- self.assertEqual('contents of a\n',t.get('a').read())
126+ stderr = tests.StringIOWrapper()
127+ ui.ui_factory = tests.TestUIFactory(stdin='foo\n',
128+ stdout=stdout, stderr=stderr)
129+ self.assertEqual('contents of a\n', t.get('a').read())
130 # stdin should be empty
131 self.assertEqual('', ui.ui_factory.stdin.readline())
132 self._check_password_prompt(t._unqualified_scheme, 'joe',
133- stdout.getvalue())
134+ stderr.getvalue())
135+ self.assertEquals('', stdout.getvalue())
136 # And we shouldn't prompt again for a different request
137 # against the same transport.
138 self.assertEqual('contents of b\n',t.get('b').read())
139
140=== modified file 'bzrlib/tests/test_ui.py'
141--- bzrlib/tests/test_ui.py 2009-04-24 13:30:48 +0000
142+++ bzrlib/tests/test_ui.py 2009-06-02 14:35:20 +0000
143@@ -67,15 +67,17 @@
144 self.assertEqual('', stdout.getvalue())
145
146 def test_text_factory_ascii_password(self):
147- ui = TestUIFactory(stdin='secret\n', stdout=StringIOWrapper())
148+ ui = TestUIFactory(stdin='secret\n', stdout=StringIOWrapper(),
149+ stderr=StringIOWrapper())
150 pb = ui.nested_progress_bar()
151 try:
152 self.assertEqual('secret',
153 self.apply_redirected(ui.stdin, ui.stdout,
154- ui.stdout,
155+ ui.stderr,
156 ui.get_password))
157 # ': ' is appended to prompt
158- self.assertEqual(': ', ui.stdout.getvalue())
159+ self.assertEqual(': ', ui.stderr.getvalue())
160+ self.assertEqual('', ui.stdout.readline())
161 # stdin should be empty
162 self.assertEqual('', ui.stdin.readline())
163 finally:
164@@ -88,21 +90,22 @@
165 it to utf8 to test that we transport the password correctly.
166 """
167 ui = TestUIFactory(stdin=u'baz\u1234'.encode('utf8'),
168- stdout=StringIOWrapper())
169- ui.stdin.encoding = 'utf8'
170- ui.stdout.encoding = ui.stdin.encoding
171+ stdout=StringIOWrapper(),
172+ stderr=StringIOWrapper())
173+ ui.stderr.encoding = ui.stdout.encoding = ui.stdin.encoding = 'utf8'
174 pb = ui.nested_progress_bar()
175 try:
176- password = self.apply_redirected(ui.stdin, ui.stdout, ui.stdout,
177+ password = self.apply_redirected(ui.stdin, ui.stdout, ui.stderr,
178 ui.get_password,
179 u'Hello \u1234 %(user)s',
180 user=u'some\u1234')
181 # We use StringIO objects, we need to decode them
182 self.assertEqual(u'baz\u1234', password.decode('utf8'))
183 self.assertEqual(u'Hello \u1234 some\u1234: ',
184- ui.stdout.getvalue().decode('utf8'))
185- # stdin should be empty
186+ ui.stderr.getvalue().decode('utf8'))
187+ # stdin and stdout should be empty
188 self.assertEqual('', ui.stdin.readline())
189+ self.assertEqual('', ui.stdout.readline())
190 finally:
191 pb.finished()
192
193@@ -193,6 +196,7 @@
194 "yes\nn\nnot an answer\n"
195 "no\nfoo\n")
196 factory.stdout = StringIO()
197+ factory.stderr = StringIO()
198 # there is no output from the base factory
199 self.assertEqual(True, factory.get_boolean(""))
200 self.assertEqual(True, factory.get_boolean(""))
201@@ -223,8 +227,10 @@
202
203 def test_text_factory_prompt(self):
204 # see <https://launchpad.net/bugs/365891>
205- factory = TextUIFactory(None, StringIO(), StringIO())
206+ factory = TextUIFactory(None, StringIO(), StringIO(), StringIO())
207 factory.prompt('foo %2e')
208+ self.assertEqual('', factory.stdout.getvalue())
209+ self.assertEqual('foo %2e', factory.stderr.getvalue())
210
211 def test_text_factory_prompts_and_clears(self):
212 # a get_boolean call should clear the pb before prompting
213@@ -263,37 +269,41 @@
214 factory = SilentUIFactory()
215 factory.stdin = StringIO("someuser\n\n")
216 factory.stdout = StringIO()
217- self.assertEquals(None,
218+ factory.stderr = StringIO()
219+ self.assertEquals(None,
220 factory.get_username(u'Hello\u1234 %(host)s', host=u'some\u1234'))
221 self.assertEquals("", factory.stdout.getvalue())
222+ self.assertEquals("", factory.stderr.getvalue())
223 self.assertEquals("someuser\n\n", factory.stdin.getvalue())
224
225 def test_text_ui_getusername(self):
226 factory = TextUIFactory(None, None, None)
227 factory.stdin = StringIO("someuser\n\n")
228 factory.stdout = StringIO()
229+ factory.stderr = StringIO()
230 factory.stdout.encoding = "utf8"
231 # there is no output from the base factory
232- self.assertEqual("someuser",
233- factory.get_username('Hello %(host)s', host='some'))
234- self.assertEquals("Hello some: ", factory.stdout.getvalue())
235+ self.assertEqual("someuser",
236+ factory.get_username('Hello %(host)s', host='some'))
237+ self.assertEquals("Hello some: ", factory.stderr.getvalue())
238+ self.assertEquals('', factory.stdout.getvalue())
239 self.assertEqual("", factory.get_username("Gebruiker"))
240 # stdin should be empty
241 self.assertEqual('', factory.stdin.readline())
242
243 def test_text_ui_getusername_utf8(self):
244 ui = TestUIFactory(stdin=u'someuser\u1234'.encode('utf8'),
245- stdout=StringIOWrapper())
246- ui.stdin.encoding = "utf8"
247- ui.stdout.encoding = ui.stdin.encoding
248+ stdout=StringIOWrapper(), stderr=StringIOWrapper())
249+ ui.stderr.encoding = ui.stdout.encoding = ui.stdin.encoding = "utf8"
250 pb = ui.nested_progress_bar()
251 try:
252 # there is no output from the base factory
253- username = self.apply_redirected(ui.stdin, ui.stdout, ui.stdout,
254+ username = self.apply_redirected(ui.stdin, ui.stdout, ui.stderr,
255 ui.get_username, u'Hello\u1234 %(host)s', host=u'some\u1234')
256 self.assertEquals(u"someuser\u1234", username.decode('utf8'))
257- self.assertEquals(u"Hello\u1234 some\u1234: ",
258- ui.stdout.getvalue().decode("utf8"))
259+ self.assertEquals(u"Hello\u1234 some\u1234: ",
260+ ui.stderr.getvalue().decode("utf8"))
261+ self.assertEquals('', ui.stdout.getvalue())
262 finally:
263 pb.finished()
264
265
266=== modified file 'bzrlib/ui/__init__.py'
267--- bzrlib/ui/__init__.py 2009-04-24 15:49:33 +0000
268+++ bzrlib/ui/__init__.py 2009-06-02 14:35:20 +0000
269@@ -227,7 +227,7 @@
270 prompt = prompt % kwargs
271 prompt = prompt.encode(osutils.get_terminal_encoding(), 'replace')
272 self.clear_term()
273- self.stdout.write(prompt)
274+ self.stderr.write(prompt)
275
276 def note(self, msg):
277 """Write an already-formatted message."""