Merge lp:~jelmer/brz/stores-revno into lp:brz

Proposed by Jelmer Vernooij on 2019-06-16
Status: Merged
Approved by: Jelmer Vernooij on 2019-06-22
Approved revision: 7349
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/stores-revno
Merge into: lp:brz
Diff against target: 335 lines (+125/-30)
9 files modified
breezy/branch.py (+10/-1)
breezy/bzr/remote.py (+3/-0)
breezy/commit.py (+28/-15)
breezy/config.py (+10/-0)
breezy/git/branch.py (+17/-0)
breezy/git/tests/test_blackbox.py (+30/-0)
breezy/log.py (+11/-10)
breezy/tests/per_branch/test_branch.py (+6/-0)
doc/en/release-notes/brz-3.1.txt (+10/-4)
To merge this branch: bzr merge lp:~jelmer/brz/stores-revno
Reviewer Review Type Date Requested Status
Martin Packman 2019-06-16 Approve on 2019-06-21
Review via email: mp+368873@code.launchpad.net

Commit message

Add a ``calculate_revnos`` setting that determines when revision numbers are calculated.

Description of the change

Add a ``calculate_revnos`` setting that determines when revision numbers are calculated.

This setting is enabled by default, keeping the existing behaviour.

This only impacts log and commit (currently) for branch formats that don't
natively store revision numbers. Disabling calculate_revnos significantly
speeds up operations in git repositories with a lot of history.

For samba, it reduces the time it takes to run:

* commit from >10s to ~1.5s
* log from ~35s to ~22s

To post a comment you must log in.
Martin Packman (gz) wrote :

Looks reasonable, see one inline nit. I wonder if the default should just start as unset, so the behaviour does vary based on whether the format in question stores revnos.

review: Approve
lp:~jelmer/brz/stores-revno updated on 2019-06-22
7349. By Jelmer Vernooij on 2019-06-22

