( )⚙ D7705 phases: make the working directory consistently a draft

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7705

phases: make the working directory consistently a draft
ClosedPublic

Authored by rdamazio on Dec 19 2019, 3:34 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHG9c1fd975e9ac: phases: make the working directory consistently a draft
rHG2ab225377c27: phases: make the working directory consistently a draft
Summary

Before this change, hg log -r 'wdir() and public()' would return it.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

rdamazio created this revision.Dec 19 2019, 3:34 AM
marmoute added a subscriber: marmoute.Dec 19 2019, 8:25 AM

The working copy is not necessarly draft phase. For example, it will be secret is the working copy parent is secret. One can also control it using the phases.new-commit config.

The mercurial.phases.newcommitphase(ui) function can help you there.

marmoute requested changes to this revision.Dec 19 2019, 8:25 AM
This revision now requires changes to proceed.Dec 19 2019, 8:25 AM
rdamazio updated this revision to Diff 18921.Dec 23 2019, 9:04 PM
rdamazio added a comment.Dec 23 2019, 9:05 PM
In D7705#113383, @marmoute wrote:

The working copy is not necessarly draft phase. For example, it will be secret is the working copy parent is secret. One can also control it using the phases.new-commit config.
The mercurial.phases.newcommitphase(ui) function can help you there.

Thanks.
About newcommitphase - that only accounts for the config option, and it seems that the working directory doesn't change phase based on that in other parts of the code (e.g. in commitablectx), so I kept that consistent here.

marmoute added a comment.Dec 27 2019, 11:01 AM
In D7705#113667, @rdamazio wrote:
In D7705#113383, @marmoute wrote:

The working copy is not necessarly draft phase. For example, it will be secret is the working copy parent is secret. One can also control it using the phases.new-commit config.
The mercurial.phases.newcommitphase(ui) function can help you there.

Thanks.
About newcommitphase - that only accounts for the config option, and it seems that the working directory doesn't change phase based on that in other parts of the code (e.g. in commitablectx), so I kept that consistent here.

commitablectx code seems wrong here. I am sending a patch.

rdamazio added a comment.Dec 27 2019, 3:42 PM

commitablectx code seems wrong here. I am sending a patch.

Sure. Let me know if your patch is going to somehow cover the use case I'm trying to address here, or what else you'd like me to do with this patch (is it just a matter of adding more tests here to account for the config option mix as well, once you add that?).

marmoute added a comment.EditedDec 27 2019, 3:45 PM

I simply sent this: D7726. If am I following your changeset right, we should have wdir() match draft() or secret() according to the projected phase (config + parent)

rdamazio added a comment.Dec 27 2019, 6:21 PM
In D7705#113845, @marmoute wrote:

I simply sent this. If am I following your changeset right, we should have wdir() match draft() or secret() according to the projected phase (config + parent)

I don't understand what you're asking me to do here, can you clarify?

marmoute added a comment.EditedDec 28 2019, 8:55 AM
In D7705#114045, @rdamazio wrote:

I don't understand what you're asking me to do here, can you clarify?

In short we should have a test matching

$ hg log -r 'wdir() and secret()' -T '{phase}\n' --config phases.new-commit=secret
secret
rdamazio updated this revision to Diff 19050.Jan 6 2020, 7:56 PM
rdamazio added a comment.Jan 6 2020, 7:56 PM
In D7705#114157, @marmoute wrote:
In D7705#114045, @rdamazio wrote:

I don't understand what you're asking me to do here, can you clarify?

In short we should have a test matching

$ hg log -r 'wdir() and secret()' -T '{phase}\n' --config phases.new-commit=secret
secret

Done.

marmoute accepted this revision.Jan 7 2020, 9:53 AM

Looks good to me

rdamazio added a commit: rHG2ab225377c27: phases: make the working directory consistently a draft.Jan 8 2020, 11:50 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG2ab225377c27: phases: make the working directory consistently a draft (authored by rdamazio). · Explain Why
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Jan 9 2020, 8:54 AM

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

rdamazio added a comment.Jan 13 2020, 9:55 PM
In D7705#114943, @yuja wrote:

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

