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

Proposed by Vincent Ladeuil on 2009-05-15
Status: Merged
Approved by: Ian Clatworthy on 2009-05-28
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 2009-05-15 Approve on 2009-05-26
Review via email: mp+6609@code.launchpad.net
To post a comment you must log in.
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.

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
lp:~vila/bzr/376582-auth-prompt updated on 2009-06-02
4370. By Vincent Ladeuil on 2009-06-02

Merge bzr.dev

4371. By Vincent Ladeuil on 2009-06-02

Fixed as per Ian's review.

4372. By Vincent Ladeuil on 2009-06-02

Fix plugin test too :-/

* bzrlib/plugins/launchpad/test_register.py:
(TestGatherUserCredentials.test_gather_user_credentials_prompts):prompt
is now displayed on stderr.

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."""