Patchwork [5,of,6] hg: add updatetotally skipupdate to examine destination before actual update

login
register
mail settings
Submitter Katsunori FUJIWARA
Date March 11, 2016, 7:41 p.m.
Message ID <4ec341fd8f85ca07dc6f.1457725270@feefifofum>
Download mbox | patch
Permalink /patch/13811/
State Changes Requested
Headers show

Comments

Katsunori FUJIWARA - March 11, 2016, 7:41 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1457724942 -32400
#      Sat Mar 12 04:35:42 2016 +0900
# Node ID 4ec341fd8f85ca07dc6f99d82c155e9d559be390
# Parent  231ea15298afd3e1a4347791d4eb904742bf8243
hg: add updatetotally skipupdate to examine destination before actual update

This is useful for client, which wants to show any message before
(abortion of) actual updating, for example.
Pierre-Yves David - March 11, 2016, 11:14 p.m.
On 03/11/2016 07:41 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1457724942 -32400
> #      Sat Mar 12 04:35:42 2016 +0900
> # Node ID 4ec341fd8f85ca07dc6f99d82c155e9d559be390
> # Parent  231ea15298afd3e1a4347791d4eb904742bf8243
> hg: add updatetotally skipupdate to examine destination before actual update
>
> This is useful for client, which wants to show any message before
> (abortion of) actual updating, for example.

Patch 1-4 are on the clowncopter. Thanks

I'm deeply confused about why this is useful and why the complexity is 
worth it. Can you elaborate on this?
Katsunori FUJIWARA - March 12, 2016, 12:02 p.m.
At Fri, 11 Mar 2016 23:14:04 +0000,
Pierre-Yves David wrote:
> 
> 
> 
> On 03/11/2016 07:41 PM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1457724942 -32400
> > #      Sat Mar 12 04:35:42 2016 +0900
> > # Node ID 4ec341fd8f85ca07dc6f99d82c155e9d559be390
> > # Parent  231ea15298afd3e1a4347791d4eb904742bf8243
> > hg: add updatetotally skipupdate to examine destination before actual update
> >
> > This is useful for client, which wants to show any message before
> > (abortion of) actual updating, for example.
> 
> Patch 1-4 are on the clowncopter. Thanks
> 
> I'm deeply confused about why this is useful and why the complexity is 
> worth it. Can you elaborate on this?

"hg pull --rebase" and "hg fetch" skip actual updating procedure (=
merge.update()), if update destination is current parent itself.

(this series doesn't care "hg fetch", because corresponded code path
has issue and inefficiency to be fixed in my next series :-))

Because "hg pull --rebase" also shows "nothing to rebase" or "nothing
to rebase - updating instead" message according to the destination of
update, just introducing "skip if update isn't needed" flag isn't
useful for this purpose (at least, if current behavior should be
kept).

If invoking destupdate() twice is acceptable redundancy, this change
isn't needed.


BTW, there are commands below, which imply bare "hg update" action at
the end of them, and some of them omit updating procedure (including
hook invocation), if there is no descendant.

                     update to     inform about
  command            parent itself omission
  ================== ============= ============
  fetch (*1)           x            x
  pull --rebase (*1)   x            o
  pull --update        o            -
  unbundle --update    o            -

(*1) work as bare "hg update", only if there is no another branch head

Which is preferable behavior ? (or should we keep current behavior of
them ?)

IMHO, omission of updating to parent itself seems reasonable also for
pull/unbundle, because '--update' option of them is described as
"update to NEW branch head if changesets were pulled/unbundled".


> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - March 19, 2016, 6:19 p.m.
On 03/12/2016 04:02 AM, FUJIWARA Katsunori wrote:
>
> At Fri, 11 Mar 2016 23:14:04 +0000,
> Pierre-Yves David wrote:
>>
>>
>>
>> On 03/11/2016 07:41 PM, FUJIWARA Katsunori wrote:
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>> # Date 1457724942 -32400
>>> #      Sat Mar 12 04:35:42 2016 +0900
>>> # Node ID 4ec341fd8f85ca07dc6f99d82c155e9d559be390
>>> # Parent  231ea15298afd3e1a4347791d4eb904742bf8243
>>> hg: add updatetotally skipupdate to examine destination before actual update
>>>
>>> This is useful for client, which wants to show any message before
>>> (abortion of) actual updating, for example.
>>
>> Patch 1-4 are on the clowncopter. Thanks
>>
>> I'm deeply confused about why this is useful and why the complexity is
>> worth it. Can you elaborate on this?
>
> "hg pull --rebase" and "hg fetch" skip actual updating procedure (=
> merge.update()), if update destination is current parent itself.
>
> (this series doesn't care "hg fetch", because corresponded code path
> has issue and inefficiency to be fixed in my next series :-))
>
> Because "hg pull --rebase" also shows "nothing to rebase" or "nothing
> to rebase - updating instead" message according to the destination of
> update, just introducing "skip if update isn't needed" flag isn't
> useful for this purpose (at least, if current behavior should be
> kept).
>
> If invoking destupdate() twice is acceptable redundancy, this change
> isn't needed.
>
>
> BTW, there are commands below, which imply bare "hg update" action at
> the end of them, and some of them omit updating procedure (including
> hook invocation), if there is no descendant.
>
>                       update to     inform about
>    command            parent itself omission
>    ================== ============= ============
>    fetch (*1)           x            x
>    pull --rebase (*1)   x            o
>    pull --update        o            -
>    unbundle --update    o            -
>
> (*1) work as bare "hg update", only if there is no another branch head
>
> Which is preferable behavior ? (or should we keep current behavior of
> them ?)
>
> IMHO, omission of updating to parent itself seems reasonable also for
> pull/unbundle, because '--update' option of them is described as
> "update to NEW branch head if changesets were pulled/unbundled".

Okay I see why this is useful (could you plese update the docstring to 
reflect that?) and this seems sensible to be able to avoid the double 
dest computation (and therefore double locking).

I checked patch 6 and I see why this need to be a function (rebase issue 
different message in this case) but this feel very strange to me. How 
could `hg pull` fair in this case?

Lets chat about it at the sprint

Cheers,
Pierre-Yves David - March 20, 2016, 12:21 a.m.
On 03/19/2016 11:19 AM, Pierre-Yves David wrote:
>
>
> On 03/12/2016 04:02 AM, FUJIWARA Katsunori wrote:
>>
>> At Fri, 11 Mar 2016 23:14:04 +0000,
>> Pierre-Yves David wrote:
>>>
>>>
>>>
>>> On 03/11/2016 07:41 PM, FUJIWARA Katsunori wrote:
>>>> # HG changeset patch
>>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>>> # Date 1457724942 -32400
>>>> #      Sat Mar 12 04:35:42 2016 +0900
>>>> # Node ID 4ec341fd8f85ca07dc6f99d82c155e9d559be390
>>>> # Parent  231ea15298afd3e1a4347791d4eb904742bf8243
>>>> hg: add updatetotally skipupdate to examine destination before
>>>> actual update
>>>>
>>>> This is useful for client, which wants to show any message before
>>>> (abortion of) actual updating, for example.
>>>
>>> Patch 1-4 are on the clowncopter. Thanks
>>>
>>> I'm deeply confused about why this is useful and why the complexity is
>>> worth it. Can you elaborate on this?
>>
>> "hg pull --rebase" and "hg fetch" skip actual updating procedure (=
>> merge.update()), if update destination is current parent itself.
>>
>> (this series doesn't care "hg fetch", because corresponded code path
>> has issue and inefficiency to be fixed in my next series :-))
>>
>> Because "hg pull --rebase" also shows "nothing to rebase" or "nothing
>> to rebase - updating instead" message according to the destination of
>> update, just introducing "skip if update isn't needed" flag isn't
>> useful for this purpose (at least, if current behavior should be
>> kept).
>>
>> If invoking destupdate() twice is acceptable redundancy, this change
>> isn't needed.

We chatted about it at the sprint and as this only affect rebase, we'll 
use the redundancy approach.

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -698,7 +698,8 @@  def clean(repo, node, show_stats=True, q
 # naming conflict in updatetotally()
 _clean = clean
 
-def updatetotally(ui, repo, checkout, brev, clean=False, check=False):
+def updatetotally(ui, repo, checkout, brev, clean=False, check=False,
+                 skipupdate=None):
     """Update the working directory with extra care for non-file components
 
     This takes care of non-file components below:
@@ -711,6 +712,7 @@  def updatetotally(ui, repo, checkout, br
     :brev: a name, which might be a bookmark to be activated after updating
     :clean: whether changes in the working directory can be discarded
     :check: whether changes in the working directory should be checked
+    :skipupdate: func(destrev), which returns True if skip actual updating
 
     This returns whether conflict is detected at updating or not.
     """
@@ -720,6 +722,8 @@  def updatetotally(ui, repo, checkout, br
         if checkout is None:
             updata = destutil.destupdate(repo, clean=clean, check=check)
             checkout, movemarkfrom, brev = updata
+            if skipupdate and skipupdate(checkout):
+                return False
             warndest = True
 
         if clean: