Patchwork D3187: phase: Add dry-run functionality

login
register
mail settings
Submitter phabricator
Date April 7, 2018, 11:33 a.m.
Message ID <differential-rev-PHID-DREV-3fqipp7hvgyqnxt4ss3d-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30540/
State New
Headers show

Comments

phabricator - April 7, 2018, 11:33 a.m.
khanchi97 created this revision.
khanchi97 added a reviewer: pulkit.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Added the logic to count the number of csets whose phases will be changed,
  for now it just prints the same stats as we get without dry-run, remaining
  work is to show the which cset and what phase to be changed.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/commands.py
  tests/test-phases.t

CHANGE DETAILS




To: khanchi97, pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - April 8, 2018, 5:58 a.m.
av6 added inline comments.

INLINE COMMENTS

> commands.py:3872
> +        if opts['dry_run']:
> +            raise error.Abort(_("can't use --dryrun when no targetphase"))
>          # display

`"cannot use --dry-run without target phase"`

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - April 10, 2018, 9:42 a.m.
khanchi97 added a comment.


  @pulkit Can you please review it?

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - May 20, 2018, 7:36 p.m.
pulkit added inline comments.

INLINE COMMENTS

> commands.py:3898
>                      if newdata[cl.rev(n)] < targetphase]
> +        if opts['dry_run']:
> +            rejected = []

Since all the logic is copy-pasted from phases.advanceboundary and phases.retractboundary, why not we just pass the `--dry-run` option there and get the count from those functions only.

> test-phases.t:990
> +  $ hg phase --draft 0 5 --force -n
> +  phase changed for 4 changesets
> +

This is not very much helpful. In such cases, how about showing the range of changesets, something like:

  f7b1eb17ad24::925d80f479bb public -> draft
  b385d13d5ed4::fdc0253c25cf secret -> draft

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - May 24, 2018, 10:42 a.m.
khanchi97 added a comment.


  okay, but I have some queries like
  
  1. How about showing revision no. instead of cset id?
  2. And in this https://pastebin.com/raw/kWcr9xVK example if I change revision 2 phase to --secret then how it should print the range, I mean now we have branches in this range?
  
  Can we show that range like this:
  
  0316ce92851d : : b385d13d5ed4  draft  ->  secret
  0316ce92851d : : 4ccc844d5454   draft  -> secret

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - May 24, 2018, 9:19 p.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D3187#57349, @khanchi97 wrote:
  
  > okay, but I have some queries like
  >
  > 1. How about showing revision no. instead of cset id?
  
  
  I think we should show cset ids. If you want to with rev numbers, go with that. This should not be a blocker to get the initial patch in.
  
  > 2. And in this https://pastebin.com/raw/kWcr9xVK example if I change revision 2 phase to --secret then how it should print the range, I mean now we have branches in this range?
  > 
  >   Can we show that range like this:
  > 
  >   0316ce92851d : : b385d13d5ed4  draft  ->  secret 0316ce92851d : : 4ccc844d5454   draft  -> secret
  
  There are two ways you can show the range in this case:
  
  1. 0316ce92851d::b385d13d5ed4 and f19b7f89f44e::4ccc844d5454
  2. 0316ce92851d::4ccc844d5454 and b385d13d5ed4
  
  Look into `hg help revsets` to understand what `::` means.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - May 29, 2018, 8:55 a.m.
khanchi97 added a comment.


  @pulkit Now I moved the logic for finding revs (those phase will change)  to advanceboundry and retractboundry function and for now it shows all nodes. I am working to show range instead. Take a look at this when you are free :)

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel
phabricator - May 29, 2018, 7:35 p.m.
pulkit added a comment.


  Nice work! I like where how you simplified things from previous version.
  
  If you look at the patch, this seems to do multiple things at once which are:
  
  - adding dry-run argument to advanceboundary and retractboundary functions
  - adding a new --dry-run flag to hg phase
  
  Let's break things up in individual patches so that we can improve more on individual pieces. Can you split this into a separate patch for adding dryrun argument to each function, i.e. one patch for adcanceboundary, one for retractboundary, and then one last patch which adds the new flag? That will be very much helpful in reviewing this work and also making incremental changes and moving forward step by step.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, pulkit, #hg-reviewers
Cc: av6, mercurial-devel

Patch

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,217 @@ 
   rollback completed
   abort: pretxnclose-phase.nopublish_D hook exited with status 1
   [255]
+
+Test dry-run functionality
+
+  $ hg init dryrunrepo
+  $ cd dryrunrepo
+  $ echo a > a
+  $ hg ci -qAm 0
+  test-debug-phase: new rev 0:  x -> 1
+  test-hook-close-phase: f7b1eb17ad24730a1651fccd46c43826d1bbc2ac:   -> draft
+  $ echo b > b
+  $ hg ci -qAm 1
+  test-debug-phase: new rev 1:  x -> 1
+  test-hook-close-phase: 925d80f479bb026b0fb3deb27503780b13f74123:   -> draft
+  $ echo c > c
+  $ hg ci -qAm 2
+  test-debug-phase: new rev 2:  x -> 1
+  test-hook-close-phase: 0316ce92851d493393d2181900388caa05d256c3:   -> draft
+  $ echo d > d
+  $ hg ci -qAm 3
+  test-debug-phase: new rev 3:  x -> 1
+  test-hook-close-phase: 14b465a7e25bf201e963c055be0e780414cff648:   -> draft
+  $ echo e > e
+  $ hg ci -qAm 4
+  test-debug-phase: new rev 4:  x -> 1
+  test-hook-close-phase: b385d13d5ed4ceb2b67ced172470734a60187cd1:   -> draft
+  $ echo f > f
+  $ hg ci -qAm 5
+  test-debug-phase: new rev 5:  x -> 1
+  test-hook-close-phase: fdc0253c25cfd67fe42b7be81e3abc9f92bebbd5:   -> draft
+  $ hg up 3 -q
+  $ echo g > g
+  $ hg ci -qAm 6
+  test-debug-phase: new rev 6:  x -> 1
+  test-hook-close-phase: f19b7f89f44eee9ffe34ba58b4e4ee3b3cea1f34:   -> draft
+  $ echo h > h
+  $ hg ci -qAm 7
+  test-debug-phase: new rev 7:  x -> 1
+  test-hook-close-phase: 4ccc844d545402eb0f39cd294227cd38de3ece20:   -> draft
+  $ hg log -G -T phases
+  @  changeset:   7:4ccc844d5454
+  |  tag:         tip
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     7
+  |
+  o  changeset:   6:f19b7f89f44e
+  |  phase:       draft
+  |  parent:      3:14b465a7e25b
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     6
+  |
+  | o  changeset:   5:fdc0253c25cf
+  | |  phase:       draft
+  | |  user:        test
+  | |  date:        Thu Jan 01 00:00:00 1970 +0000
+  | |  summary:     5
+  | |
+  | o  changeset:   4:b385d13d5ed4
+  |/   phase:       draft
+  |    user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     4
+  |
+  o  changeset:   3:14b465a7e25b
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     3
+  |
+  o  changeset:   2:0316ce92851d
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     2
+  |
+  o  changeset:   1:925d80f479bb
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     1
+  |
+  o  changeset:   0:f7b1eb17ad24
+     phase:       draft
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     0
+  
+
+  $ hg phase --public 1
+  test-debug-phase: move rev 0: 1 -> 0
+  test-debug-phase: move rev 1: 1 -> 0
+  test-hook-close-phase: f7b1eb17ad24730a1651fccd46c43826d1bbc2ac:  draft -> public
+  test-hook-close-phase: 925d80f479bb026b0fb3deb27503780b13f74123:  draft -> public
+  $ hg phase --secret 4 --force
+  test-debug-phase: move rev 2: 1 -> 2
+  test-debug-phase: move rev 3: 1 -> 2
+  test-debug-phase: move rev 4: 1 -> 2
+  test-debug-phase: move rev 5: 1 -> 2
+  test-hook-close-phase: 0316ce92851d493393d2181900388caa05d256c3:  draft -> secret
+  test-hook-close-phase: 14b465a7e25bf201e963c055be0e780414cff648:  draft -> secret
+  test-hook-close-phase: b385d13d5ed4ceb2b67ced172470734a60187cd1:  draft -> secret
+  test-hook-close-phase: fdc0253c25cfd67fe42b7be81e3abc9f92bebbd5:  draft -> secret
+
+  $ hg log -G -T phases
+  @  changeset:   7:4ccc844d5454
+  |  tag:         tip
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     7
+  |
+  o  changeset:   6:f19b7f89f44e
+  |  phase:       draft
+  |  parent:      3:14b465a7e25b
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     6
+  |
+  | o  changeset:   5:fdc0253c25cf
+  | |  phase:       secret
+  | |  user:        test
+  | |  date:        Thu Jan 01 00:00:00 1970 +0000
+  | |  summary:     5
+  | |
+  | o  changeset:   4:b385d13d5ed4
+  |/   phase:       secret
+  |    user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     4
+  |
+  o  changeset:   3:14b465a7e25b
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     3
+  |
+  o  changeset:   2:0316ce92851d
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     2
+  |
+  o  changeset:   1:925d80f479bb
+  |  phase:       public
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     1
+  |
+  o  changeset:   0:f7b1eb17ad24
+     phase:       public
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     0
+  
+  $ hg phase --draft 0 5 -n
+  cannot move 1 changesets to a higher phase, use --force
+  phase changed for 2 changesets
+  [1]
+  $ hg phase --draft 0 5 --force -n
+  phase changed for 4 changesets
+
+  $ hg log -G -T phases
+  @  changeset:   7:4ccc844d5454
+  |  tag:         tip
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     7
+  |
+  o  changeset:   6:f19b7f89f44e
+  |  phase:       draft
+  |  parent:      3:14b465a7e25b
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     6
+  |
+  | o  changeset:   5:fdc0253c25cf
+  | |  phase:       secret
+  | |  user:        test
+  | |  date:        Thu Jan 01 00:00:00 1970 +0000
+  | |  summary:     5
+  | |
+  | o  changeset:   4:b385d13d5ed4
+  |/   phase:       secret
+  |    user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    summary:     4
+  |
+  o  changeset:   3:14b465a7e25b
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     3
+  |
+  o  changeset:   2:0316ce92851d
+  |  phase:       draft
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     2
+  |
+  o  changeset:   1:925d80f479bb
+  |  phase:       public
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     1
+  |
+  o  changeset:   0:f7b1eb17ad24
+     phase:       public
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     0
+  
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3828,7 +3828,7 @@ 
      ('s', 'secret', False, _('set changeset phase to secret')),
      ('f', 'force', False, _('allow to move boundary backward')),
      ('r', 'rev', [], _('target revision'), _('REV')),
