Internal-changeset concept (was: re: [PATCH RFC] repo: add an ability to hide nodes in an appropriate way)

[Jun Wu]

IIRC at the sprint that people tend to agree on root based hidden
separated from phase root.

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.

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.

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.

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

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.

> 
> 
> What do you think ?
> 
> Cheers,
>