[PATCH 4 of 8] merge: change debug logging and progress
Mads Kiilerich
mads at kiilerich.com
Thu May 8 06:33:12 CDT 2014
On 05/07/2014 11:07 PM, Pierre-Yves David wrote:
>
>
> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1398125425 -7200
>> # Tue Apr 22 02:10:25 2014 +0200
>> # Branch stable
>> # Node ID feac4ee682bea8ebb19ead775bdb9ef413da8e3e
>> # Parent 28162b4271a8a4c4cdee6532c49aa5316f9133d5
>> merge: change debug logging and progress
>>
>> Preparing for action list split-up ...
>
> Description is not very helpful
There is not much to say. The patch moves debug statements around
without really changing anything. Arguably, it temporarily makes the
code worse. Then only justification is that it makes it easier to review
the test changes ... and in the end the 8th don't change test output at all.
>
>>
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>> --- a/mercurial/merge.py
>> +++ b/mercurial/merge.py
>> @@ -571,9 +571,9 @@ def getremove(repo, mctx, overwrite, arg
>> wwrite = repo.wwrite
>> audit = repo.wopener.audit
>> i = 0
>> - for arg in args:
>> - f = arg[0]
>> - if arg[1] == 'r':
>> + for f, m, args, msg in args:
>> + repo.ui.debug(" %s: %s -> %s\n" % (f, msg, m))
>> + if m == 'r':
>> if verbose:
>> repo.ui.note(_("removing %s\n") % f)
>> audit(f)
>> @@ -585,7 +585,7 @@ def getremove(repo, mctx, overwrite, arg
>> else:
>> if verbose:
>> repo.ui.note(_("getting %s\n") % f)
>> - wwrite(f, fctx(f).data(), arg[2][0])
>> + wwrite(f, fctx(f).data(), args[0])
>> if i == 100:
>> yield i, f
>> i = 0
>> @@ -612,12 +612,11 @@ def applyupdates(repo, actions, wctx, mc
>> # prescan for merges
>> for a in actions:
>> f, m, args, msg = a
>> - repo.ui.debug(" %s: %s -> %s\n" % (f, msg, m))
>> if m == "m": # merge
>> f1, f2, fa, move, anc = args
>> if f == '.hgsubstate': # merged internally
>> continue
>> - repo.ui.debug(" preserving %s for resolve of %s\n" %
>> (f1, f))
>> + repo.ui.debug(" preserving %s for resolve of %s\n" %
>> (f1, f))
>
> unexpected space change?
Not really. The debug statement will no longer have the single space
indented "->" before it. Having this message double indented will thus
no longer make sense.
>
>
>> fcl = wctx[f1]
>> fco = mctx[f2]
>> actx = repo[anc]
>> @@ -647,7 +646,7 @@ def applyupdates(repo, actions, wctx, mc
>> updated = len(updateactions)
>> removeactions = [a for a in workeractions if a[1] == 'r']
>> removed = len(removeactions)
>> - actions = [a for a in actions if a[1] not in 'grk']
>> + actions = [a for a in actions if a[1] not in 'gr']
>>
>> hgsub = [a[1] for a in workeractions if a[0] == '.hgsubstate']
>> if hgsub and hgsub[0] == 'r':
>> @@ -671,10 +670,30 @@ def applyupdates(repo, actions, wctx, mc
>> if hgsub and hgsub[0] == 'g':
>> subrepo.submerge(repo, wctx, mctx, wctx, overwrite)
>>
>> - for i, a in enumerate(actions):
>> - f, m, args, msg = a
>> - progress(_updating, z + i + 1, item=f, total=numupdates,
>> unit=_files)
>> - if m == "m": # merge
>> + for f, m, args, msg in actions:
>> +
>> + # forget (manifest only, just log it) (must come first)
>> + if m == "f":
>> + repo.ui.debug(" %s: %s -> f\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> +
>> + # re-add (manifest only, just log it)
>> + elif m == "a":
>> + repo.ui.debug(" %s: %s -> a\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> +
>> + # keep (noop, just log it)
>> + elif m == "k":
>> + repo.ui.debug(" %s: %s -> k\n" % (f, msg))
>> + # no progress
>> +
>> + # merge
>> + elif m == "m":
>> + repo.ui.debug(" %s: %s -> m\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>
> Those almost identical and repeated call to ui.debug(…) are bugging
> me. Can't we factor them?
Look at how it is after the 8th patch where we gain code readability and
efficiency. It is possible that we could introduce a "process this
action list and do some logging and progress reporting and apply this
function" later.
>
>> f1, f2, fa, move, anc = args
>> if f == '.hgsubstate': # subrepo states need updating
>> subrepo.submerge(repo, wctx, mctx,
>> wctx.ancestor(mctx),
>> @@ -689,35 +708,61 @@ def applyupdates(repo, actions, wctx, mc
>> updated += 1
>> else:
>> merged += 1
>> - elif m == "dm": # directory rename, move local
>> +
>> + # directory rename, move local
>> + elif m == "dm":
>> + repo.ui.debug(" %s: %s -> dm\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> f0, flags = args
>> repo.ui.note(_("moving %s to %s\n") % (f0, f))
>> audit(f)
>> repo.wwrite(f, wctx.filectx(f0).data(), flags)
>> util.unlinkpath(repo.wjoin(f0))
>> updated += 1
>> - elif m == "dg": # local directory rename, get
>> +
>> + # local directory rename, get
>> + elif m == "dg":
>> + repo.ui.debug(" %s: %s -> dg\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> f0, flags = args
>> repo.ui.note(_("getting %s to %s\n") % (f0, f))
>> repo.wwrite(f, mctx.filectx(f0).data(), flags)
>> updated += 1
>> - elif m == "dr": # divergent renames
>> +
>> + # divergent renames
>> + elif m == "dr":
>> + repo.ui.debug(" %s: %s -> dr\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> fl, = args
>> repo.ui.warn(_("note: possible conflict - %s was renamed "
>> "multiple times to:\n") % f)
>> for nf in fl:
>> repo.ui.warn(" %s\n" % nf)
>> - elif m == "rd": # rename and delete
>> +
>> + # rename and delete
>> + elif m == "rd":
>> + repo.ui.debug(" %s: %s -> rd\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> fl, = args
>> repo.ui.warn(_("note: possible conflict - %s was deleted "
>> "and renamed to:\n") % f)
>> for nf in fl:
>> repo.ui.warn(" %s\n" % nf)
>> - elif m == "e": # exec
>> +
>> + # exec
>> + elif m == "e":
>> + repo.ui.debug(" %s: %s -> e\n" % (f, msg))
>> + z += 1
>> + progress(_updating, z, item=f, total=numupdates,
>> unit=_files)
>> flags, = args
>> audit(f)
>> util.setflags(repo.wjoin(f), 'l' in flags, 'x' in flags)
>> updated += 1
>> +
>> ms.commit()
>> progress(_updating, None, total=numupdates, unit=_files)
>>
>> @@ -744,7 +789,7 @@ def calculateupdates(repo, wctx, mctx, a
>> actions = manifestmerge(repo, wctx, mctx, ancestor,
>> branchmerge, force,
>> partial, acceptremote,
>> followcopies)
>> - for a in sorted(actions):
>> + for a in sorted(actions, key=lambda a: (a[1], a)):
>
> New obscure key added to the sorted call?
It is only temporary so the convoluted code gives the same output as the
simplified code will do in step 8.
/Mads
More information about the Mercurial-devel
mailing list