| 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
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?
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
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,
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: