| Submitter | Martin von Zweigbergk |
|---|---|
| Date | Dec. 3, 2014, 5:17 p.m. |
| Message ID | <d7c59ced01f5bc3ddc34.1417627033@martinvonz.mtv.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/6979/ |
| State | Superseded |
| Headers | show |
Comments
Sorry, let me think a bit more about this before you review it. It might make it somewhat simpler, but my end goal was to simplify largefiles' overridecalculateupdates() and maybe bidmerge. I still think the first two patches are good, so please review those. On Wed Dec 03 2014 at 9:17:18 AM Martin von Zweigbergk < martinvonz@google.com> wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1416786380 28800 > # Sun Nov 23 15:46:20 2014 -0800 > # Node ID d7c59ced01f5bc3ddc34c3b597b8915740ed79c0 > # Parent 17f0fda2398ba2463589a3a59efe78b095b5d593 > merge: split move-merge into two actions > > The merge action for merges ('m') carries a 'move' argument, which > tells whether to also remove the copy source. The code can be > simplified by removing that action argument and producing a second > action for the removal instead. Since the removal doesn't behave > exactly like an 'r' action (specifically, it doesn't count as > 'removed' file in the stats), we need to create new type of action. > > diff --git a/mercurial/merge.py b/mercurial/merge.py > --- a/mercurial/merge.py > +++ b/mercurial/merge.py > @@ -343,8 +343,9 @@ > for m in 'a', 'f', 'g', 'cd', 'dc': > for f, args, msg in actions[m]: > pmmf.add(f) > - for f, args, msg in actions['r']: > - pmmf.discard(f) > + for m in 'r', 'rm': > + for f, args, msg in actions[m]: > + pmmf.discard(f) > for f, args, msg in actions['dm']: > f2, flags = args > pmmf.discard(f2) > @@ -352,9 +353,6 @@ > for f, args, msg in actions['dg']: > pmmf.add(f) > for f, args, msg in actions['m']: > - f1, f2, fa, move, anc = args > - if move: > - pmmf.discard(f1) > pmmf.add(f) > > # check case-folding collision in provisional merged manifest > @@ -376,7 +374,8 @@ > acceptremote = accept the incoming changes without prompting > """ > > - actions = dict((m, []) for m in 'a f g cd dc r dm dg m dr e rd > k'.split()) > + actions = dict((m, []) for m in > + 'a f g cd dc r rm dm dg m dr e rd k'.split()) > copy, movewithdir = {}, {} > > # manifests fetched in order are going to be faster, so prime the > caches > @@ -418,10 +417,10 @@ > if f not in ma: > fa = copy.get(f, None) > if fa is not None: > - actions['m'].append((f, (f, f, fa, False, pa.node()), > + actions['m'].append((f, (f, f, fa, pa.node()), > "both renamed from " + fa)) > else: > - actions['m'].append((f, (f, f, None, False, > pa.node()), > + actions['m'].append((f, (f, f, None, pa.node()), > "both created")) > else: > a = ma[f] > @@ -439,7 +438,7 @@ > elif nol and n1 == a: # local only changed 'x' > actions['g'].append((f, (fl1,), "remote is newer")) > else: # both changed something > - actions['m'].append((f, (f, f, f, False, pa.node()), > + actions['m'].append((f, (f, f, f, pa.node()), > "versions differ")) > elif n1: # file exists only on local side > if f in copied: > @@ -450,7 +449,7 @@ > "remote directory rename - move from " + > f)) > elif f in copy: > f2 = copy[f] > - actions['m'].append((f, (f, f2, f2, False, pa.node()), > + actions['m'].append((f, (f, f2, f2, pa.node()), > "local copied/moved from " + f2)) > elif f in ma: # clean, a different, no remote > if n1 != ma[f]: > @@ -476,10 +475,12 @@ > elif f in copy: > f2 = copy[f] > if f2 in m2: > - actions['m'].append((f, (f2, f, f2, False, pa.node()), > + actions['m'].append((f, (f2, f, f2, pa.node()), > "remote copied from " + f2)) > else: > - actions['m'].append((f, (f2, f, f2, True, pa.node()), > + actions['rm'].append((f2, None, > + "remote moved to " + f)) > + actions['m'].append((f, (f2, f, f2, pa.node()), > "remote moved from " + f2)) > elif f not in ma: > # local unknown, remote created: the logic is described > by the > @@ -499,7 +500,7 @@ > else: > different = _checkunknownfile(repo, wctx, p2, f) > if force and branchmerge and different: > - actions['m'].append((f, (f, f, None, False, > pa.node()), > + actions['m'].append((f, (f, f, None, pa.node()), > "remote differs from untracked > local")) > elif not force and different: > aborts.append((f, 'ud')) > @@ -702,13 +703,12 @@ > updated, merged, removed, unresolved = 0, 0, 0, 0 > ms = mergestate(repo) > ms.reset(wctx.p1().node(), mctx.node()) > - moves = [] > for m, l in actions.items(): > l.sort() > > # prescan for merges > for f, args, msg in actions['m']: > - f1, f2, fa, move, anc = args > + f1, f2, fa, anc = args > if f == '.hgsubstate': # merged internally > continue > repo.ui.debug(" preserving %s for resolve of %s\n" % (f1, f)) > @@ -720,8 +720,6 @@ > else: > fca = repo.filectx(f1, fileid=nullrev) > ms.add(fcl, fco, fca, f) > - if f1 != f and move: > - moves.append(f1) > > audit = repo.wopener.audit > _updating = _('updating') > @@ -729,13 +727,13 @@ > progress = repo.ui.progress > > # remove renamed files after safely stored > - for f in moves: > + for f, args, msg in actions['rm']: > if os.path.lexists(repo.wjoin(f)): > repo.ui.debug("removing %s\n" % f) > audit(f) > util.unlinkpath(repo.wjoin(f)) > > - numupdates = sum(len(l) for m, l in actions.items() if m != 'k') > + numupdates = sum(len(l) for m, l in actions.items() if m not in ('k', > 'rm')) > > if [a for a in actions['r'] if a[0] == '.hgsubstate']: > subrepo.submerge(repo, wctx, mctx, wctx, overwrite) > @@ -888,14 +886,12 @@ > > # merge > for f, args, msg in actions['m']: > - f1, f2, fa, move, anc = args > + f1, f2, fa, anc = args > if branchmerge: > # We've done a branch merge, mark this file as merged > # so that we properly record the merger later > repo.dirstate.merge(f) > if f1 != f2: # copy/rename > - if move: > - repo.dirstate.remove(f1) > if f1 != f: > repo.dirstate.copy(f1, f) > else: > @@ -908,8 +904,13 @@ > # modification. > if f2 == f: # file not locally copied/moved > repo.dirstate.normallookup(f) > - if move: > - repo.dirstate.drop(f1) > + > + # remove file that moved away > + for f, args, msg in actions['rm']: > + if branchmerge: > + repo.dirstate.remove(f) > + else: > + repo.dirstate.drop(f) > > # directory rename, move local > for f, args, msg in actions['dm']: >
On 12/03/2014 06:17 PM, Martin von Zweigbergk wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1416786380 28800 > # Sun Nov 23 15:46:20 2014 -0800 > # Node ID d7c59ced01f5bc3ddc34c3b597b8915740ed79c0 > # Parent 17f0fda2398ba2463589a3a59efe78b095b5d593 > merge: split move-merge into two actions > > The merge action for merges ('m') carries a 'move' argument, which > tells whether to also remove the copy source. The code can be > simplified by removing that action argument and producing a second > action for the removal instead. Since the removal doesn't behave > exactly like an 'r' action (specifically, it doesn't count as > 'removed' file in the stats), we need to create new type of action. > > diff --git a/mercurial/merge.py b/mercurial/merge.py > --- a/mercurial/merge.py > +++ b/mercurial/merge.py > @@ -343,8 +343,9 @@ > for m in 'a', 'f', 'g', 'cd', 'dc': > for f, args, msg in actions[m]: > pmmf.add(f) > - for f, args, msg in actions['r']: > - pmmf.discard(f) > + for m in 'r', 'rm': I would expect () around the tuple here. But well ... it seems to work without it. But still ... I would perhaps prefer actions['r']+actions['m'] in the next line ... (and same pattern in later patches) but perhaps not ... > + for f, args, msg in actions[m]: > + pmmf.discard(f) > for f, args, msg in actions['dm']: > f2, flags = args > pmmf.discard(f2) > @@ -352,9 +353,6 @@ > for f, args, msg in actions['dg']: > pmmf.add(f) > for f, args, msg in actions['m']: > - f1, f2, fa, move, anc = args > - if move: > - pmmf.discard(f1) > pmmf.add(f) > > # check case-folding collision in provisional merged manifest > @@ -376,7 +374,8 @@ > acceptremote = accept the incoming changes without prompting > """ > > - actions = dict((m, []) for m in 'a f g cd dc r dm dg m dr e rd k'.split()) > + actions = dict((m, []) for m in > + 'a f g cd dc r rm dm dg m dr e rd k'.split()) I wonder if this should be turned into a named dict too ;-) /Mads
Patch
diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -343,8 +343,9 @@ for m in 'a', 'f', 'g', 'cd', 'dc': for f, args, msg in actions[m]: pmmf.add(f) - for f, args, msg in actions['r']: - pmmf.discard(f) + for m in 'r', 'rm': + for f, args, msg in actions[m]: + pmmf.discard(f) for f, args, msg in actions['dm']: f2, flags = args pmmf.discard(f2) @@ -352,9 +353,6 @@ for f, args, msg in actions['dg']: pmmf.add(f) for f, args, msg in actions['m']: - f1, f2, fa, move, anc = args - if move: - pmmf.discard(f1) pmmf.add(f) # check case-folding collision in provisional merged manifest @@ -376,7 +374,8 @@ acceptremote = accept the incoming changes without prompting """ - actions = dict((m, []) for m in 'a f g cd dc r dm dg m dr e rd k'.split()) + actions = dict((m, []) for m in + 'a f g cd dc r rm dm dg m dr e rd k'.split()) copy, movewithdir = {}, {} # manifests fetched in order are going to be faster, so prime the caches @@ -418,10 +417,10 @@ if f not in ma: fa = copy.get(f, None) if fa is not None: - actions['m'].append((f, (f, f, fa, False, pa.node()), + actions['m'].append((f, (f, f, fa, pa.node()), "both renamed from " + fa)) else: - actions['m'].append((f, (f, f, None, False, pa.node()), + actions['m'].append((f, (f, f, None, pa.node()), "both created")) else: a = ma[f] @@ -439,7 +438,7 @@ elif nol and n1 == a: # local only changed 'x' actions['g'].append((f, (fl1,), "remote is newer")) else: # both changed something - actions['m'].append((f, (f, f, f, False, pa.node()), + actions['m'].append((f, (f, f, f, pa.node()), "versions differ")) elif n1: # file exists only on local side if f in copied: @@ -450,7 +449,7 @@ "remote directory rename - move from " + f)) elif f in copy: f2 = copy[f] - actions['m'].append((f, (f, f2, f2, False, pa.node()), + actions['m'].append((f, (f, f2, f2, pa.node()), "local copied/moved from " + f2)) elif f in ma: # clean, a different, no remote if n1 != ma[f]: @@ -476,10 +475,12 @@ elif f in copy: f2 = copy[f] if f2 in m2: - actions['m'].append((f, (f2, f, f2, False, pa.node()), + actions['m'].append((f, (f2, f, f2, pa.node()), "remote copied from " + f2)) else: - actions['m'].append((f, (f2, f, f2, True, pa.node()), + actions['rm'].append((f2, None, + "remote moved to " + f)) + actions['m'].append((f, (f2, f, f2, pa.node()), "remote moved from " + f2)) elif f not in ma: # local unknown, remote created: the logic is described by the @@ -499,7 +500,7 @@ else: different = _checkunknownfile(repo, wctx, p2, f) if force and branchmerge and different: - actions['m'].append((f, (f, f, None, False, pa.node()), + actions['m'].append((f, (f, f, None, pa.node()), "remote differs from untracked local")) elif not force and different: aborts.append((f, 'ud')) @@ -702,13 +703,12 @@ updated, merged, removed, unresolved = 0, 0, 0, 0 ms = mergestate(repo) ms.reset(wctx.p1().node(), mctx.node()) - moves = [] for m, l in actions.items(): l.sort() # prescan for merges for f, args, msg in actions['m']: - f1, f2, fa, move, anc = args + f1, f2, fa, anc = args if f == '.hgsubstate': # merged internally continue repo.ui.debug(" preserving %s for resolve of %s\n" % (f1, f)) @@ -720,8 +720,6 @@ else: fca = repo.filectx(f1, fileid=nullrev) ms.add(fcl, fco, fca, f) - if f1 != f and move: - moves.append(f1) audit = repo.wopener.audit _updating = _('updating') @@ -729,13 +727,13 @@ progress = repo.ui.progress # remove renamed files after safely stored - for f in moves: + for f, args, msg in actions['rm']: if os.path.lexists(repo.wjoin(f)): repo.ui.debug("removing %s\n" % f) audit(f) util.unlinkpath(repo.wjoin(f)) - numupdates = sum(len(l) for m, l in actions.items() if m != 'k') + numupdates = sum(len(l) for m, l in actions.items() if m not in ('k', 'rm')) if [a for a in actions['r'] if a[0] == '.hgsubstate']: subrepo.submerge(repo, wctx, mctx, wctx, overwrite) @@ -888,14 +886,12 @@ # merge for f, args, msg in actions['m']: - f1, f2, fa, move, anc = args + f1, f2, fa, anc = args if branchmerge: # We've done a branch merge, mark this file as merged # so that we properly record the merger later repo.dirstate.merge(f) if f1 != f2: # copy/rename - if move: - repo.dirstate.remove(f1) if f1 != f: repo.dirstate.copy(f1, f) else: @@ -908,8 +904,13 @@ # modification. if f2 == f: # file not locally copied/moved repo.dirstate.normallookup(f) - if move: - repo.dirstate.drop(f1) + + # remove file that moved away + for f, args, msg in actions['rm']: + if branchmerge: + repo.dirstate.remove(f) + else: + repo.dirstate.drop(f) # directory rename, move local for f, args, msg in actions['dm']:
# HG changeset patch # User Martin von Zweigbergk <martinvonz@google.com> # Date 1416786380 28800 # Sun Nov 23 15:46:20 2014 -0800 # Node ID d7c59ced01f5bc3ddc34c3b597b8915740ed79c0 # Parent 17f0fda2398ba2463589a3a59efe78b095b5d593 merge: split move-merge into two actions The merge action for merges ('m') carries a 'move' argument, which tells whether to also remove the copy source. The code can be simplified by removing that action argument and producing a second action for the removal instead. Since the removal doesn't behave exactly like an 'r' action (specifically, it doesn't count as 'removed' file in the stats), we need to create new type of action.