Patchwork [2,of,4] phases: add an internal phases

login
register
mail settings
Submitter Boris Feld
Date Aug. 27, 2018, 10:39 a.m.
Message ID <de8fadb2459129a8ee09.1535366394@FB-lair>
Download mbox | patch
Permalink /patch/34077/
State Accepted
Headers show

Comments

Boris Feld - Aug. 27, 2018, 10:39 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1535152788 -7200
#      Sat Aug 25 01:19:48 2018 +0200
# Node ID de8fadb2459129a8ee097d2a8cadb23ee02473c7
# Parent  1a32c924756abb3794e81d3fa19136939d83d5e5
# EXP-Topic internal-phase.new-phase
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r de8fadb24591
phases: add an internal phases

The phase is meant to be used for internal commit that are a byproduct of
command operation (eg:shelve).

This changeset focus on getting the most important feature in, more advanced
API is to be introduced in later changesets.

The phase approach to handle internal has multiple advantages:
* simple to implement, reuse optimized code,
* fits well with existing phases. We don't want internal changeset to be
  exchanged or served.
* easy to extend to for lifecycle handling.

More thinking about internal changeset at https://www.mercurial-scm.org/wiki/InternalsPlan

We add this new phases with a high number to leave some room to possible other
phases. We also try out playing with the idea of "flag" for phases, each flag
would convey a distinct meaning. We can drop the flag idea in the future
(keeping the existing numbers). The flag property should still move in a
monotonic direction (enabled -> disabled) or be immutable like the "internal"
flag.

To simplify the addition of this new phase, we introduce many placeholder
phases. They are not meant to be used for now. Keeping `allphases` as a list
ensure existing algorithm works fine.

The full performance impact of adding that many hollow phases is unclear so
far. The impact on phase computation is visible but not worrisome.

