D3672: retractboundary: add dryrun parameter
This revision now requires changes to proceed. baymax added a comment. baymax requested changes to this revision. There seems to have been no activities on this Diff for the past 3 Months. By policy, we are automatically moving it out of the `need-review` state. Please, move it back to `need-review` without hesitation if this diff should still be discussed. :baymax:need-review-idle: REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D3672/new/ REVISION DETAIL https://phab.mercurial-scm.org/D3672 To: khanchi97, #hg-reviewers, baymax Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
pulkit added a subscriber: yuja. pulkit added inline comments. INLINE COMMENTS > khanchi97 wrote in phases.py:417 > Because I think this affected set is calculated after performing actions, > when phases are changed. And calculating affected using new phaseroots. Am I > missing something here? I think affected can be used here. Maybe @yuja might know better. @yuja we want to return a set of revs whose phase is changed by this function. Does affected looks like the correct set to you? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3672 To: khanchi97, #hg-reviewers Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
khanchi97 updated this revision to Diff 8960.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3672?vs=8959=8960
REVISION DETAIL
https://phab.mercurial-scm.org/D3672
AFFECTED FILES
mercurial/phases.py
CHANGE DETAILS
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -404,32 +404,46 @@
repo.invalidatevolatilesets()
return changes
-def retractboundary(self, repo, tr, targetphase, nodes):
+def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
+"""If dryrun is True, no actions will be performed
+
+Return a set of revs whose phase is changed or should be changed.
+"""
oldroots = self.phaseroots[:targetphase + 1]
if tr is None:
phasetracking = None
else:
phasetracking = tr.changes.get('phases')
repo = repo.unfiltered()
-if (self._retractboundary(repo, tr, targetphase, nodes)
-and phasetracking is not None):
+changes = set() # set of revisions whose phase can be changed.
-# find the affected revisions
-new = self.phaseroots[targetphase]
-old = oldroots[targetphase]
-affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+getphase = repo._phasecache.phase
+nds = [n for n in nodes
+ if getphase(repo, repo[n].rev()) < targetphase]
+targetphroots = oldroots[-1]
+affected = set(repo.revs('(%ln::) - (%ln::)', nds, targetphroots))
+changes.update(affected)
+if not dryrun:
+if (self._retractboundary(repo, tr, targetphase, nodes)
+and phasetracking is not None):
-# find the phase of the affected revision
-for phase in xrange(targetphase, -1, -1):
-if phase:
-roots = oldroots[phase]
-revs = set(repo.revs('%ln::%ld', roots, affected))
-affected -= revs
-else: # public phase
-revs = affected
-for r in revs:
-_trackphasechange(phasetracking, r, phase, targetphase)
-repo.invalidatevolatilesets()
+# find the affected revisions
+new = self.phaseroots[targetphase]
+old = oldroots[targetphase]
+affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+
+# find the phase of the affected revision
+for phase in xrange(targetphase, -1, -1):
+if phase:
+roots = oldroots[phase]
+revs = set(repo.revs('%ln::%ld', roots, affected))
+affected -= revs
+else: # public phase
+revs = affected
+for r in revs:
+_trackphasechange(phasetracking, r, phase, targetphase)
+repo.invalidatevolatilesets()
+return changes
def _retractboundary(self, repo, tr, targetphase, nodes):
# Be careful to preserve shallow-copied values: do not update
@@ -509,17 +523,25 @@
repo._phasecache.replace(phcache)
return changes
-def retractboundary(repo, tr, targetphase, nodes):
+def retractboundary(repo, tr, targetphase, nodes, dryrun=None):
"""Set nodes back to a phase changing other nodes phases if
necessary.
This function move boundary *backward* this means that all nodes
are set in the target phase or kept in a *higher* phase.
-Simplify boundary to contains phase roots only."""
+Simplify boundary to contains phase roots only.
+
+If dryrun is True, no actions will be performed
+
+Return a set of revs whose phase is changed or should be changed.
+"""
phcache = repo._phasecache.copy()
-phcache.retractboundary(repo, tr, targetphase, nodes)
-repo._phasecache.replace(phcache)
+changes = phcache.retractboundary(repo, tr, targetphase, nodes,
+ dryrun=dryrun)
+if not dryrun:
+repo._phasecache.replace(phcache)
+return changes
def registernew(repo, tr, targetphase, nodes):
"""register a new revision and its phase
To: khanchi97, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
khanchi97 updated this revision to Diff 8959.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3672?vs=8948=8959
REVISION DETAIL
https://phab.mercurial-scm.org/D3672
AFFECTED FILES
mercurial/phases.py
CHANGE DETAILS
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -404,32 +404,46 @@
repo.invalidatevolatilesets()
return changes
-def retractboundary(self, repo, tr, targetphase, nodes):
+def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
+"""If dryrun is True, no actions will be performed
+
+Return a set of revs whose phase is changed or should be changed.
+"""
oldroots = self.phaseroots[:targetphase + 1]
if tr is None:
phasetracking = None
else:
phasetracking = tr.changes.get('phases')
repo = repo.unfiltered()
-if (self._retractboundary(repo, tr, targetphase, nodes)
-and phasetracking is not None):
+changes = set() # set or revisions whose phase can be changed.
-# find the affected revisions
-new = self.phaseroots[targetphase]
-old = oldroots[targetphase]
-affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+getphase = repo._phasecache.phase
+nds = [n for n in nodes
+ if getphase(repo, repo[n].rev()) < targetphase]
+targetphroots = oldroots[-1]
+affected = set(repo.revs('(%ln::) - (%ln::)', nds, targetphroots))
+changes.update(affected)
+if not dryrun:
+if (self._retractboundary(repo, tr, targetphase, nodes)
+and phasetracking is not None):
-# find the phase of the affected revision
-for phase in xrange(targetphase, -1, -1):
-if phase:
-roots = oldroots[phase]
-revs = set(repo.revs('%ln::%ld', roots, affected))
-affected -= revs
-else: # public phase
-revs = affected
-for r in revs:
-_trackphasechange(phasetracking, r, phase, targetphase)
-repo.invalidatevolatilesets()
+# find the affected revisions
+new = self.phaseroots[targetphase]
+old = oldroots[targetphase]
+affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+
+# find the phase of the affected revision
+for phase in xrange(targetphase, -1, -1):
+if phase:
+roots = oldroots[phase]
+revs = set(repo.revs('%ln::%ld', roots, affected))
+affected -= revs
+else: # public phase
+revs = affected
+for r in revs:
+_trackphasechange(phasetracking, r, phase, targetphase)
+repo.invalidatevolatilesets()
+return changes
def _retractboundary(self, repo, tr, targetphase, nodes):
# Be careful to preserve shallow-copied values: do not update
@@ -509,17 +523,25 @@
repo._phasecache.replace(phcache)
return changes
-def retractboundary(repo, tr, targetphase, nodes):
+def retractboundary(repo, tr, targetphase, nodes, dryrun=None):
"""Set nodes back to a phase changing other nodes phases if
necessary.
This function move boundary *backward* this means that all nodes
are set in the target phase or kept in a *higher* phase.
-Simplify boundary to contains phase roots only."""
+Simplify boundary to contains phase roots only.
+
+If dryrun is True, no actions will be performed
+
+Return a set of revs whose phase is changed or should be changed.
+"""
phcache = repo._phasecache.copy()
-phcache.retractboundary(repo, tr, targetphase, nodes)
-repo._phasecache.replace(phcache)
+changes = phcache.retractboundary(repo, tr, targetphase, nodes,
+ dryrun=dryrun)
+if not dryrun:
+repo._phasecache.replace(phcache)
+return changes
def registernew(repo, tr, targetphase, nodes):
"""register a new revision and its phase
To: khanchi97, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
khanchi97 added inline comments. INLINE COMMENTS > pulkit wrote in phases.py:395 > Add documentation about dry-run and the return value. okay > pulkit wrote in phases.py:417 > Why are we not using this affected set here to find the changesets whose > phase is changed? Because I think this affected set is calculated after performing actions, when phases are changed. And calculating affected using new phaseroots. Am I missing something here? > pulkit wrote in phases.py:430 > (Not sure which line I should put this comment on) > > Just like https://phab.mercurial-scm.org/D3671, here also, the function > should return the correct set of changes whose phase have been changed > irrespective of the dryrun value passed. yeah, I got it. > pulkit wrote in phases.py:511 > This should better be: > > `If dryrun is True, no actions will be performed > > returns a set of revs whose phase is changed or should be changed` okay, will do this for 'advanceboundry' too REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3672 To: khanchi97, #hg-reviewers Cc: pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
pulkit added inline comments.
INLINE COMMENTS
> phases.py:395
>
> -def retractboundary(self, repo, tr, targetphase, nodes):
> +def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
> oldroots = self.phaseroots[:targetphase + 1]
Add documentation about dry-run and the return value.
> phases.py:417
> +old = oldroots[targetphase]
> +affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
> +
Why are we not using this affected set here to find the changesets whose phase
is changed?
> phases.py:430
> +repo.invalidatevolatilesets()
> +return changes
>
(Not sure which line I should put this comment on)
Just like https://phab.mercurial-scm.org/D3671, here also, the function should
return the correct set of changes whose phase have been changed irrespective of
the dryrun value passed.
> phases.py:511
> +
> +If dryrun is true then it will not perform any action and only returns
> +set of revision whose phase can be changed.
This should better be:
`If dryrun is True, no actions will be performed
returns a set of revs whose phase is changed or should be changed`
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3672
To: khanchi97, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
khanchi97 updated this revision to Diff 8948.
khanchi97 edited the summary of this revision.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3672?vs=8929=8948
REVISION DETAIL
https://phab.mercurial-scm.org/D3672
AFFECTED FILES
mercurial/phases.py
CHANGE DETAILS
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -392,32 +392,42 @@
self._retractboundary(repo, tr, targetphase, delroots)
repo.invalidatevolatilesets()
-def retractboundary(self, repo, tr, targetphase, nodes):
+def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
oldroots = self.phaseroots[:targetphase + 1]
if tr is None:
phasetracking = None
else:
phasetracking = tr.changes.get('phases')
repo = repo.unfiltered()
-if (self._retractboundary(repo, tr, targetphase, nodes)
-and phasetracking is not None):
-
-# find the affected revisions
-new = self.phaseroots[targetphase]
-old = oldroots[targetphase]
-affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+changes = set() # set or revisions whose phase can be changed.
+if dryrun:
+getphase = repo._phasecache.phase
+nds = [n for n in nodes
+ if getphase(repo, repo[n].rev()) < targetphase]
+targetphroots = oldroots[-1]
+affected = set(repo.revs('(%ln::) - (%ln::)', nds, targetphroots))
+changes.update(affected)
+else:
+if (self._retractboundary(repo, tr, targetphase, nodes)
+and phasetracking is not None):
-# find the phase of the affected revision
-for phase in xrange(targetphase, -1, -1):
-if phase:
-roots = oldroots[phase]
-revs = set(repo.revs('%ln::%ld', roots, affected))
-affected -= revs
-else: # public phase
-revs = affected
-for r in revs:
-_trackphasechange(phasetracking, r, phase, targetphase)
-repo.invalidatevolatilesets()
+# find the affected revisions
+new = self.phaseroots[targetphase]
+old = oldroots[targetphase]
+affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+
+# find the phase of the affected revision
+for phase in xrange(targetphase, -1, -1):
+if phase:
+roots = oldroots[phase]
+revs = set(repo.revs('%ln::%ld', roots, affected))
+affected -= revs
+else: # public phase
+revs = affected
+for r in revs:
+_trackphasechange(phasetracking, r, phase, targetphase)
+repo.invalidatevolatilesets()
+return changes
def _retractboundary(self, repo, tr, targetphase, nodes):
# Be careful to preserve shallow-copied values: do not update
@@ -489,17 +499,24 @@
phcache.advanceboundary(repo, tr, targetphase, nodes)
repo._phasecache.replace(phcache)
-def retractboundary(repo, tr, targetphase, nodes):
+def retractboundary(repo, tr, targetphase, nodes, dryrun=None):
"""Set nodes back to a phase changing other nodes phases if
necessary.
This function move boundary *backward* this means that all nodes
are set in the target phase or kept in a *higher* phase.
-Simplify boundary to contains phase roots only."""
+Simplify boundary to contains phase roots only.
+
+If dryrun is true then it will not perform any action and only returns
+set of revision whose phase can be changed.
+"""
phcache = repo._phasecache.copy()
-phcache.retractboundary(repo, tr, targetphase, nodes)
-repo._phasecache.replace(phcache)
+changes = phcache.retractboundary(repo, tr, targetphase, nodes,
+ dryrun=dryrun)
+if not dryrun:
+repo._phasecache.replace(phcache)
+return changes
def registernew(repo, tr, targetphase, nodes):
"""register a new revision and its phase
To: khanchi97, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3672: retractboundary: add dryrun parameter
khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Added the logic to find those csets whose phase will be changed
without dry-run while retracting boundary and return those csets.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3672
AFFECTED FILES
mercurial/phases.py
CHANGE DETAILS
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -392,32 +392,44 @@
self._retractboundary(repo, tr, targetphase, delroots)
repo.invalidatevolatilesets()
-def retractboundary(self, repo, tr, targetphase, nodes):
+def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
oldroots = self.phaseroots[:targetphase + 1]
if tr is None:
phasetracking = None
else:
phasetracking = tr.changes.get('phases')
repo = repo.unfiltered()
-if (self._retractboundary(repo, tr, targetphase, nodes)
-and phasetracking is not None):
-
-# find the affected revisions
-new = self.phaseroots[targetphase]
-old = oldroots[targetphase]
-affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+changes = [set(), set(), set()]
+if dryrun:
+getphase = repo._phasecache.phase
+nds = [n for n in nodes
+ if getphase(repo, repo[n].rev()) < targetphase]
+targetphroots = oldroots[-1]
+affected = set(repo.revs('(%ln::) - (%ln::)', nds, targetphroots))
+for rev in affected:
+revphase = getphase(repo, rev)
+changes[revphase].update((rev,))
+else:
+if (self._retractboundary(repo, tr, targetphase, nodes)
+and phasetracking is not None):
-# find the phase of the affected revision
-for phase in xrange(targetphase, -1, -1):
-if phase:
-roots = oldroots[phase]
-revs = set(repo.revs('%ln::%ld', roots, affected))
-affected -= revs
-else: # public phase
-revs = affected
-for r in revs:
-_trackphasechange(phasetracking, r, phase, targetphase)
-repo.invalidatevolatilesets()
+# find the affected revisions
+new = self.phaseroots[targetphase]
+old = oldroots[targetphase]
+affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
+
+# find the phase of the affected revision
+for phase in xrange(targetphase, -1, -1):
+if phase:
+roots = oldroots[phase]
+revs = set(repo.revs('%ln::%ld', roots, affected))
+affected -= revs
+else: # public phase
+revs = affected
+for r in revs:
+_trackphasechange(phasetracking, r, phase, targetphase)
+repo.invalidatevolatilesets()
+return changes
def _retractboundary(self, repo, tr, targetphase, nodes):
# Be careful to preserve shallow-copied values: do not update
@@ -489,17 +501,20 @@
phcache.advanceboundary(repo, tr, targetphase, nodes)
repo._phasecache.replace(phcache)
-def retractboundary(repo, tr, targetphase, nodes):
+def retractboundary(repo, tr, targetphase, nodes, dryrun=None):
"""Set nodes back to a phase changing other nodes phases if
necessary.
This function move boundary *backward* this means that all nodes
are set in the target phase or kept in a *higher* phase.
Simplify boundary to contains phase roots only."""
phcache = repo._phasecache.copy()
-phcache.retractboundary(repo, tr, targetphase, nodes)
-repo._phasecache.replace(phcache)
+changes = phcache.retractboundary(repo, tr, targetphase, nodes,
+ dryrun=dryrun)
+if not dryrun:
+repo._phasecache.replace(phcache)
+return changes
def registernew(repo, tr, targetphase, nodes):
"""register a new revision and its phase
To: khanchi97, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
