| Submitter | Idan Kamara |
|---|---|
| Date | Dec. 17, 2012, 3:35 p.m. |
| Message ID | <dfe35839906aaf57d18e.1355758536@idan> |
| Download | mbox | patch |
| Permalink | /patch/165/ |
| State | Changes Requested, archived |
| Headers | show |
Comments
On 8 janv. 2013, at 20:34, Idan Kamara wrote: > On Tue, Jan 8, 2013 at 9:23 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > > > > On 8 janv. 2013, at 20:10, Idan Kamara wrote: > > > > > On Tue, Jan 8, 2013 at 8:00 PM, Pierre-Yves David > > > <pierre-yves.david@logilab.fr> wrote: > > > > > > > > On Mon, Dec 17, 2012 at 05:35:36PM +0200, Idan Kamara wrote: > > > > > # HG changeset patch > > > > > # User Idan Kamara <idankk86@gmail.com> > > > > > # Date 1355568420 -7200 > > > > > # Branch stable > > > > > # Node ID dfe35839906aaf57d18eeebfc529425de7c0ac85 > > > > > # Parent 9157ffe3d66d41bb7ea3cfa062d775a87731206a > > > > > rollback: invalidate dirstate through the filecache > > > > > > > > > > There's no reason to use invalidate on the dirstate instance since > > > > > .hg/dirstate > > > > > was just replaced and the filecache will spot this and reload it. > > > > > > > > > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > > > > > --- a/mercurial/localrepo.py > > > > > +++ b/mercurial/localrepo.py > > > > > @@ -1009,6 +1009,7 @@ > > > > > parents[1] not in self.changelog.nodemap) > > > > > if parentgone: > > > > > util.rename(self.join('undo.dirstate'), > > > > > self.join('dirstate')) > > > > > + self.invalidatedirstate() > > > > > try: > > > > > branch = self.opener.read('undo.branch') > > > > > self.dirstate.setbranch(encoding.tolocal(branch)) > > > > > > > > What is this this extra call for ? You description only talk about the > > > > removed call bellow. > > > > > > Which call? To dirstate.setbranch? > > > > The: > > + self.invalidatedirstate() > > it replaces the old invalidate call. > > But on second thought I might drop this patch altogether > since it causes a stat on .hg/dirstate when it's not > really needed. My point is: Your description say: - call to XXX is not necessary, dropping it Your patch do - add call to YYY - drop call to XXX I think you should either: - explain why you add YYY - split adding of YYY from the dropping of XXX - not add call to YY
On Wed, Jan 9, 2013 at 8:52 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > On 8 janv. 2013, at 20:34, Idan Kamara wrote: > > > On Tue, Jan 8, 2013 at 9:23 PM, Pierre-Yves David > > <pierre-yves.david@ens-lyon.org> wrote: > > > > > > > > > On 8 janv. 2013, at 20:10, Idan Kamara wrote: > > > > > > > On Tue, Jan 8, 2013 at 8:00 PM, Pierre-Yves David > > > > <pierre-yves.david@logilab.fr> wrote: > > > > > > > > > > On Mon, Dec 17, 2012 at 05:35:36PM +0200, Idan Kamara wrote: > > > > > > # HG changeset patch > > > > > > # User Idan Kamara <idankk86@gmail.com> > > > > > > # Date 1355568420 -7200 > > > > > > # Branch stable > > > > > > # Node ID dfe35839906aaf57d18eeebfc529425de7c0ac85 > > > > > > # Parent 9157ffe3d66d41bb7ea3cfa062d775a87731206a > > > > > > rollback: invalidate dirstate through the filecache > > > > > > > > > > > > There's no reason to use invalidate on the dirstate instance > > > > > > since > > > > > > .hg/dirstate > > > > > > was just replaced and the filecache will spot this and reload > > > > > > it. > > > > > > > > > > > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > > > > > > --- a/mercurial/localrepo.py > > > > > > +++ b/mercurial/localrepo.py > > > > > > @@ -1009,6 +1009,7 @@ > > > > > > parents[1] not in self.changelog.nodemap) > > > > > > if parentgone: > > > > > > util.rename(self.join('undo.dirstate'), > > > > > > self.join('dirstate')) > > > > > > + self.invalidatedirstate() > > > > > > try: > > > > > > branch = self.opener.read('undo.branch') > > > > > > > > > > > > self.dirstate.setbranch(encoding.tolocal(branch)) > > > > > > > > > > What is this this extra call for ? You description only talk about > > > > > the > > > > > removed call bellow. > > > > > > > > Which call? To dirstate.setbranch? > > > > > > The: > > > + self.invalidatedirstate() > > > > it replaces the old invalidate call. > > > > But on second thought I might drop this patch altogether > > since it causes a stat on .hg/dirstate when it's not > > really needed. > > My point is: > > Your description say: > - call to XXX is not necessary, dropping it > > Your patch do > - add call to YYY > - drop call to XXX > > I think you should either: > - explain why you add YYY > - split adding of YYY from the dropping of XXX > - not add call to YY The first line says: rollback: invalidate dirstate through the filecache So I'm not sure what you're complaining about. Anyway like I said, I dropped this patch on second thought.
Patch
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -1009,6 +1009,7 @@ parents[1] not in self.changelog.nodemap) if parentgone: util.rename(self.join('undo.dirstate'), self.join('dirstate')) + self.invalidatedirstate() try: branch = self.opener.read('undo.branch') self.dirstate.setbranch(encoding.tolocal(branch)) @@ -1017,7 +1018,6 @@ 'current branch is still \'%s\'\n') % self.dirstate.branch()) - self.dirstate.invalidate() parents = tuple([p.rev() for p in self.parents()]) if len(parents) > 1: ui.status(_('working directory now based on '