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

Martin von Zweigbergk martinvonz at google.com
Tue Jun 27 19:06:26 EDT 2017


On Tue, Jun 27, 2017 at 2:01 PM, Augie Fackler <raf at durin42.com> wrote:
> On Tue, Jun 27, 2017 at 01:16:17PM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> 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).
>
> I think it'd be fine to add a TODO that we should look for cases where
> this is valid and try to constrain it - my gut reaction is that Jun is
> right, and that it can't be a programming error if we ever allow
> instability (which is sometimes useful for power users, and might be
> useful if we get a little more smarts around defering stabilization of
> changes.)
>
> Does that make sense? Or have I not quite understood this conversation?

I'm not convinced either way, but let's just drop this for now. Let's
instead get the other patches that add callers of this method in.
Then, when that's done, we can revisit this. It should be clearer then
if we have any legitimate cases where the warning happens.


More information about the Mercurial-devel mailing list