Bazaar

Merge lp:~mbp/bzr/430529-extension-warnings into lp:bzr

  1. 430529-extension-warnings
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Merged
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/430529-extension-warnings
Merge into: lp:bzr
Diff against target: None lines
To merge this branch: bzr merge lp:~mbp/bzr/430529-extension-warnings
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Robert Collins (community) Approve
Review via email: mp+11950@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

Not broken in 2.0, therefore not proposed for it.

You now get just this:

mbp@lithe% ./bzr st
default2a plugin set default format to 2a
unknown:
  bzrlib/tmp.diff
bzr: warning: some compiled extensions could not be loaded; see <https://answers.launchpad.net/bzr/+faq/703>

Revision history for this message
Robert Collins (lifeless) wrote :

> You now get just this:
>
> mbp@lithe% ./bzr st
> default2a plugin set default format to 2a
> unknown:
> bzrlib/tmp.diff
> bzr: warning: some compiled extensions could not be loaded; see <https://answers.launchpad.net/bzr/+faq/703>

That looks a lot nicer. Using a FAQ is ok, but it might be nicer to make
a small help topic for it instead; that way it will be present on the
users machine even if they are offline.

 review +1

review: Approve
Revision history for this message
Vincent Ladeuil (vila) :
review: Approve
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> Review: Approve
>> You now get just this:
>>
>> mbp@lithe% ./bzr st
>> default2a plugin set default format to 2a
>> unknown:
>> bzrlib/tmp.diff
>> bzr: warning: some compiled extensions could not be loaded; see <https://answers.launchpad.net/bzr/+faq/703>
>
> That looks a lot nicer. Using a FAQ is ok, but it might be nicer to make
> a small help topic for it instead; that way it will be present on the
> users machine even if they are offline.
>
> review +1
>

Aren't you missing a:
trace.mutter('Some extensions failed to load:\n%s'
             % ('\n '.join(_extension_load_failures,))

At least, in the FAQ you say that the failures are in ~/.bzr.log. Maybe
they are there when they are first imported? It might be nice to have a
simple summary at the end.

I agree that it would probably be better as a help topic.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqyKykACgkQJdeBCYSNAANRxgCeLEPSAHcNZ9uhjjVIly7sualI
mSAAn0QGSB6qEmtN2XAQebdpfJNAJAtE
=gJTX
-----END PGP SIGNATURE-----

Revision history for this message
Martin Pool (mbp) wrote :

2009/9/17 John A Meinel <email address hidden>:
> Aren't you missing a:
> trace.mutter('Some extensions failed to load:\n%s'
>             % ('\n    '.join(_extension_load_failures,))
>
> At least, in the FAQ you say that the failures are in ~/.bzr.log. Maybe
> they are there when they are first imported? It might be nice to have a
> simple summary at the end.

That's done at the time the exception originally occurs.

> I agree that it would probably be better as a help topic.

You may be right. I put it here for a couple of reasons:

 - A help topic about 'c extensions' seems like it out to have a
larger scope than just how to deal with this warning and I didn't want
to write that today :-)
 - In general I'm wondering if advice to users should be moved onto the web

--
Martin <http://launchpad.net/~mbp/>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-09-16 10:29:18 +0000
3+++ NEWS 2009-09-17 06:49:43 +0000
4@@ -68,7 +68,7 @@
5 * Make sure that we unlock the tree if we fail to create a TreeTransform
6 object when doing a merge, and there is limbo, or pending-deletions
7 directory. (Gary van der Merwe, #427773)
8-
9+
10 * Prevent some kinds of incomplete data from being committed to a 2a
11 repository, such as revisions without inventories or inventories without
12 chk_bytes root records.
13@@ -83,7 +83,8 @@
14 running using pure-Python versions, but this may be substantially
15 slower. The warning can be disabled by setting
16 ``ignore_missing_extensions = True`` in ``bazaar.conf``.
17- (Martin Pool, #406113)
18+ See also <https://answers.launchpad.net/bzr/+faq/703>.
19+ (Martin Pool, #406113, #430529)
20
21 Documentation
22 *************
23
24=== modified file 'bzrlib/osutils.py'
25--- bzrlib/osutils.py 2009-09-11 06:39:56 +0000
26+++ bzrlib/osutils.py 2009-09-17 06:49:43 +0000
27@@ -920,12 +920,13 @@
28 if GlobalConfig().get_user_option_as_bool('ignore_missing_extensions'):
29 return
30 # the warnings framework should by default show this only once
31- warnings.warn(
32- "bzr: warning: Failed to load compiled extensions:\n"
33- " %s\n"
34- " Bazaar can run, but performance may be reduced.\n"
35- " Check Bazaar is correctly installed or set ignore_missing_extensions"
36- % '\n '.join(_extension_load_failures,))
37+ from bzrlib.trace import warning
38+ warning(
39+ "bzr: warning: some compiled extensions could not be loaded; "
40+ "see <https://answers.launchpad.net/bzr/+faq/703>")
41+ # we no longer show the specific missing extensions here, because it makes
42+ # the message too long and scary - see
43+ # https://bugs.launchpad.net/bzr/+bug/430529
44
45
46 try:
47
48=== modified file 'bzrlib/tests/test_osutils.py'
49--- bzrlib/tests/test_osutils.py 2009-09-11 06:36:50 +0000
50+++ bzrlib/tests/test_osutils.py 2009-09-17 06:49:43 +0000
51@@ -29,6 +29,7 @@
52 errors,
53 osutils,
54 tests,
55+ trace,
56 win32utils,
57 )
58 from bzrlib.tests import (
59@@ -1824,7 +1825,19 @@
60 self.assertEquals(osutils._extension_load_failures[0],
61 "No module named _fictional_extension_py")
62
63- def test_report_extension_load_failures(self):
64+ def test_report_extension_load_failures_no_warning(self):
65 self.assertTrue(self._try_loading())
66 warnings, result = self.callCatchWarnings(osutils.report_extension_load_failures)
67- self.assertLength(1, warnings)
68+ # it used to give a Python warning; it no longer does
69+ self.assertLength(0, warnings)
70+
71+ def test_report_extension_load_failures_message(self):
72+ log = StringIO()
73+ trace.push_log_file(log)
74+ self.assertTrue(self._try_loading())
75+ osutils.report_extension_load_failures()
76+ self.assertContainsRe(
77+ log.getvalue(),
78+ r"bzr: warning: some compiled extensions could not be loaded; "
79+ "see <https://answers\.launchpad\.net/bzr/\+faq/703>\n"
80+ )