Fix test.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/branch.py'
2--- breezy/branch.py 2019-06-18 13:48:45 +0000
3+++ breezy/branch.py 2019-06-22 18:47:15 +0000
4@@ -1661,6 +1661,10 @@
5 """True if uncommitted changes can be stored in this branch."""
6 return True
7
8+ def stores_revno(self):
9+ """True if this branch format store revision numbers."""
10+ return True
11+
12
13 class BranchHooks(Hooks):
14 """A dictionary mapping hook name to a list of callables for branch hooks.
15@@ -2019,7 +2023,12 @@
16 tag_updates = getattr(self, "tag_updates", None)
17 if not is_quiet():
18 if self.old_revid != self.new_revid:
19- note(gettext('Pushed up to revision %d.') % self.new_revno)
20+ if self.new_revno is not None:
21+ note(gettext('Pushed up to revision %d.'),
22+ self.new_revno)
23+ else:
24+ note(gettext('Pushed up to revision id %s.'),
25+ self.new_revid.decode('utf-8'))
26 if tag_updates:
27 note(ngettext('%d tag updated.', '%d tags updated.',
28 len(tag_updates)) % len(tag_updates))
29
30=== modified file 'breezy/bzr/remote.py'
31--- breezy/bzr/remote.py 2019-06-18 11:21:15 +0000
32+++ breezy/bzr/remote.py 2019-06-22 18:47:15 +0000
33@@ -3398,6 +3398,9 @@
34 self._ensure_real()
35 return self._custom_format.supports_set_append_revisions_only()
36
37+ def stores_revno(self):
38+ return True
39+
40 def _use_default_local_heads_to_fetch(self):
41 # If the branch format is a metadir format *and* its heads_to_fetch
42 # implementation is not overridden vs the base class, we can use the
43
44=== modified file 'breezy/commit.py'
45--- breezy/commit.py 2019-06-15 15:59:17 +0000
46+++ breezy/commit.py 2019-06-22 18:47:15 +0000
47@@ -158,13 +158,16 @@
48 unescape_for_display(location, 'utf-8'))
49
50 def completed(self, revno, rev_id):
51- self._note(gettext('Committed revision %d.'), revno)
52- # self._note goes to the console too; so while we want to log the
53- # rev_id, we can't trivially only log it. (See bug 526425). Long
54- # term we should rearrange the reporting structure, but for now
55- # we just mutter seperately. We mutter the revid and revno together
56- # so that concurrent bzr invocations won't lead to confusion.
57- mutter('Committed revid %s as revno %d.', rev_id, revno)
58+ if revno is not None:
59+ self._note(gettext('Committed revision %d.'), revno)
60+ # self._note goes to the console too; so while we want to log the
61+ # rev_id, we can't trivially only log it. (See bug 526425). Long
62+ # term we should rearrange the reporting structure, but for now
63+ # we just mutter seperately. We mutter the revid and revno together
64+ # so that concurrent bzr invocations won't lead to confusion.
65+ mutter('Committed revid %s as revno %d.', rev_id, revno)
66+ else:
67+ self._note(gettext('Committed revid %s.'), rev_id)
68
69 def deleted(self, path):
70 self._note(gettext('deleted %s'), path)
71@@ -373,6 +376,9 @@
72 # Setup the bound branch variables as needed.
73 self._check_bound_branch(operation, possible_master_transports)
74
75+ if self.config_stack is None:
76+ self.config_stack = self.work_tree.get_config_stack()
77+
78 # Check that the working tree is up to date
79 old_revno, old_revid, new_revno = self._check_out_of_date_tree()
80
81@@ -381,8 +387,6 @@
82 self.reporter = reporter
83 elif self.reporter is None:
84 self.reporter = self._select_reporter()
85- if self.config_stack is None:
86- self.config_stack = self.work_tree.get_config_stack()
87
88 # Setup the progress bar. As the number of files that need to be
89 # committed in unknown, progress is reported as stages.
90@@ -494,6 +498,9 @@
91 self.branch.fetch(self.master_branch, self.rev_id)
92
93 # and now do the commit locally.
94+ if new_revno is None:
95+ # Keep existing behaviour around ghosts
96+ new_revno = 1
97 self.branch.set_last_revision_info(new_revno, self.rev_id)
98 else:
99 try:
100@@ -587,19 +594,25 @@
101 # - in a checkout scenario the tree may have no
102 # parents but the branch may do.
103 first_tree_parent = breezy.revision.NULL_REVISION
104- try:
105- old_revno, master_last = self.master_branch.last_revision_info()
106- except errors.UnsupportedOperation:
107+ if (self.master_branch._format.stores_revno() or
108+ self.config_stack.get('calculate_revnos')):
109+ try:
110+ old_revno, master_last = self.master_branch.last_revision_info()
111+ except errors.UnsupportedOperation:
112+ master_last = self.master_branch.last_revision()
113+ old_revno = self.branch.revision_id_to_revno(master_last)
114+ else:
115 master_last = self.master_branch.last_revision()
116- old_revno = self.branch.revision_id_to_revno(master_last)
117+ old_revno = None
118 if master_last != first_tree_parent:
119 if master_last != breezy.revision.NULL_REVISION:
120 raise errors.OutOfDateTree(self.work_tree)
121- if self.branch.repository.has_revision(first_tree_parent):
122+ if (old_revno is not None and
123+ self.branch.repository.has_revision(first_tree_parent)):
124 new_revno = old_revno + 1
125 else:
126 # ghost parents never appear in revision history.
127- new_revno = 1
128+ new_revno = None
129 return old_revno, master_last, new_revno
130
131 def _process_pre_hooks(self, old_revno, new_revno):
132
133=== modified file 'breezy/config.py'
134--- breezy/config.py 2019-06-16 01:03:51 +0000
135+++ breezy/config.py 2019-06-22 18:47:15 +0000
136@@ -2482,6 +2482,16 @@
137 bug tracker was specified.
138 '''))
139 option_registry.register(
140+ Option('calculate_revnos', default=True,
141+ from_unicode=bool_from_store,
142+ help='''\
143+Calculate revision numbers if they are not known.
144+
145+Always show revision numbers, even for branch formats that don't store them
146+natively (such as Git). Calculating the revision number requires traversing
147+the left hand ancestry of the branch and can be slow on very large branches.
148+'''))
149+option_registry.register(
150 Option('check_signatures', default=CHECK_IF_POSSIBLE,
151 from_unicode=signature_policy_from_unicode,
152 help='''\
153
154=== modified file 'breezy/git/branch.py'
155--- breezy/git/branch.py 2019-06-18 13:48:45 +0000
156+++ breezy/git/branch.py 2019-06-22 18:47:15 +0000
157@@ -82,12 +82,21 @@
158 )
159
160
161+def _calculate_revnos(branch):
162+ if branch._format.stores_revno():
163+ return True
164+ config = branch.get_config_stack()
165+ return config.get('calculate_revnos')
166+
167+
168 class GitPullResult(branch.PullResult):
169 """Result of a pull from a Git branch."""
170
171 def _lookup_revno(self, revid):
172 if not isinstance(revid, bytes):
173 raise TypeError(revid)
174+ if not _calculate_revnos(self.target_branch):
175+ return None
176 # Try in source branch first, it'll be faster
177 with self.target_branch.lock_read():
178 return self.target_branch.revision_id_to_revno(revid)
179@@ -324,6 +333,10 @@
180 def set_reference(self, controldir, name, target):
181 return controldir.set_branch_reference(target, name)
182
183+ def stores_revno(self):
184+ """True if this branch format store revision numbers."""
185+ return False
186+
187
188 class LocalGitBranchFormat(GitBranchFormat):
189
190@@ -808,6 +821,8 @@
191 raise TypeError(revid)
192 # Try in source branch first, it'll be faster
193 with local_branch.lock_read():
194+ if not _calculate_revnos(local_branch):
195+ return None
196 try:
197 return local_branch.revision_id_to_revno(revid)
198 except errors.NoSuchRevision:
199@@ -816,6 +831,8 @@
200 return graph.find_distance_to_null(
201 revid, [(revision.NULL_REVISION, 0)])
202 except errors.GhostRevisionsHaveNoRevno:
203+ if not _calculate_revnos(remote_branch):
204+ return None
205 # FIXME: Check using graph.find_distance_to_null() ?
206 with remote_branch.lock_read():
207 return remote_branch.revision_id_to_revno(revid)
208
209=== modified file 'breezy/git/tests/test_blackbox.py'
210--- breezy/git/tests/test_blackbox.py 2019-06-18 17:32:43 +0000
211+++ breezy/git/tests/test_blackbox.py 2019-06-22 18:47:15 +0000
212@@ -151,6 +151,17 @@
213 self.assertEqual(b"", output)
214 self.assertTrue(error.endswith(b"Created new branch.\n"))
215
216+ def test_push_without_calculate_revnos(self):
217+ self.run_bzr(['init', '--git', 'bla'])
218+ self.run_bzr(['init', '--git', 'foo'])
219+ self.run_bzr(['commit', '--unchanged', '-m', 'bla', 'foo'])
220+ output, error = self.run_bzr(
221+ ['push', '-Ocalculate_revnos=no', '-d', 'foo', 'bla'])
222+ self.assertEqual("", output)
223+ self.assertContainsRe(
224+ error,
225+ 'Pushed up to revision id git(.*).\n')
226+
227 def test_push_lossy_non_mainline(self):
228 self.run_bzr(['init', '--git', 'bla'])
229 self.run_bzr(['init', 'foo'])
230@@ -185,6 +196,25 @@
231
232 # Check that bzr log does not fail and includes the revision.
233 output, error = self.run_bzr(['log', '-v'])
234+ self.assertContainsRe(output, 'revno: 1')
235+
236+ def test_log_without_revno(self):
237+ # Smoke test for "bzr log -v" in a git repository.
238+ self.simple_commit()
239+
240+ # Check that bzr log does not fail and includes the revision.
241+ output, error = self.run_bzr(['log', '-Ocalculate_revnos=no'])
242+ self.assertNotContainsRe(output, 'revno: 1')
243+
244+ def test_commit_without_revno(self):
245+ repo = GitRepo.init(self.test_dir)
246+ output, error = self.run_bzr(
247+ ['commit', '-Ocalculate_revnos=yes', '--unchanged', '-m', 'one'])
248+ self.assertContainsRe(error, 'Committed revision 1.')
249+ output, error = self.run_bzr(
250+ ['commit', '-Ocalculate_revnos=no', '--unchanged', '-m', 'two'])
251+ self.assertNotContainsRe(error, 'Committed revision 2.')
252+ self.assertContainsRe(error, 'Committed revid .*.')
253
254 def test_tags(self):
255 git_repo, commit_sha1 = self.simple_commit()
256
257=== modified file 'breezy/log.py'
258--- breezy/log.py 2019-06-06 23:06:25 +0000
259+++ breezy/log.py 2019-06-22 18:47:15 +0000
260@@ -733,13 +733,19 @@
261 repo = branch.repository
262 graph = repo.get_graph()
263 if start_rev_id is None and end_rev_id is None:
264- try:
265- br_revno, br_rev_id = branch.last_revision_info()
266- except errors.GhostRevisionsHaveNoRevno:
267+ if branch._format.stores_revno() or \
268+ config.GlobalStack().get('calculate_revnos'):
269+ try:
270+ br_revno, br_rev_id = branch.last_revision_info()
271+ except errors.GhostRevisionsHaveNoRevno:
272+ br_rev_id = branch.last_revision()
273+ cur_revno = None
274+ else:
275+ cur_revno = br_revno
276+ else:
277 br_rev_id = branch.last_revision()
278 cur_revno = None
279- else:
280- cur_revno = br_revno
281+
282 graph_iter = graph.iter_lefthand_ancestry(br_rev_id,
283 (_mod_revision.NULL_REVISION,))
284 while True:
285@@ -1091,11 +1097,6 @@
286 raise TypeError(start_revision)
287 end_rev_id = end_revision.rev_id
288 end_revno = end_revision.revno
289- if end_revno is None:
290- try:
291- end_revno = branch.revno()
292- except errors.GhostRevisionsHaveNoRevno:
293- end_revno = None
294
295 if branch.last_revision() != _mod_revision.NULL_REVISION:
296 if (start_rev_id == _mod_revision.NULL_REVISION
297
298=== modified file 'breezy/tests/per_branch/test_branch.py'
299--- breezy/tests/per_branch/test_branch.py 2019-03-04 05:17:48 +0000
300+++ breezy/tests/per_branch/test_branch.py 2019-06-22 18:47:15 +0000
301@@ -1131,3 +1131,9 @@
302 self.bind(branch, tree.branch)
303 unshelver = branch.get_unshelver(tree)
304 self.assertIsNot(None, unshelver)
305+
306+
307+class TestFormatMetadata(per_branch.TestCaseWithBranch):
308+
309+ def test_stores_revno(self):
310+ self.assertIn(self.branch_format.stores_revno(), (True, False))
311
312=== modified file 'doc/en/release-notes/brz-3.1.txt'
313--- doc/en/release-notes/brz-3.1.txt 2019-06-17 23:01:58 +0000
314+++ doc/en/release-notes/brz-3.1.txt 2019-06-22 18:47:15 +0000
315@@ -30,10 +30,16 @@
316 * The 'quilt' plugin, extracted from brz-debian, is now
317 bundled. (Jelmer Vernooij)
318
319-* Directly read mtab rather than using psutil when trying to figure out
320- filesystem types. This removes a dependency that not all users may
321- have installed and speeds up import time since psutil brings in
322- various other modules. (Jelmer Vernooij)
323+ * A new ``calculate_revnos`` configuration option (defaults to enabled)
324+ can be used to disable revno display for branch formats that
325+ do not natively store revnos. This speeds up ``brz log`` on
326+ the Samba git branch by 33%.
327+ (Jelmer Vernooij)
328+
329+ * Directly read mtab rather than using psutil when trying to figure out
330+ filesystem types. This removes a dependency that not all users may
331+ have installed and speeds up import time since psutil brings in
332+ various other modules. (Jelmer Vernooij)
333
334 Improvements
335 ************

Subscribers

People subscribed via source and target branches