D3694: shelve: use more accurate description in conflict marker

lothiraldan (Boris Feld) phabricator at mercurial-scm.org
Tue Jun 12 12:16:01 EDT 2018


lothiraldan added a comment.


  In https://phab.mercurial-scm.org/D3694#58242, @durin42 wrote:
  
  > In https://phab.mercurial-scm.org/D3694#58241, @lothiraldan wrote:
  >
  > > > Interesting. I think I like this, it's a bummer that it requires a format bump in requires.
  > >
  > >
  > >
  > > - There are no format changes per-se, older client would preserve the phases for internal changesets, but have them visible,
  >
  >
  > We didn't do a format bump when we introduced draft/secret phases, so maybe we should punt here too.
  
  
  Phases did not introduce any regression in behavior. When draft/secret where introduced, older clients could keep interacting with the repository the same way as before. The new changesets created by new clients were seen just fine (but not the associated behavior).
  
  However in the shelve case, if both a new client and an old client access the same local repository, we have a change in behavior:
  
  - before: shelve created from the new client are stripped and invisible to the old client
  - after: shelve created from the new client are visible as changeset to the old client
  
  The danger is that users could push those intermediary shelve commits if they are not careful. The new requirements and the error message will make them think about the issue and not push intermediary commits.
  
  >> - This is also something we would need for obsshelve (non evolve enabled client complaining about markers),
  > 
  > I don't think so, because in general shelve isn't being run on a repository that's accessed remotely. (In general - that's not always true, but I think it's "true enough" esp. since we _need_ to ship obsmarkers on by default soon.)
  
  We have the same scenario for obsshelve:
  
  - before: shelve created from the new client are stripped
  - after: shelve created from the new client create obsmarkers and trigger related warnings.
  
  This is not a theoretical case, we have seen valid situations where new and old client access the same repositories.
  
  >> - There are more format bumps coming (when solving issue5480) and to other future series.
  >> 
  >>> I'm a little anxious about defining the `internal` phase because it might restrict the potential for other phases past secret
  >> 
  >> I kept things simple in the current series, but I had a similar thinking. One option is to encode the internal phase as "32" instead of "3" to leave us room for other phases. If we this we need to update the implementation with one of the following:
  >> 
  >>   (1) add 29 "reserved" empty phase that will remain empty.
  >>    
  >>   (2) rework the phases touching code to work on non-contiguous numbers.
  > 
  > How much work is this, do you have any idea?
  
  The first option (adding "reserved" phase) should be very quick to implement. It might need minor adjustment for performance but I don't expect many.
  
  The second option (changing all algorithm to handle the gap) is more work since about all algorithm touching phases in Core and extensions assume they can be handled as a simple list.
  
  So I would pick the first option.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3694

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel


More information about the Mercurial-devel mailing list