Update single_head_per_branch hook for 3.1 compatibility

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gps, Assigned: bkero)

Tracking

Details

(Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/265] )

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
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?
Flags: needinfo?(bkero)
(Assignee)

Comment 2

4 years ago
Sure, I'll take this.
Assignee: nobody → bkero
Flags: needinfo?(bkero)
Product: Release Engineering → Developer Services
(Assignee)

Comment 3

4 years ago
Created attachment 8505865 [details] [diff] [review]
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()
Attachment #8505865 - Flags: review?(gps)
(Reporter)

Comment 4

4 years ago
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.
Attachment #8505865 - Flags: review?(gps) → feedback+
(Assignee)

Comment 5

4 years ago
Created attachment 8507223 [details] [diff] [review]
pull-to-push.patch

Here the test is updated to use a push instead of pull
Attachment #8507223 - Flags: review?(gps)
(Assignee)

Comment 6

4 years ago
Created attachment 8507240 [details] [diff] [review]
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.
Attachment #8507240 - Flags: review?(gps)

Updated

4 years ago
Whiteboard: [kanban:engops:https://kanbanize.com/ctrl_board/6/265]
(Reporter)

Updated

4 years ago
Attachment #8507240 - Flags: review?(gps) → review+
(Reporter)

Comment 7

4 years ago
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.
Attachment #8507223 - Flags: review?(gps) → review+
(Reporter)

Comment 8

4 years ago
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.
Attachment #8505865 - Flags: review+
(Assignee)

Comment 9

4 years ago
merged
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

4 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 11

4 years ago
(I should have caught this during review.)
(Assignee)

Comment 12

4 years ago
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.
(Reporter)

Comment 13

4 years ago
Yeah, my tests pass now. Yay!
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Whiteboard: [kanban:engops:https://kanbanize.com/ctrl_board/6/265] → [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/265]
You need to log in before you can comment on or make changes to this bug.