[PATCH 2 of 2 STABLE MOST-WANTED] push: make locking of source optional (issue3684)

[Pierre-Yves David]

On Tue, Apr 30, 2013 at 08:51:31AM -0500, Kevin Bullock wrote:
> On 30 Apr 2013, at 5:04 AM, pierre-yves.david at logilab.fr wrote:
> 
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at logilab.fr>
> > # Date 1367315511 -7200
> > #      Tue Apr 30 11:51:51 2013 +0200
> > # Branch stable
> > # Node ID 4be1d06df73f0092277b83e243ba4ca583c040e0
> > # Parent  b5c9626ec444375a6368d40ba5218ede6401c68d
> > push: make locking of source optional (issue3684)
> > 
> > Having the permission to lock the source repo on push is now optional. When the
> > repo cannot be locked, phase are not changed locally. A status message is issue
> > when some actual phase movement are skipped:
> > 
> >    public phase unchanged locally, cannot lock repo
> 
> I think in order to be useful, the message would need to somehow reference what _changesets_ couldn't have their phases changed.

Yes, but then you have to handle the fact that this message could reference a
gazillion different heads.

So it needs to be something like:

    cannot locally turn 4be1d06df73f (and 13 others heads) public, unable to lock repo

That exceeded my "deserves another patch" threshold. Lets put the simplest
version first and then think about the best message.

I'm happy that you brought this up !


> 
> > 
> > A debug message with the exact reason of the locking failure is issued in all
> > case.
> > 
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -1764,11 +1764,30 @@ class localrepository(object):
> >         unfi = self.unfiltered()
> >         def localphasemove(nodes, phase=phases.public):
> >             """move <nodes> to <phase> in the local source repo"""
> >             phases.advanceboundary(self, phase, nodes)
> >         # get local lock as we might write phase data
> > -        locallock = self.lock()
> > +        locallock = None
> > +        try:
> > +            locallock = self.lock()
> > +        except IOError, err:
> > +            if err.errno != errno.EACCES:
> > +                raise
> > +            # source repo cannot be locked.
> > +            # We do not abort the push, but just disable the local phase
> > +            # synchronisation.
> > +            msg = 'cannot lock source repository: %s\n' % err
> > +            self.ui.debug(msg)
> > +            def localphasemove(nodes, phase=phases.public):
> 
> Why not just check 'if locallock is None' inside the closure instead of
> completely redefining it? This way is a bit more confusing to read.

Good idea.

-- 
Pierre-Yves David

http://www.logilab.fr/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130430/de9136b0/attachment.pgp>