[PATCH 1 of 4 V2] hg: combine the remainder of clean() and update() into updaterepo()

[Mads Kiilerich]

On 01/11/2013 04:20 AM, Matt Harbison wrote:
> Mads Kiilerich wrote:
>> On 01/06/2013 11:16 PM, Matt Harbison wrote:
>>> # HG changeset patch
>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>> # Date 1357448404 18000
>>> # Node ID a35f6d98d65f24d5d6f1a7928342b2bc479ef44c
>>> # Parent 2c1276825e938872ebc099c191eb202f0dbadfcc
>>> hg: combine the remainder of clean() and update() into updaterepo()
>>>
>>> This sets the stage for largefiles to override updaterepo() instead of
>>> clean()
>>> and update(), while not altering the order of the largefile changes
>>> output and
>>> the updated/merged/removed/unresolved output. The need to override
>>> updaterepo()
>>> is to allow largefiles in a subrepo to be updated when the subrepo is
>>> updated.
>>> This change also opens the door to subrepos printing their 
>>> statistics (it
>>> currently doesn't because it calls updaterepo() without setting
>>> showstats,
>>> having previously lacked the capability entirely).
>>>
>>> Note that the 'use hg resolve' line was never output for subrepos,
>>> because
>>> updaterepo() previously didn't print anything, and prior to 
>>> 17c030014ddf,
>>> subrepos were only cleaned. The message probably shouldn't occur with
>>> subrepos
>>> because of the '(l)ocal or (r)emote' prompt when merging.
>>>
>>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>>> --- a/mercurial/hg.py
>>> +++ b/mercurial/hg.py
>>> @@ -463,20 +463,24 @@
>>> repo.ui.status(_("%d files updated, %d files merged, "
>>> "%d files removed, %d files unresolved\n") % stats)
>>> -def updaterepo(repo, node, overwrite):
>>> +def updaterepo(repo, node, overwrite, showstats=False):
>>> """Update the working directory to node.
>>> When overwrite is set, changes are clobbered, merged else
>>> returns stats (see pydoc mercurial.merge.applyupdates)"""
>>> - return mergemod.update(repo, node, False, overwrite, None)
>>> + stats = mergemod.update(repo, node, False, overwrite, None)
>>> + if showstats:
>>> + _showstats(repo, stats)
>>> +
>>> + if not overwrite and stats[3]:
>>> + repo.ui.status(_("use 'hg resolve' to retry unresolved file 
>>> merges\n"))
>>
>> Wouldn't it be better to keep this message in update() ?
>
> Initially, that's where I left it, then I moved it thinking it might 
> be needed, and then when it didn't appear in the tests, I reasoned 
> that the (l)ocal/(r)emote prompt was why.
>
> I wasn't sure it mattered at that point so I left it, justifying it in 
> my mind by thinking someone might see how subrepos are calling 
> updaterepo(), add that in new code and miss the prompt.  I think 
> calling that directly is wrong, because clean() and update() are so 
> much more readable, but maybe it avoids future problems?
>
> I can change it back if you want, but I'm curious why you think one 
> way has merit over another?

updaterepo seems to be a more low level function that doesn't have as 
much ui interaction. The UI messages are added at a higher level (except 
for showstats which however is controlled directly). It seems nice to 
keep it that way.

We also want this message to appear after any largefiles messages. And 
we probably want another and more help message for subrepos.

> I thought about that too, and almost did it, but then thought that if 
> some extension somewhere called it, that code would break.  I figured 
> it was less risky this close to the freeze.

We are before freeze and the stable rules do not apply. And we are 
changing the semantics anyway ... and extensions should follow Mercurial 
anyway.

Anyway, I have given some input. Please resend if you for this or other 
reasons would propose different patches.

/Mads