[PATCH 1 of 2] strip: add a delayedstrip method that works in a transaction

Jun Wu quark at fb.com
Tue Jun 27 13:38:50 EDT 2017


Excerpts from Martin von Zweigbergk's message of 2017-06-26 21:33:50 -0700:
> >
> > If we abort, what do you think about the "hg rebase --continue" case in
> > issue5606? Should we *enforce* the user to turn on evolve to continue?
> > I don't think that is a good idea.
> 
> Yes, either turn on evolve or abort the rebase (aborting the rebase
> was what I was thinking while writing the paragraph above, but
> continuing with evolve on should also be a solution). This is such a
> corner case that I don't care much.

I think the key disagreement is whether should we "allowunstable" when
obsstore is disabled. I think for power users, if they explicitly ask for,
we should continue.

> > [1]: https://bitbucket.org/facebook/hg-experimental/issues/6/hg-absorb-merges-diverged-commits 
> 
> I spent a few minutes, but I couldn't understand how it would break.

It's similar to amend a non-head with obsstore disabled. He is a power user
that wants advanced stack editing without obsstore.

> I'm not sure, but it's probably the person writing the "hg
> rebase-and-commit" command made the mistake. To me, just looking at
> the result from the user's perspective makes it clearly wrong. If the
> rebasing leaves both an old version and a new version in the repo, I
> would definitely say that there's a bug in the command.

How about:

  1. Add a way to allowunstable without evolve, maybe call it
     "allowduplicates" for power users that I care about.
  2. ProgrammingError if allowduplicates is not set (which is the default),
     but continue with warning if allowduplicates is set.

> While writing this, I saw your other reply about users also producing
> commits on top. That's an interesting case, and I'm glad we have a
> test case for it. However, I don't see how that case will actually
> trigger the ProgrammingError I'm suggesting. The ProgrammingError
> would happen in the postclose hook, which happens after the
> transaction close, but still before the repo lock is released, right?
> If that's the case, I don't see how the user could create a commit in
> the meantime.

The rebase was interrupted because of merge conflicts. So the user can
modify the DAG during the time they were supposed to fix conflicts. I guess
you might want to make the repo read-only for non-rebase commands if
.hg/rebasestate exists.

> I saw your other email about finding a way to make that work. Even if
> you didn't, the case seems so obscure that I wouldn't mind losing
> support for it.
> 
> Let me know if it's not actually as obscure as it seems; i.e. I'd be
> curious to hear of a useful command that might produce that.

I think a reasonable path to move ahead is:

  1. Add the allowduplicates config for power users
  2. Set that config to True in test-rebase-interruptions.t
  3. In strip code, raise ProgrammingError if allowduplicates is not set,
     continue with warning otherwise.
  4. Make repo read-only for non-rebase commands when .hg/rebasestate exists
     and allowduplicates is not set.

I can complete 1, 2, 3. 4 is not easy and is not of my main focus, so I'll
leave it for grabs.

What do you think?


More information about the Mercurial-devel mailing list