[MERGE][0.8] break-lock command

[Robert Collins]

On Thu, 2006-05-04 at 09:33 -0500, John Arbash Meinel wrote:
> Robert Collins wrote:
> > This is also available at http://bazaar-vcs.org/bzr/break-lock.
> > 
> > diffstat follows: note the strange presence of 'b' in just the new
> > files. I think think diff prefix patch is buggy :p.
> > 
> 
> Well diffstat seems a little buggy. But the rest of the files all have
> an a/NEWS, b/NEWS.
> I think the reason might be that you are comparing /dev/null to
> b/bzrlib/tests/...
> 
> And diffstat doesn't see an obvious prefix for /dev/null (but we
> certainly shouldn't be writing it a/dev/null)

yeah. I think it evaluates each file for prefix rather than doing it
once for the whole patch - which is probably right in other
circumstances.


> ...
> 
> >      def can_convert_format(self):
> >          """Return true if this bzrdir is one whose format we can convert from."""
> > @@ -581,6 +599,10 @@
> >                                              self._format._lock_file_name,
> >                                              self._format._lock_class)
> >  
> > +    def break_lock(self):
> > +        """Pre-splitout bzrdirs do not suffer from stale locks."""
> > +        raise NotImplementedError(self.break_lock)
> > +
> 
> Not true, they can get stale locks from an sftp lock. Local branches
> can't get stale (because they are an OS lock).
> 
> I'm not sure if you want to support break_lock() for sftp-style locking.
> But you should at least make a comment about it.

mmm. The SFTP locking was always completely ignored by bzr locally. I
can add a comment if you really think its needed, but frankly I'd rather
forget about pre-0.8 SFTP support as a bad dream.

> ...
> 
> > +    def assert_get_bool_acceptance_of_user_input(self, factory):
> > +        factory.stdin = StringIO("y\nyes with garbage\nyes\nn\nnot an answer\nno\nfoo\n")
> > +        factory.stdout = StringIO()
> > +        # there is no output from the base factory
> > +        self.assertEqual(True, factory.get_boolean(""))
> > +        self.assertEqual(True, factory.get_boolean(""))
> > +        self.assertEqual(False, factory.get_boolean(""))
> > +        self.assertEqual(False, factory.get_boolean(""))
> > +        self.assertEqual("foo\n", factory.stdin.read())
> 
> What about just a plain empty string? Should we support
> get_boolean(prompt, default=None)?


mmm. So [Y/n] for default=True and [y/N] for default=False ?


> This would also help since SilentUIFactory shouldn't be prompting the
> user. And thus the defaults could be used.


I think that the SilentUIFactory is not the same as 'NoUIFactory' - it
is silent yes, but its still hooked into stdin - at least it is now :).

(I read 'silent' as 'no output', not 'no input'). You could for example
do 'echo Y | bzr -qq break-lock'.

Also, I dont think that the defaults will help here: the default for
break lock should surely be 'N'.

> > +
> > +    def test_silent_ui_getbool(self):
> > +        factory = bzrlib.ui.SilentUIFactory()
> > +        self.assert_get_bool_acceptance_of_user_input(factory)
> 
> ...
> 
> Otherwise this looks pretty good.

Thanks,
Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060505/a8467f5d/attachment.pgp