[PATCH] workingctx: unlink paths while holding the wlock

Matt Mackall mpm at selenic.com
Tue May 24 12:37:41 CDT 2011


On Tue, 2011-05-24 at 15:27 +0200, Adrian Buehlmann wrote:
> On 2011-05-24 14:46, Adrian Buehlmann wrote:
> > # HG changeset patch
> > # User Adrian Buehlmann <adrian at cadifra.com>
> > # Date 1306238900 -7200
> > # Node ID 17e7ee042dc451fae1363803ce10af8543bffd05
> > # Parent  25137d99a5ed215f302ffc1793590bdbdb437b55
> > workingctx: unlink paths while holding the wlock
> > 
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -852,15 +852,15 @@
> >              yield changectx(self._repo, a)
> >  
> >      def remove(self, list, unlink=False):
> > -        if unlink:
> > -            for f in list:
> > -                try:
> > -                    util.unlinkpath(self._repo.wjoin(f))
> > -                except OSError, inst:
> > -                    if inst.errno != errno.ENOENT:
> > -                        raise
> >          wlock = self._repo.wlock()
> >          try:
> > +            if unlink:
> > +                for f in list:
> > +                    try:
> > +                        util.unlinkpath(self._repo.wjoin(f))
> > +                    except OSError, inst:
> > +                        if inst.errno != errno.ENOENT:
> > +                            raise
> >              for f in list:
> >                  if unlink and os.path.lexists(self._repo.wjoin(f)):
> >                      self._repo.ui.warn(_("%s still exists!\n") % f)
> 
> I'm wondering here: why do we check for the existence of the files
> again, right after having unlinked them?

This is what we've got version control for:

http://www.selenic.com/hg/diff/c6e6ca96a033/mercurial/localrepo.py

The message predates the introduction of unlinking in this function, so
it warns when we remove something without unlinking it.

Then someone came along and decided the warning only made sense for
unlinked files:

http://www.selenic.com/hg/diff/9770d260a405/mercurial/localrepo.py

> FWIW, if I apply the following patch on top, the testsuite is ok
> 
> # HG changeset patch
> # User Adrian Buehlmann <adrian at cadifra.com>
> # Date 1306241543 -7200
> # Node ID 3641b251b2a34dc22dbd73cd4796c5e87cc9925c
> # Parent  17e7ee042dc451fae1363803ce10af8543bffd05
> workingctx.remove: don't stat file again after unlinking
> 
> we just unlinked it right before
> 
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -862,9 +862,7 @@
>                          if inst.errno != errno.ENOENT:
>                              raise
>              for f in list:
> -                if unlink and os.path.lexists(self._repo.wjoin(f)):
> -                    self._repo.ui.warn(_("%s still exists!\n") % f)
> -                elif self._repo.dirstate[f] == 'a':
> +                if self._repo.dirstate[f] == 'a':
>                      self._repo.dirstate.forget(f)
>                  elif f not in self._repo.dirstate:
>                      self._repo.ui.warn(_("%s not tracked!\n") % f)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list