1075274 - Update single_head_per_branch hook for 3.1 compatibility

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Gregory Szorc [:gps]]

single_head_per_branch is lacking test coverage. This is important because it calls repo.branchtags(), which doesn't exist in 3.0+. This is the same failure that caused pushlog to die when hgweb was upgraded to 3.1.1.

We should add tests and update the hook to work with modern hg.

bkero: this should be a good bug for you to get your hands dirty. Want to have a go at it?

Comment 1

[Gregory Szorc [:gps]]

https://ci.mozilla.org/job/version-control-tools/coveragepy/hghooks_mozhghooks_single_head_per_branch.html

Comment 2

[Ben Kero [:bkero]]

Sure, I'll take this.

Comment 3

[Ben Kero [:bkero]]

Attachment: Updated hook and associated test

Here's an updated hook version and associated test.

The old branchtags method does not exist in Mercurial > 2.9, so it must be replaced with iterating over branchmap()

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8505865 [details] [diff] [review]
Updated hook and associated test

Review of attachment 8505865 [details] [diff] [review]:
-----------------------------------------------------------------

Main code change looks good. Just needs some minor testing work.

::: hghooks/tests/test-single_head_per_branch.t
@@ +12,5 @@
> +  $ echo 'new text in orig repo' > mozilla-central/file.txt
> +  $ hg commit -R mozilla-central -A -m 'second commit in mc'
> +  $ echo 'different' > client/file.txt
> +  $ hg commit -R client -A -m 'different commit'
> +  $ hg pull -R mozilla-central -u client

Using pull to trigger the hook is a bit weird. It's the same code path in Mercurial (adding changegroup), so it is technically accurate. Still, can you refactor this to trigger from a push?

A test that verifies a repo with multiple branches continue to work would also be nice. It shouldn't fail. But test coverage is nice.

Comment 5

[Ben Kero [:bkero]]

Attachment: pull-to-push.patch

Here the test is updated to use a push instead of pull

Comment 6

[Ben Kero [:bkero]]

Attachment: newbranch.patch

Patch to add a test which creates a new branch on the client repo, commits a change, then attempts to push a new branch to the upstream repository.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8507223 [details] [diff] [review]
pull-to-push.patch

Review of attachment 8507223 [details] [diff] [review]:
-----------------------------------------------------------------

Please squash these patches into 1 when landing.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8505865 [details] [diff] [review]
Updated hook and associated test

Review of attachment 8505865 [details] [diff] [review]:
-----------------------------------------------------------------

Upgrading to r+ with follow-ups addressed.

Comment 9

[Ben Kero [:bkero]]

merged

Comment 10

[Gregory Szorc [:gps]]

My tests are failing locally.

--- /Users/gps/src/hgcustom/version-control-tools/hghooks/tests/test-single_head_per_branch.t
+++ /Users/gps/src/hgcustom/version-control-tools/hghooks/tests/test-single_head_per_branch.t.err
@@ -20,18 +20,59 @@
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files (+1 heads)
-
-
-  ************************** ERROR ****************************
-  Multiple heads detected on branch 'default'
-  Only one head per branch is allowed!
-  *************************************************************
-
-
+  error: pretxnchangegroup.b_singlehead hook raised an exception: 'localrepository' object has no attribute 'branchtags'
   transaction abort!
   rollback completed
-  abort: pretxnchangegroup.b_singlehead hook failed
-  [255]
+  ** unknown exception encountered, please report by visiting
+  ** http://mercurial.selenic.com/wiki/BugTracker
+  ** Python 2.7.8 (default, Aug 24 2014, 21:26:19) [GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
+  ** Mercurial Distributed SCM (version 3.0.1)
+  ** Extensions loaded:
+  Traceback (most recent call last):
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/bin/hg", line 38, in <module>
+      mercurial.dispatch.run()
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 28, in run
+      sys.exit((dispatch(request(sys.argv[1:])) or 0) & 255)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 69, in dispatch
+      ret = _runcatch(req)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 138, in _runcatch
+      return _dispatch(req)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 819, in _dispatch
+      cmdpats, cmdoptions)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 599, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 910, in _runcommand
+      return checkargs()
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 881, in checkargs
+      return cmdfunc()
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/dispatch.py", line 816, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **cmdoptions)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/util.py", line 518, in check
+      return func(*args, **kwargs)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/commands.py", line 4717, in push
+      newbranch=opts.get('new_branch'))
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/localrepo.py", line 1725, in push
+      return exchange.push(self, remote, force, revs, newbranch)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/exchange.py", line 140, in push
+      _pushchangeset(pushop)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/exchange.py", line 289, in _pushchangeset
+      'push')
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/localrepo.py", line 128, in unbundle
+      ret = exchange.unbundle(self._repo, cg, heads, 'push', url)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/exchange.py", line 760, in unbundle
+      r = changegroup.addchangegroup(repo, cg, source, url)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/changegroup.py", line 679, in addchangegroup
+      url=url, pending=p, **tr.hookargs)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/localrepo.py", line 479, in hook
+      return hook.hook(self.ui, self, name, throw, **args)
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/hook.py", line 203, in hook
+      r = _pythonhook(ui, repo, name, hname, hookfn, args, throw) or r
+    File "/Users/gps/src/hgcustom/version-control-tools/venv/lib/python2.7/site-packages/mercurial/hook.py", line 89, in _pythonhook
+      r = obj(ui=ui, repo=repo, hooktype=name, **args)
+    File "/Users/gps/src/hgcustom/version-control-tools/hghooks/mozhghooks/single_head_per_branch.py", line 20, in hook
+      for b in repo.branchtags():
+  AttributeError: 'localrepository' object has no attribute 'branchtags'
+  [1]

This hook needs to work with 2.5.4 *and* 3.x to make the tests happy. See the code in hgext/pushlog-legacy/pushlog-feed.py around use of repo.branchtags() to make this 3.x safe.

Comment 11

[Gregory Szorc [:gps]]

(I should have caught this during review.)

Comment 12

[Ben Kero [:bkero]]

Negative, that was me failing to commit the actual updated hook. The first commit was just the tests. I pushed a new commit with the updated hook that does not include branchtags, instead using branchmap.

Comment 13

[Gregory Szorc [:gps]]

Yeah, my tests pass now. Yay!