Patchwork [1,of,8,STABLE] graft: do not use `.remove` on a smart set (regression)

login
register
mail settings
Submitter Pierre-Yves David
Date April 29, 2014, 12:48 a.m.
Message ID <80f58cb63468525ff8a3.1398732508@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/4445/
State Accepted
Commit a1381eea7c7d3ee8a92b4d9fcc403b4b518aa56c
Headers show

Comments

Pierre-Yves David - April 29, 2014, 12:48 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1398731136 25200
#      Mon Apr 28 17:25:36 2014 -0700
# Branch stable
# Node ID 80f58cb63468525ff8a3f17536608e307bdb26ca
# Parent  d36440d843284ba546858b241da9cc4817811fe5
graft: do not use `.remove` on a smart set (regression)

Revset calls use to return a list. Graft use to mutate that list. We cannot do
this anymore leading to a crash when grafting multiple changeset with a revset.

    File ".../mercurial/commands.py", line 3117, in graft
      revs.remove(rev)
    AttributeError: '_addset' object has no attribute 'remove'

We are late in code-freeze so we make the shortest possible fix by turning it
back to a list.
Pierre-Yves David - May 7, 2014, 12:20 a.m.
On 04/28/2014 05:48 PM, pierre-yves.david@ens-lyon.org wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1398731136 25200
> #      Mon Apr 28 17:25:36 2014 -0700
> # Branch stable
> # Node ID 80f58cb63468525ff8a3f17536608e307bdb26ca
> # Parent  d36440d843284ba546858b241da9cc4817811fe5
> graft: do not use `.remove` on a smart set (regression)

This series was partially queued

(waving at Augies patchbot)

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3108,14 +3108,18 @@  def graft(ui, repo, *revs, **opts):
         return -1
 
     # check for ancestors of dest branch
     crev = repo['.'].rev()
     ancestors = repo.changelog.ancestors([crev], inclusive=True)
+    # Cannot use x.remove(y) on smart set, this has to be a list.
+    # XXX make this lazy in the future
+    revs = list(revs)
     # don't mutate while iterating, create a copy
     for rev in list(revs):
         if rev in ancestors:
             ui.warn(_('skipping ancestor revision %s\n') % rev)
+            # XXX remove on list is slow
             revs.remove(rev)
     if not revs:
         return -1
 
     # analyze revs for earlier grafts
diff --git a/tests/test-graft.t b/tests/test-graft.t
--- a/tests/test-graft.t
+++ b/tests/test-graft.t
@@ -569,5 +569,16 @@  All copies of a cset
   tag:         tip
   user:        foo
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     2
   
+
+graft works on complex revset
+
+  $ hg graft 'origin(13) or destination(origin(13))'
+  skipping ancestor revision 21
+  skipping ancestor revision 22
+  skipping revision 2 (already grafted to 22)
+  grafting revision 7
+  grafting revision 13
+  grafting revision 19
+  merging b