Patchwork D7705: phases: make the working directory consistently a draft

login
register
mail settings
Submitter phabricator
Date Jan. 8, 2020, 4:50 p.m.
Message ID <753f1d369bb66d1988d87d355075f77f@localhost.localdomain>
Download mbox | patch
Permalink /patch/44178/
State Not Applicable
Headers show

Comments

phabricator - Jan. 8, 2020, 4:50 p.m.
Closed by commit rHG2ab225377c27: phases: make the working directory consistently a draft (authored by rdamazio).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7705?vs=19050&id=19067

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7705/new/

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

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

CHANGE DETAILS




To: rdamazio, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
Yuya Nishihara - Jan. 9, 2020, 1:51 p.m.
> @@ -252,25 +254,44 @@
>                  revs = set.union(*[self._phasesets[p] for p in phases])
>              if repo.changelog.filteredrevs:
>                  revs = revs - repo.changelog.filteredrevs
> +
>              if subset is None:
>                  return smartset.baseset(revs)
>              else:
> +                if wdirrev in subset and repo[None].phase() in phases:
> +                    # The working dir would never be in the cache, but it was
> +                    # in the subset being filtered for its phase, so add it to
> +                    # the output.
> +                    revs.add(wdirrev)

Suppose `self._phasesets[]` is a read-only object in this method,
I think `revs` shouldn't be mutated if `revs is self._phasesets[p]`.

> +            if wdirrev in subset and repo[None].phase() in phases:
> +                # The working dir is in the subset being filtered, and its
> +                # phase is in the phases *not* being returned, so add it to the
> +                # set of revisions to filter out.
> +                revs.add(wdirrev)

Same here.
phabricator - Jan. 9, 2020, 1:54 p.m.
yuja added a comment.


  > @@ -252,25 +254,44 @@
  >
  >       revs = set.union(*[self._phasesets[p] for p in phases])
  >   if repo.changelog.filteredrevs:
  >       revs = revs - repo.changelog.filteredrevs
  >
  > +
  >
  >   if subset is None:
  >       return smartset.baseset(revs)
  >   else:
  >
  > +                if wdirrev in subset and repo[None].phase() in phases:
  > +                    # The working dir would never be in the cache, but it was
  > +                    # in the subset being filtered for its phase, so add it to
  > +                    # the output.
  > +                    revs.add(wdirrev)
  
  Suppose `self._phasesets[]` is a read-only object in this method,
  I think `revs` shouldn't be mutated if `revs is self._phasesets[p]`.
  
  > +            if wdirrev in subset and repo[None].phase() in phases:
  > +                # The working dir is in the subset being filtered, and its
  > +                # phase is in the phases *not* being returned, so add it to the
  > +                # set of revisions to filter out.
  > +                revs.add(wdirrev)
  
  Same here.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7705/new/

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

To: rdamazio, #hg-reviewers, marmoute
Cc: yuja, marmoute, 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
@@ -48,13 +48,58 @@ 
   1 1 B
   0 1 A
 
-Draft commit are properly created over public one:
+Working directory phase is secret when its parent is secret.
+
+  $ hg phase --force --secret .
+  test-debug-phase: move rev 0: 1 -> 2
+  test-debug-phase: move rev 1: 1 -> 2
+  test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:  draft -> secret
+  test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56:  draft -> secret
+  $ hg log -r 'wdir()' -T '{phase}\n'
+  secret
+  $ hg log -r 'wdir() and public()' -T '{phase}\n'
+  $ hg log -r 'wdir() and draft()' -T '{phase}\n'
+  $ hg log -r 'wdir() and secret()' -T '{phase}\n'
+  secret
+
+Working directory phase is draft when its parent is draft.
+
+  $ hg phase --draft .
+  test-debug-phase: move rev 1: 2 -> 1
+  test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56:  secret -> draft
+  $ hg log -r 'wdir()' -T '{phase}\n'
+  draft
+  $ hg log -r 'wdir() and public()' -T '{phase}\n'
+  $ hg log -r 'wdir() and draft()' -T '{phase}\n'
+  draft
+  $ hg log -r 'wdir() and secret()' -T '{phase}\n'
+
+Working directory phase is secret when a new commit will be created as secret,
+even if the parent is draft.
+
+  $ hg log -r 'wdir() and secret()' -T '{phase}\n' \
+  > --config phases.new-commit='secret'
+  secret
+
+Working directory phase is draft when its parent is public.
 
   $ hg phase --public .
   test-debug-phase: move rev 0: 1 -> 0
   test-debug-phase: move rev 1: 1 -> 0
   test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256:  draft -> public
   test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56:  draft -> public
+  $ hg log -r 'wdir()' -T '{phase}\n'
+  draft
+  $ hg log -r 'wdir() and public()' -T '{phase}\n'
+  $ hg log -r 'wdir() and draft()' -T '{phase}\n'
+  draft
+  $ hg log -r 'wdir() and secret()' -T '{phase}\n'
+  $ hg log -r 'wdir() and secret()' -T '{phase}\n' \
+  > --config phases.new-commit='secret'
+  secret
+
+Draft commit are properly created over public one:
+
   $ hg phase
   1: public
   $ hglog
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -112,6 +112,7 @@ 
     nullid,
     nullrev,
     short,
+    wdirrev,
 )
 from .pycompat import (
     getattr,
@@ -242,6 +243,7 @@ 
         """return a smartset for the given phases"""
         self.loadphaserevs(repo)  # ensure phase's sets are loaded
         phases = set(phases)
+
         if public not in phases:
             # fast path: _phasesets contains the interesting sets,
             # might only need a union and post-filtering.
@@ -252,25 +254,44 @@ 
                 revs = set.union(*[self._phasesets[p] for p in phases])
             if repo.changelog.filteredrevs:
                 revs = revs - repo.changelog.filteredrevs
+
             if subset is None:
                 return smartset.baseset(revs)
             else:
+                if wdirrev in subset and repo[None].phase() in phases:
+                    # The working dir would never be in the cache, but it was
+                    # in the subset being filtered for its phase, so add it to
+                    # the output.
+                    revs.add(wdirrev)
+
                 return subset & smartset.baseset(revs)
         else:
+            # phases keeps all the *other* phases.
             phases = set(allphases).difference(phases)
             if not phases:
                 return smartset.fullreposet(repo)
+
+            # revs has the revisions in all *other* phases.
             if len(phases) == 1:
                 [p] = phases
                 revs = self._phasesets[p]
             else:
                 revs = set.union(*[self._phasesets[p] for p in phases])
+
             if subset is None:
                 subset = smartset.fullreposet(repo)
+
+            if wdirrev in subset and repo[None].phase() in phases:
+                # The working dir is in the subset being filtered, and its
+                # phase is in the phases *not* being returned, so add it to the
+                # set of revisions to filter out.
+                revs.add(wdirrev)
+
             if not revs:
                 return subset
             return subset.filter(lambda r: r not in revs)
 
+
     def copy(self):
         # Shallow copy meant to ensure isolation in
         # advance/retractboundary(), nothing more.