[PATCH 09 of 10] mq: mq.secret prevent mq changeset outside secret phase

[Pierre-Yves David]

On Tue, Jan 31, 2012 at 12:07:04PM -0600, Matt Mackall wrote:
> On Tue, 2012-01-31 at 17:25 +0100, Pierre-Yves David wrote:
> > On Mon, Jan 30, 2012 at 02:03:27PM -0600, Matt Mackall wrote:
> > > On Mon, 2012-01-30 at 17:49 +0100, pierre-yves.david at logilab.fr wrote:
> > > > +  $ cp -r . ../remote
> > > > +  $ hg -R ../remote qfinish qbase
> > > > +  patch add-file1 finalized without changeset message
> > > > +  $ hg pull ../remote # phase sync
> > > > +  pulling from ../remote
> > > > +  searching for changes
> > > > +  no changes found
> > > > +  abort: cannot set mq changeset to public with mq in secret mode
> > > > +  (qfinish them or disable mq.secret)
> > > 
> > > It's not obvious to me that this is a good thing.
> > 
> > As stated before I think it's a good idea to prevent case for mq secret to
> > because draft. In particular if this case is rare but probable. Because it will
> > break user expectation on the tool which is Bad (mayve even Not Cool™)
> > 
> > My test case is a bit minimalistisc but it also cover the  following situation
> > (see issue1099):
> 
> I read 1099, which is actually about MQ patches _on the server_.
> 
> > You have two paches applied localy
> > 
> >     o C (MQ)
> >     |
> >     o B (MQ)
> >     |
> >     o A
> > 
> > With one existing remotly (because you sent it for review and it got accepted)
> > 
> >     o D
> >     |
> >     o B
> >     |
> >     o A
> > 
> > The pull will result in the following:
> > 
> >     o D
> >     |
> >     | o C (MQ)
> >     |/
> >     o B (MQ)
> >     |
> >     o A
> > 
> > This is very bad because this is an invalid mq state (because of D) (and mq
> > will fails to qpop or qref B)
> 
> I don't know if I'd call it invalid. MQ has been ready to handle this
> situation for many years:
> 
> abort: popping would remove a revision not managed by this patch queue
> abort: cannot refresh a revision with children
> 
> > Moreover this result in B to because either draft or public (with out without D to exist)
> > 
> > A) if mq.secret=true and B becomes draft: This break user expectation
> 
> Do you mean public?

No I mean draft: if a mq changeset is draft while mq.secret=True, it break user
expectation.

> > B) if B becomes public: we have a public changeset handle by mq that can
> >    rewrite it. this break expection of the public phase.
> 
> Reconcile these two statements for me:
> 
> "mq will fails to qpop or qref B"
> "if B becomes public [...] mq that can rewrite it"

If D don't exist but B is on the server, B is public and the safety above don't
applies. This probably qualifies as a bug and mq should refuse to qpop or qref
public changeset too.

> Remind me why we're doing this whole phases thing again. I thought it
> was so that when a changeset became public, we would recognize that and
> cease trying to modify it.

> This patch seems entirely at odds with that.  It's basically covering your
> ears and saying "no no no, no one knows my secret".

This patch is more about: You asked me to keep this secret but other people are
already aware of it. I'm a stupid machine, please tell me what to do!

-- 
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://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20120131/57b382cc/attachment.asc>