[PATCH 1 of 2] strip: add a delayedstrip method that works in a transaction
Martin von Zweigbergk
martinvonz at google.com
Tue Jun 27 16:16:17 EDT 2017
On Tue, Jun 27, 2017 at 10:38 AM, Jun Wu <quark at fb.com> wrote:
> 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.
I think allowduplicates is quite different from allowunstable. We can
automate resolution of unstable commits, but we can't automate
resolution of duplicates. I'm still convinced it's a bug in a the
command that creates the duplicate commit (unless that's the command's
purpose, of course, but then it really shouldn't attempt to strip one
of the commits).
>
>> > [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.
As I said above, I don't think 1 is a good idea. Maybe I'm just being
stubborn (I encourage a third party to step in and say that if that's
the case), but I'd really prefer to make it just a ProgrammingError.
If we ever come up with a valid use case, we can add add the
allowduplicates config then (we'll know, because the user/developer
ran into the exception).
>
>> 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 understand that the user can add commits, and I suppose that's
covered by that test-rebase-interruptions.t. However, I don't see how
they can trigger the ProgrammingError.
>
>> 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.
See above.
> 4 is not easy and is not of my main focus, so I'll
> leave it for grabs.
Sure, 4 is definitely out of scope.
>
> What do you think?
More information about the Mercurial-devel
mailing list