Bazaar

Merge lp:~mbp/bzr/456077-cross-format-fetch into lp:bzr/2.0

  1. 456077-cross-format-fetch
  2. Merge into 2.0
Proposed by Martin Pool
Status: Merged
Approved by: Andrew Bennetts
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/456077-cross-format-fetch
Merge into: lp:bzr/2.0
Diff against target: 69 lines (+27/-1)
3 files modified
NEWS (+6/-0)
bzrlib/repository.py (+11/-1)
bzrlib/tests/blackbox/test_pull.py (+10/-0)
To merge this branch: bzr merge lp:~mbp/bzr/456077-cross-format-fetch
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Review via email: mp+17200@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

This is a partial fix for bug 456077, handling only the local-local case, so that you get a clean warning about transfers that are likely to be slow. Suggestions of improved phrasing welcome.

I will do a follow up for the streaming case.

This is proposed for and based off 2.0. This is a real bug and does cause some pain, but tbh I'm not sure if it's a sufficiently serious bug to be worth changing it. What do you think?

Revision history for this message
Andrew Bennetts (spiv) wrote :

"... other things using streaming"

I think that comment would be clearer as "... other fetches use streaming"

I think the message is ok. I'd be tempted to put the "from %s to %s" in parentheses because I think that feels better grammar-wise, but less punctuation is probably preferable... so, fine as is.

I think this is ok for 2.0. The major risk I suppose is false positives... but I think if serializers differ it's safe to assume there's at least some performance penalty, so I think this is ok. e.g. pack-0.92 -> 1.6 shouldn't warn, but 1.14 -> 2a should, and I think this patch will get that right.

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 2010-01-12 01:44:13 +0000
3+++ NEWS 2010-01-12 06:21:14 +0000
4@@ -41,6 +41,12 @@
5 * Give a clearer message if the lockdir disappears after being apparently
6 successfully taken. (Martin Pool, #498378)
7
8+* Give a warning when fetching between local repositories with
9+ sufficiently different formats that the content will need to be
10+ serialized (ie ``InterDifferingSerializer``) so the user has a clue that
11+ upgrading could make it faster.
12+ (Martin Pool, #456077)
13+
14 * If we fail to open ``~/.bzr.log`` write a clear message to stderr rather
15 than using ``warning()``. The log file is opened before logging is set
16 up, and it leads to very confusing: 'no handlers for "bzr"' messages for
17
18=== modified file 'bzrlib/repository.py'
19--- bzrlib/repository.py 2009-09-03 02:04:04 +0000
20+++ bzrlib/repository.py 2010-01-12 06:21:14 +0000
21@@ -1,4 +1,4 @@
22-# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
23+# Copyright (C) 2005-2010 Canonical Ltd
24 #
25 # This program is free software; you can redistribute it and/or modify
26 # it under the terms of the GNU General Public License as published by
27@@ -39,6 +39,7 @@
28 osutils,
29 revision as _mod_revision,
30 symbol_versioning,
31+ trace,
32 tsort,
33 ui,
34 versionedfile,
35@@ -3979,6 +3980,15 @@
36 """See InterRepository.fetch()."""
37 if fetch_spec is not None:
38 raise AssertionError("Not implemented yet...")
39+ # See <https://launchpad.net/bugs/456077> asking for a warning here
40+ #
41+ # nb this is only active for local-local fetches; other things using
42+ # streaming.
43+ trace.warning("Fetching between repositories with different formats\n"
44+ "from %s to %s.\n"
45+ "This may take some time. Upgrade the branches to the same format \n"
46+ "for better results.\n"
47+ % (self.source._format, self.target._format))
48 if (not self.source.supports_rich_root()
49 and self.target.supports_rich_root()):
50 self._converting_to_rich_root = True
51
52=== modified file 'bzrlib/tests/blackbox/test_pull.py'
53--- bzrlib/tests/blackbox/test_pull.py 2009-06-15 06:47:14 +0000
54+++ bzrlib/tests/blackbox/test_pull.py 2010-01-12 06:21:14 +0000
55@@ -390,4 +390,14 @@
56 self.assertLength(18, self.hpss_calls)
57 remote = Branch.open('stacked')
58 self.assertEndsWith(remote.get_stacked_on_url(), '/parent')
59+
60+ def test_pull_cross_format_warning(self):
61+ """You get a warning for probably slow cross-format pulls.
62+ """
63
64+ from_tree = self.make_branch_and_tree('from', format='2a')
65+ to_tree = self.make_branch_and_tree('to', format='1.14-rich-root')
66+ from_tree.commit(message='first commit')
67+ out, err = self.run_bzr(['pull', '-d', 'to', 'from'])
68+ self.assertContainsRe(err,
69+ "(?m)Fetching between repositories with different formats.*")

Subscribers

People subscribed via source and target branches