Good point, I'll send another change.

Revision Contents
Changeset List

PathPackages
Mmercurial/phases.py (19 lines)
Mtests/test-phases.t (9 lines)

Diff 18878

mercurial/phases.py

from .i18n import _ from .i18n import _
from .node import ( from .node import (
bin, bin,
hex, hex,
nullid, nullid,
nullrev, nullrev,
short, short,
wdirrev,
) )
from .pycompat import ( from .pycompat import (
getattr, getattr,
setattr, setattr,
) )
from . import ( from . import (
error, error,
pycompat, pycompat,
# might only need a union and post-filtering. # might only need a union and post-filtering.
if len(phases) == 1: if len(phases) == 1:
[p] = phases [p] = phases
revs = self._phasesets[p] revs = self._phasesets[p]
else: else:
revs = set.union(*[self._phasesets[p] for p in phases]) revs = set.union(*[self._phasesets[p] for p in phases])
if repo.changelog.filteredrevs: if repo.changelog.filteredrevs:
revs = revs - repo.changelog.filteredrevs revs = revs - repo.changelog.filteredrevs
if subset is None: if subset is None:
return smartset.baseset(revs) return smartset.baseset(revs)
else: else:
if wdirrev in subset and draft in phases:
# The working dir would never be in the cache, but it should
# be considered a draft - if it's in the subset being
# filtered for drafts, add it to the output.
revs.add(wdirrev)
return subset & smartset.baseset(revs) return subset & smartset.baseset(revs)
else: else:
# phases keeps all the *other* phases.
phases = set(allphases).difference(phases) phases = set(allphases).difference(phases)
if not phases: if not phases:
return smartset.fullreposet(repo) return smartset.fullreposet(repo)
# revs has the revisions in all *other* phases.
if len(phases) == 1: if len(phases) == 1:
[p] = phases [p] = phases
revs = self._phasesets[p] revs = self._phasesets[p]
else: else:
revs = set.union(*[self._phasesets[p] for p in phases]) revs = set.union(*[self._phasesets[p] for p in phases])
if subset is None: if subset is None:
subset = smartset.fullreposet(repo) subset = smartset.fullreposet(repo)
if not revs: if not revs:
return subset return subset
if wdirrev in subset and draft in phases:
# The working dir is in the subset being filtered, and draft is
# in the phases *not* being returned, so add it to the set of
# revisions to filter out.
revs.add(wdirrev)
return subset.filter(lambda r: r not in revs) return subset.filter(lambda r: r not in revs)
def copy(self): def copy(self):
# Shallow copy meant to ensure isolation in # Shallow copy meant to ensure isolation in
# advance/retractboundary(), nothing more. # advance/retractboundary(), nothing more.
ph = self.__class__(None, None, _load=False) ph = self.__class__(None, None, _load=False)
ph.phaseroots = self.phaseroots[:] ph.phaseroots = self.phaseroots[:]
ph.dirty = self.dirty ph.dirty = self.dirty
ph.opener = self.opener ph.opener = self.opener
ph._loadedrevslen = self._loadedrevslen ph._loadedrevslen = self._loadedrevslen

tests/test-phases.t

$ mkcommit B $ mkcommit B
test-debug-phase: new rev 1: x -> 1 test-debug-phase: new rev 1: x -> 1
test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56: -> draft test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56: -> draft
$ hglog $ hglog
1 1 B 1 1 B
0 1 A 0 1 A
Working directory is a 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'
Draft commit are properly created over public one: Draft commit are properly created over public one:
$ hg phase --public . $ hg phase --public .
test-debug-phase: move rev 0: 1 -> 0 test-debug-phase: move rev 0: 1 -> 0
test-debug-phase: move rev 1: 1 -> 0 test-debug-phase: move rev 1: 1 -> 0
test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256: draft -> public test-hook-close-phase: 4a2df7238c3b48766b5e22fafbb8a2f506ec8256: draft -> public
test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56: draft -> public test-hook-close-phase: 27547f69f25460a52fff66ad004e58da7ad3fb56: draft -> public
$ hg phase $ hg phase