-    ],
+    ] + dryrunopts,
     _('[-p|-d|-s] [-f] [-r] [REV...]'))
 def phase(ui, repo, *revs, **opts):
     """set or show the current phase name
@@ -3868,6 +3868,8 @@ 
 
     ret = 0
     if targetphase is None:
+        if opts['dry_run']:
+            raise error.Abort(_("can't use --dryrun when no targetphase"))
         # display
         for r in revs:
             ctx = repo[r]
@@ -3883,22 +3885,45 @@ 
             unfi = repo.unfiltered()
             getphase = unfi._phasecache.phase
             olddata = [getphase(unfi, r) for r in unfi]
-            phases.advanceboundary(repo, tr, targetphase, nodes)
-            if opts['force']:
-                phases.retractboundary(repo, tr, targetphase, nodes)
+            if not opts['dry_run']:
+                phases.advanceboundary(repo, tr, targetphase, nodes)
+                if opts['force']:
+                    phases.retractboundary(repo, tr, targetphase, nodes)
         getphase = unfi._phasecache.phase
         newdata = [getphase(unfi, r) for r in unfi]
         changes = sum(newdata[r] != olddata[r] for r in unfi)
         cl = unfi.changelog
         rejected = [n for n in nodes
                     if newdata[cl.rev(n)] < targetphase]
+        if opts['dry_run']:
+            rejected = []
+            if not opts['force']:
+                rejected = [n for n in nodes
+                            if olddata[cl.rev(n)] < targetphase]
+            affected = set()
+            allphases = range(3)
+            phcache = repo._phasecache.copy()
+            for phase in xrange(targetphase + 1, len(allphases)):
+                nds = [n for n in nodes
+                       if phcache.phase(repo, repo[n].rev()) >= phase]
+                if not nds:
+                    break
+                oldroots = repo._phasecache.phaseroots[phase]
+                affected.update(unfi.revs('%ln::%ln', oldroots, nds))
+            if opts['force']:
+                nds = [n for n in nodes
+                       if phcache.phase(repo, repo[n].rev()) < targetphase]
+                oldroots = repo._phasecache.phaseroots[targetphase]
+                affected.update(unfi.revs('(%ln::) - (%ln::)', nds,
+                                          oldroots))
+            changes = len(affected)
         if rejected:
             ui.warn(_('cannot move %i changesets to a higher '
                       'phase, use --force\n') % len(rejected))
             ret = 1
         if changes:
             msg = _('phase changed for %i changesets\n') % changes
-            if ret:
+            if ret or opts['dry_run']:
                 ui.status(msg)
             else:
                 ui.note(msg)