Internal-changeset concept

[Kostia Balytskyi]

On 03/04/2017 12:11, Pierre-Yves David wrote:

> On 04/02/2017 05:56 AM, Jun Wu wrote:
>> IIRC at the sprint that people tend to agree on root based hidden
>> separated from phase root.
> There seems to be a large confusion here. This thread is talking about
> "internal" changeset only. As explain in my two previous large emails,
> in practice, today, the "internal" changeset operate in their own
> "space" and the users do not need to care about them. Their various
> properties make phases a possible option and even probably a good choice.
> This is -distinct- from what we have discussed at the sprint: local-only
> hiding of changeset in the -real-space-. I've been part of these people
> cautious about using phases for local-only-hiding on changesets in the
> -real-space-, but this proposal is unrelated and does not contradict it.
> Local-hidding still needs its own solution.
>> I was about to ask questions about "strip", and some "obsstore" interaction,
>> but I think the main focus should be in the obsstore thread. So I'd not
>> distract you here.
> This thread is more concrete and closer to completion. I hope we can
> reach a conclusion shortly and unlock Kostia work on shelve with it.
> Possibly before 4.2-freeze.
Regardless of the other topics discussed in this thread (and sorry if I 
am derailing the conversation),
one of the things discussed at a sprint was that moving from 
strip/abort-based shelve to obs-based
shelve is still desirable, given that it stays an experimental feature. 
*Even though* we might (and
probably will) go and make it <new-hiding-concept-based>-shelve one day. 
From my perspective,
getting off the "transaction abort" hook is worth having a "temporary" 
shelve implementation. In
the end, this "temporary" is still much closer to whichever follows than 
to what's used now.
Is this sentiment not shared?
>>   See inline comments, mainly about reviving, and is it really worth 2 new
>> phases.
>> Excerpts from Pierre-Yves David's message of 2017-04-01 02:17:12 +0200:
>> [snip]
>>> Conclusion
>>> ==========
>>> short version
>>> -------------
>>> At the current stage of my reflexion, my personal choice will be:
>>> * introduce two new phases ('internal' and 'internal-archived'),
>> I think two phases is making things unnecessarily complicated.
>> "internal" is only used for histedit or shelve merge conflict resolution.
>> It's possible to let histedit and strip implement some "dynamic
>> blocker"-style repoview hook to affect their special commits, without
>> introducing a new phase that nobody else will use.
> This is a very good point. Especially because with current cases, we
> only needs internal changesets when they are involved in a merge
> conflict. In such case, they are "working directory parent" and the
> current code already force them visible.
> I'll update the plan page to drop the second phase and mention this point.
>> That said, I still prefer root based hidden to be a separate concept stored
>> elsewhere, and not related to phases. That is also preferred by Kevin and
>> others at the sprint. I'm not sure if they change their mind after reading
>> your analysis though.
> As pointed earlier in this email, there seems to be confusion. We are
> talking about internal changesets, not the normal changesets. Phases
> seems fine for internal changesets while we still need something else
> (not phase based) for real-space changesets.
>>> * also add a '_internal' extra key for good measure since it adds goods
>>> property.
>>> * introduce a context manager to create/interact with temporary changesets
>>> rational
>>> --------
>>> I'm going for phase-space because 'public/draft/secret' do not make
>>> sense for internal changeset anyway. Making it some explicit with an
>>> 'internal' phase seem a good move.
>>> In addition, we already have all the UI and concept around phase. So
>>> introducing a new one will not add complexity to our UI.
>>> I go for a dedicated 'internal-archived' phase to handle visibility. We
>>> could use a generic local hiding mechanism but having a dedicated phases
>>> increase insulation. Such insulation reduce the chance of a user
>>> touching internal visibility by mistake. Implementation is not more
>>> complex since we can already feed the visibility code from multiple sources.
>>> I keep the 'extra' key idea to make sure we'll never collide with
>>> 'real-space'.
>>> implementation idea
>>> -------------------
>>> * phase 'internal' is visible but not exchanged. checkout and merging
>>> with it requires special flag/mode unavailable to the user,
>>> * similar flag/mode (probably context manager) is to be used when
>>> creating internal changesets,
>>> * phase 'internal-archived' has the above property except for visibility
>>> (it is invisible).
>>> * <shelved> commit stay in the 'internal' phases and a explicitly fed to
>>> the hiding mechanism. They move to 'internal-archived' phase when deleted,
>>> * starting at '4', phases are no longer 'linear' and use 'bit flag' for
>>> property.
>>> * phase property defined earlier still apply (eg: any phase greater than
>>> 1 is not exchanged)
>>> * we use 'internal-archived=32', 'internal=96' This gives us 'bit-5(32)'
>>> → internal; 'bit-6(64)' → visible. And 'internal-archived' < 'internal'
>>> this respect the "natural" phase movement.
>>> * we add checks so that changeset with a phase having the 'bit-5' set
>>> never loose it.
>> Since some people desperately want to be able to revive commits, this does
>> not seem to be a good design.
> There seems to be confusion again. User do not care about internal
> changesets and they are no use to the user. There is not reason why a
> user would want to revive them.
> This check is here to ensure internal changeset does not stop being
> marked as such by mistake. (since there are no reason for an internal
> changeset to stop being so.
>> The *current* obsstore has trouble with reviving commits. But people can use
>> the archive phase without obsstore (as Greg wants), and phase itself does
>> not have issues with reviving.
>>> * we use the '_internal extra' to guarantee the lack of collision, (and
>>> possible safeguard.
>> If we can revive commits, I don't think this hash change is necessary.
> There seems to be confusion again, since we are talking about
> internal-changeset, they are irrelevant to the user and do not need
> reviving.
> Cheers,
>