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

[Matt Mackall]

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?

> 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"


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".

-- 
Mathematics is the supreme nostalgia of our time.