| Submitter | Pierre-Yves David |
|---|---|
| Date | Aug. 3, 2015, 9:25 p.m. |
| Message ID | <ab1f4da158c6950ed6b8.1438637127@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/10098/ |
| State | Accepted |
| Headers | show |
Comments
This series looks good to me and seems to fix the issues that we have been seeing using histedit with evolve. I talked to you offline about a test case that used to not work and that this series is fixing, let's add it to make sure that we don't regress. Thanks, Laurent > On Aug 3, 2015, at 2:25 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1438607585 25200 > # Mon Aug 03 06:13:05 2015 -0700 > # Node ID ab1f4da158c6950ed6b8eb645c5a634ec169f55a > # Parent ec0875fa682acd025a41471438c64041a37a2f82 > histedit: do not stay on a cleaned nodes on abort > > There is case where nodes are neither in tmpnodes nor leaf but still get > removed. For example, if you used the "edit" action, made a commit and run > --abort. The commit you made is not tracked by histedit, yet it will likely be > cleaned up with its parent. The commit may not tracked because no replacements > computations are done in the --abort case. > > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -789,11 +789,11 @@ def _histedit(ui, repo, state, *freeargs > changegroup.addchangegroup(repo, gen, 'histedit', > 'bundle:' + backupfile) > os.remove(backupfile) > > # check whether we should update away > - if repo.unfiltered().revs('parents() and (%n or %ln)', > + if repo.unfiltered().revs('parents() and (%n or %ln::)', > state.parentctxnode, leafs | tmpnodes): > hg.clean(repo, state.topmost) > cleanupnode(ui, repo, 'created', tmpnodes) > cleanupnode(ui, repo, 'temp', leafs) > state.clear() > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
On 08/03/2015 04:29 PM, Laurent Charignon wrote: > This series looks good to me and seems to fix the issues that we have been seeing using histedit with evolve. > I talked to you offline about a test case that used to not work and that this series is fixing, let's add it to make sure that we don't regress. The test case required the histedit extensions use at Facebook right? and so the test case should live there. Or are we talking about something else?
Oh yeah, sorry I forgot that we would need that extension to add the test. You can proceed then. Thanks, Laurent > On Aug 3, 2015, at 4:35 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > > On 08/03/2015 04:29 PM, Laurent Charignon wrote: >> This series looks good to me and seems to fix the issues that we have been seeing using histedit with evolve. >> I talked to you offline about a test case that used to not work and that this series is fixing, let's add it to make sure that we don't regress. > > The test case required the histedit extensions use at Facebook right? and so the test case should live there. Or are we talking about something else? > > -- > Pierre-Yves David
This looks good and simple enough even for someone not familiar with histedit internals that I dare push it to the clowncopter (done). Thanks! On Mon, Aug 3, 2015 at 5:29 PM Laurent Charignon <lcharignon@fb.com> wrote: > Oh yeah, sorry I forgot that we would need that extension to add the test. > You can proceed then. > > Thanks, > > Laurent > > > On Aug 3, 2015, at 4:35 PM, Pierre-Yves David < > pierre-yves.david@ens-lyon.org> wrote: > > > > > > > > On 08/03/2015 04:29 PM, Laurent Charignon wrote: > >> This series looks good to me and seems to fix the issues that we have > been seeing using histedit with evolve. > >> I talked to you offline about a test case that used to not work and > that this series is fixing, let's add it to make sure that we don't regress. > > > > The test case required the histedit extensions use at Facebook right? > and so the test case should live there. Or are we talking about something > else? > > > > -- > > Pierre-Yves David > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel >
Patch
diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -789,11 +789,11 @@ def _histedit(ui, repo, state, *freeargs changegroup.addchangegroup(repo, gen, 'histedit', 'bundle:' + backupfile) os.remove(backupfile) # check whether we should update away - if repo.unfiltered().revs('parents() and (%n or %ln)', + if repo.unfiltered().revs('parents() and (%n or %ln::)', state.parentctxnode, leafs | tmpnodes): hg.clean(repo, state.topmost) cleanupnode(ui, repo, 'created', tmpnodes) cleanupnode(ui, repo, 'temp', leafs) state.clear()