| Submitter | Mads Kiilerich |
|---|---|
| Date | April 12, 2016, 10:18 p.m. |
| Message ID | <f7c245a1e550915fca41.1460499487@madski> |
| Download | mbox | patch |
| Permalink | /patch/14568/ |
| State | Accepted |
| Delegated to: | Yuya Nishihara |
| Headers | show |
Comments
Mads Kiilerich <mads@kiilerich.com> writes: > # HG changeset patch > # User Mads Kiilerich <madski@unity3d.com> > # Date 1460499381 -7200 > # Wed Apr 13 00:16:21 2016 +0200 > # Node ID f7c245a1e550915fca416e76abff626af85e4291 > # Parent 637869e31c974eecea10853fadac216107c434e1 > convert: keep converted hg parents that are outside convert.hg.revs (BC) > > Before, when converting revisions without also including their already > converted parents in convert.hg.revs, the parents would no longer be parents. > > That seems unfortunate and we dare to assume that nobody ever wants that. > > Instead, preserve parents that are outside the current convert range but > already have been converted. > > The parents returned in getcommit() are unconditionally converted, so we > introduce a separate optparents with optional parents. Reading the code, this seems like a pretty straight-forward fix to me. Perhaps a little extra overhead but that seems negligible here.
On Wed, 13 Apr 2016 00:18:07 +0200, Mads Kiilerich wrote: > # HG changeset patch > # User Mads Kiilerich <madski@unity3d.com> > # Date 1460499381 -7200 > # Wed Apr 13 00:16:21 2016 +0200 > # Node ID f7c245a1e550915fca416e76abff626af85e4291 > # Parent 637869e31c974eecea10853fadac216107c434e1 > convert: keep converted hg parents that are outside convert.hg.revs (BC) Yeah, this looks much better than V1, thanks. > - parents = [p.hex() for p in self._parents(ctx)] > + _parents = self._parents(ctx) > + parents = [p.hex() for p in _parents] > + optparents = [p.hex() for p in ctx.parents() if p not in _parents] One nit. ctx.parents() may contain null revision if p1 is null. Perhaps we'd better filter out it like self._parents(). If you agree, I'll change s/if p not in _parents/if p and p not in _parents/ in flight.
On 04/13/2016 05:49 PM, Yuya Nishihara wrote: >> - parents = [p.hex() for p in self._parents(ctx)] >> + _parents = self._parents(ctx) >> + parents = [p.hex() for p in _parents] >> + optparents = [p.hex() for p in ctx.parents() if p not in _parents] > One nit. ctx.parents() may contain null revision if p1 is null. Perhaps we'd > better filter out it like self._parents(). > > If you agree, I'll change s/if p not in _parents/if p and p not in _parents/ > in flight. Yeah, seems reasonable. Thanks! (It would be weird if the null revision has been converted, so I doubt it makes any actual difference ...) /Mads
On Thu, 14 Apr 2016 01:07:31 +0200, Mads Kiilerich wrote: > On 04/13/2016 05:49 PM, Yuya Nishihara wrote: > >> - parents = [p.hex() for p in self._parents(ctx)] > >> + _parents = self._parents(ctx) > >> + parents = [p.hex() for p in _parents] > >> + optparents = [p.hex() for p in ctx.parents() if p not in _parents] > > One nit. ctx.parents() may contain null revision if p1 is null. Perhaps we'd > > better filter out it like self._parents(). > > > > If you agree, I'll change s/if p not in _parents/if p and p not in _parents/ > > in flight. > > Yeah, seems reasonable. Thanks! > > (It would be weird if the null revision has been converted, so I doubt > it makes any actual difference ...) There's no practical difference as optparets is filtered by converter.map. Pushed the modified version, thanks.
Patch
diff --git a/hgext/convert/common.py b/hgext/convert/common.py --- a/hgext/convert/common.py +++ b/hgext/convert/common.py @@ -55,11 +55,13 @@ SKIPREV = 'SKIP' class commit(object): def __init__(self, author, date, desc, parents, branch=None, rev=None, - extra={}, sortkey=None, saverev=True, phase=phases.draft): + extra={}, sortkey=None, saverev=True, phase=phases.draft, + optparents=None): self.author = author or 'unknown' self.date = date or '0 0' self.desc = desc - self.parents = parents + self.parents = parents # will be converted and used as parents + self.optparents = optparents or [] # will be used if already converted self.branch = branch self.rev = rev self.extra = extra diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py --- a/hgext/convert/convcmd.py +++ b/hgext/convert/convcmd.py @@ -472,6 +472,9 @@ class converter(object): parents = [self.map.get(p, p) for p in parents] except KeyError: parents = [b[0] for b in pbranches] + parents.extend(self.map[x] + for x in commit.optparents + if x in self.map) if len(pbranches) != 2: cleanp2 = set() if len(parents) < 3: diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py --- a/hgext/convert/hg.py +++ b/hgext/convert/hg.py @@ -582,7 +582,9 @@ class mercurial_source(common.converter_ def getcommit(self, rev): ctx = self._changectx(rev) - parents = [p.hex() for p in self._parents(ctx)] + _parents = self._parents(ctx) + parents = [p.hex() for p in _parents] + optparents = [p.hex() for p in ctx.parents() if p not in _parents] crev = rev return common.commit(author=ctx.user(), @@ -591,6 +593,7 @@ class mercurial_source(common.converter_ desc=ctx.description(), rev=crev, parents=parents, + optparents=optparents, branch=ctx.branch(), extra=ctx.extra(), sortkey=ctx.rev(), diff --git a/tests/test-convert-hg-startrev.t b/tests/test-convert-hg-startrev.t --- a/tests/test-convert-hg-startrev.t +++ b/tests/test-convert-hg-startrev.t @@ -222,7 +222,7 @@ Convert from specified revs o 0 "0: add a b f" files: a b f Convert in multiple steps that doesn't overlap - the link to the parent is -currently missing +preserved anyway $ hg convert --config convert.hg.revs=::1 source multistep initializing destination multistep repository @@ -237,8 +237,8 @@ currently missing converting... 0 2: copy e from a, change b $ glog multistep - o 2 "2: copy e from a, change b" files: a b c d e - + o 2 "2: copy e from a, change b" files: b e + | o 1 "1: add c, move f to d" files: c d f | o 0 "0: add a b f" files: a b f