On 12/07/2016 04:50 AM, Jeremy Wall (zaphar) wrote:
# HG changeset patch
# User Jeremy Wall (zaphar) <jer...@marzhillstudios.com>
# Date 1480542942 21600
# Wed Nov 30 15:55:42 2016 -0600
# Node ID 6e1ad0fb4f792d01e75f18e41579e52bcceee198
# Parent 9e29d4e4e08b5996adda49cdd0b497d89e2b16ee
push: add a message when pushing phases but not changes (issue4232)
Thanks for looking into this, there is interesting bit in that proposal
and it certainly bootstrap the discussion in a meaningful way.
First let me inflict you some generic tough about the BC concerns:
------------------------------------------------------------------
One of main design idea around phases (back in 2011) was that user who
doesn't need to know about them should not know about them. Typically,
user using Mercurial before phase and who have not opted in any new
feature since then should not see them (With the obvious exception of
when phases prevent them to rewrite exchanged history).
This principle can be extended to the current work on the message,
people doing standard interaction with publishing server are not moving
phases outside of the changeset they are pushing and pulling, so they
don't need extra message and it is better to keep the output undisturbed
in this case.
On the other hand, when user start to move toward less "classical" usage
that explicitly involved phases (like interacting with a non-publishing
server) it seems all right to introduce some output changes. Such logic
around output (and behavior) changes also apply to things as our current
"topic" experiment, people opt-in in the new feature can be exposed to
message and behavior change.
The good new, is that your current change seems mostly in line with this
principle as the new message will only be included only if we have phase
changes without changesets exchange, something rare if you stick to the
default publishing server setup.
About your new output
---------------------
It is usually a good practice to include sample of your new output in
the the changeset description.
Scanning through your test change I seems to pick two possibles new output.
> + sending phase public for b740e3e5c05d
> + sending phase draft for 967b449fbc94
> + updated 2 phase boundaries
It is unclear to me what triggers each of them, so I went and looked at
the code change in more details. I can't find trace of the first two
anywhere. Are these stalled test change your forgot to update?
I'm not too excited about the new message "updated 2 phase boundaries".
I'm afraid "phase boundary" is a term a bit too confusing for user. In
addition, it is a bit too vagues, "3 phase boundary" can mean 3
changesets changed phase as much as a couple thousand.
I've not though too much about it yet, but I would lean more toward
something like:
42 changesets moved to public phases,
We could also try to leverage the experimental 'journal' extension to
give the option to peek into more data about the phase changes.
About your implementation
-------------------------
I see a couple of issue with the implementation of your proposal,
First, the code introduced to handle "phase change report" live in a
function (_pushcheckoutgoing) that live in the "pushing changesets"
logic. So that seems wrong and we needs that phase related code in a
more phases related code path (or generic one).
Second, your phase movement message is only triggered if there is no
changeset pushed. We should also mention phases move for changeset that
are not included in the push. We need logic outside of a 'if not
outgoing.missing:' section to compute which changesets we move phase for
and how they overlap with the outgoing changesets.
About my interest in this issue
-------------------------------
I'm currently looking into the general issue of reporting more data
about these date who currently move silently. This recently got in my
todo-list for a paying customer (hooray). So I was planning to make
progress on this over the coming handful of weeks.
If you want use to work together on the topic I'm very open to it. In
all cases I will do my best to help something to happen on that topic.
Taking a step back: reporting more data in general
--------------------------------------------------
Your changeset focus on reporting phases change pushed to the server
during a push. We have a whole set of related movement currently unreported:
- local phase movement on push,
- phase movement on pull,
- not phase data in incoming/outgoing
And we even more unreported data when you start taking things like
obsolescence marker into account.
At that point, it is probably a good idea to take a step back and start
a Plan page on the wiki to gather all things we want to report and thing
about a unified rework of there reporting across commands. Let me know
if you are interested in looking into that.
I've made a couple of extra comments inline the patch
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 mercurial/exchange.py
--- a/mercurial/exchange.py Tue Nov 29 04:11:05 2016 -0800
+++ b/mercurial/exchange.py Wed Nov 30 15:55:42 2016 -0600
@@ -14,6 +14,7 @@
from .node import (
hex,
nullid,
+ short,
)
from . import (
base85,
@@ -643,9 +644,16 @@
def _pushcheckoutgoing(pushop):
outgoing = pushop.outgoing
unfi = pushop.repo.unfiltered()
+ ui = unfi.ui
if not outgoing.missing:
- # nothing to push
- scmutil.nochangesfound(unfi.ui, unfi, outgoing.excluded)
+ # phases to push
+ if pushop.outdatedphases:
+ ui.status(_("updated %d phase boundaries\n") %
+ len(pushop.outdatedphases))
+ # TODO(jeremy): Do I still return false?
+ else:
+ # nothing to push
+ scmutil.nochangesfound(ui, unfi, outgoing.excluded)
This is a part I've been talking about earlier, That function belong to
the code path pushing -changesets-, reporting information about phases
here seems wrong.
It looks like we need a bigger refactoring of the area.
return False
# something to push
if not pushop.force:
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 tests/test-phases-exchange.t
--- a/tests/test-phases-exchange.t Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-phases-exchange.t Wed Nov 30 15:55:42 2016 -0600
@@ -384,7 +384,7 @@
$ hg push ../alpha # from nu
pushing to ../alpha
searching for changes
- no changes found
+ sending phase public for 145e75495359
I don't find the code responsible for this, is this an outdated test
output or am I missing something ?
[1]
$ cd ..
$ cd alpha
@@ -600,7 +600,7 @@
$ hg push ../alpha
pushing to ../alpha
searching for changes
- no changes found
+ sending phase public for b740e3e5c05d
[1]
$ hgph
o 6 public a-F - b740e3e5c05d
@@ -705,7 +705,7 @@
$ hg push ../alpha
pushing to ../alpha
searching for changes
- no changes found
+ sending phase draft for 967b449fbc94
[1]
$ hgph
o 9 public a-H - 967b449fbc94
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 tests/test-push-warn.t
--- a/tests/test-push-warn.t Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-push-warn.t Wed Nov 30 15:55:42 2016 -0600
@@ -125,7 +125,7 @@
$ hg push -r 2 ../c
pushing to ../c
searching for changes
- no changes found
+ updated 1 phase boundaries
[1]
$ hg push -r 3 ../c
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 tests/test-subrepo.t
--- a/tests/test-subrepo.t Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-subrepo.t Wed Nov 30 15:55:42 2016 -0600
@@ -1519,7 +1519,13 @@
(issue3781)
$ cp -r main issue3781
+ cp: main/.hg/cache/checklink: No such file or directory
+ cp: main/s/.hg/cache/checklink: No such file or directory
+ [1]
$ cp -r main issue3781-dest
+ cp: main/.hg/cache/checklink: No such file or directory
+ cp: main/s/.hg/cache/checklink: No such file or directory
+ [1]
These line are suspicious. What version did you used to build your tests?
$ cd issue3781-dest/s
$ hg phase tip # show we have draft changeset
5: draft
@@ -1535,7 +1541,7 @@
searching for changes
no changes found
searching for changes
- no changes found
+ updated 2 phase boundaries
[1]
# clean the push cache
$ rm s/.hg/cache/storehash/*
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 tests/test-treediscovery-legacy.t
--- a/tests/test-treediscovery-legacy.t Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-treediscovery-legacy.t Wed Nov 30 15:55:42 2016 -0600
@@ -115,7 +115,7 @@
$ hg push $remote
pushing to http://localhost:$HGPORT/
searching for changes
- no changes found
+ updated 1 phase boundaries
[1]
$ cd ..
diff -r 9e29d4e4e08b -r 6e1ad0fb4f79 tests/test-treediscovery.t
--- a/tests/test-treediscovery.t Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-treediscovery.t Wed Nov 30 15:55:42 2016 -0600
@@ -104,7 +104,7 @@
$ hg push $remote
pushing to http://localhost:$HGPORT/
searching for changes
- no changes found
+ updated 1 phase boundaries
[1]
$ cd ..
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