Runnin `hg perfphase` in my mercurial development repository.
Before: ! wall 0.001807 comb 0.000000 user 0.000000 sys 0.000000 (median of 1597)
after:  ! wall 0.001906 comb 0.000000 user 0.000000 sys 0.000000 (median of 1521)
Yuya Nishihara - Aug. 28, 2018, 12:05 p.m.
On Mon, 27 Aug 2018 12:39:54 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1535152788 -7200
> #      Sat Aug 25 01:19:48 2018 +0200
> # Node ID de8fadb2459129a8ee097d2a8cadb23ee02473c7
> # Parent  1a32c924756abb3794e81d3fa19136939d83d5e5
> # EXP-Topic internal-phase.new-phase
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r de8fadb24591
> phases: add an internal phases
> 
> The phase is meant to be used for internal commit that are a byproduct of
> command operation (eg:shelve).
> 
> This changeset focus on getting the most important feature in, more advanced
> API is to be introduced in later changesets.
> 
> The phase approach to handle internal has multiple advantages:
> * simple to implement, reuse optimized code,
> * fits well with existing phases. We don't want internal changeset to be
>   exchanged or served.
> * easy to extend to for lifecycle handling.
> 
> More thinking about internal changeset at https://www.mercurial-scm.org/wiki/InternalsPlan
> 
> We add this new phases with a high number to leave some room to possible other
> phases. We also try out playing with the idea of "flag" for phases, each flag
> would convey a distinct meaning. We can drop the flag idea in the future
> (keeping the existing numbers). The flag property should still move in a
> monotonic direction (enabled -> disabled) or be immutable like the "internal"
> flag.
> 
> To simplify the addition of this new phase, we introduce many placeholder
> phases. They are not meant to be used for now. Keeping `allphases` as a list
> ensure existing algorithm works fine.
> 
> The full performance impact of adding that many hollow phases is unclear so
> far. The impact on phase computation is visible but not worrisome.
> 
> Runnin `hg perfphase` in my mercurial development repository.
> Before: ! wall 0.001807 comb 0.000000 user 0.000000 sys 0.000000 (median of 1597)
> after:  ! wall 0.001906 comb 0.000000 user 0.000000 sys 0.000000 (median of 1521)
> 
> diff --git a/mercurial/phases.py b/mercurial/phases.py
> --- a/mercurial/phases.py
> +++ b/mercurial/phases.py
> @@ -123,11 +123,22 @@ from . import (
>  
>  _fphasesentry = struct.Struct('>i20s')
>  
> -allphases = public, draft, secret = range(3)
> +INTERNAL_FLAG = 64 # Phases for mercurial internal usage only
> +HIDEABLE_FLAG = 32 # Phases that are hideable

Do we allocate large blocks for these phases so that we can add more similar
phases without introducing new repository-level requirement?

I didn't find any problem in this series, but I didn't follow the previous
discussion at all, so I leave it to someone who has expertise.
Augie Fackler - Aug. 28, 2018, 5:47 p.m.
> On Aug 28, 2018, at 8:05 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> Do we allocate large blocks for these phases so that we can add more similar
> phases without introducing new repository-level requirement?
> 
> I didn't find any problem in this series, but I didn't follow the previous
> discussion at all, so I leave it to someone who has expertise.

That was the basic idea, yeah. We know we want an `internal` phase (for things like shelve and rebase temp commits), and in the process of that discussion the idea of an `archived` phase came up (for things you’d like to hang on to but not see on a day-to-day basis - today I have a directory full of bundles to solve that issue).

Patch

diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -123,11 +123,22 @@  from . import (
 
 _fphasesentry = struct.Struct('>i20s')
 
-allphases = public, draft, secret = range(3)
+INTERNAL_FLAG = 64 # Phases for mercurial internal usage only
+HIDEABLE_FLAG = 32 # Phases that are hideable
+
+# record phase index
+public, draft, secret = range(3)
+internal = INTERNAL_FLAG | HIDEABLE_FLAG
+allphases = range(internal + 1)
 trackedphases = allphases[1:]
-phasenames = ['public', 'draft', 'secret']
+# record phase names
+phasenames = [None] * len(allphases)
+phasenames[:3] = ['public', 'draft', 'secret']
+phasenames[internal] = 'internal'
+# record phase property
 mutablephases = tuple(allphases[1:])
 remotehiddenphases = tuple(allphases[2:])
+localhiddenphases = tuple(p for p in allphases if p & HIDEABLE_FLAG)
 
 def _readroots(repo, phasedefaults=None):
     """Read phase roots from disk
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -28,7 +28,10 @@  def hideablerevs(repo):
     branchmap (see mercurial.branchmap.subsettable), you cannot set "public"
     changesets as "hideable". Doing so would break multiple code assertions and
     lead to crashes."""
-    return obsolete.getrevs(repo, 'obsolete')
+    obsoletes = obsolete.getrevs(repo, 'obsolete')
+    internals = repo._phasecache.getrevset(repo, phases.localhiddenphases)
+    internals = frozenset(internals)
+    return obsoletes | internals
 
 def pinnedrevs(repo):
     """revisions blocking hidden changesets from being filtered
diff --git a/tests/test-phases.t b/tests/test-phases.t
--- a/tests/test-phases.t
+++ b/tests/test-phases.t
@@ -826,3 +826,51 @@  Try various actions. only the draft move
   rollback completed
   abort: pretxnclose-phase.nopublish_D hook exited with status 1
   [255]
+
+  $ cd ..
+
+Test for the "internal" phase
+=============================
+
+  $ hg init internal-phase
+  $ cd internal-phase
+  $ mkcommit A
+  test-debug-phase: new rev 0:  x -> 1
+  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:   -> draft
+
+Commit an internal changesets
+
+  $ echo B > B
+  $ hg add B
+  $ hg status
+  A B
+  $ hg --config "phases.new-commit=internal" commit -m "my test internal commit"
+  test-debug-phase: new rev 1:  x -> 96
+  test-hook-close-phase: c01c42dffc7f81223397e99652a0703f83e1c5ea:   -> internal
+
+Usual visibility rules apply when working directory parents
+
+  $ hg log -G -l 3
+  @  changeset:   1:c01c42dffc7f
+  |  tag:         tip
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     my test internal commit
+  |
+  o  changeset:   0:4a2df7238c3b
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     A
+  
+
+Commit is hidden as expected
+
+  $ hg up 0
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg log -G
+  @  changeset:   0:4a2df7238c3b
+     tag:         tip
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     A
+