| Submitter | Mads Kiilerich |
|---|---|
| Date | March 25, 2016, 8:14 a.m. |
| Message ID | <ede9dd4902ae4a5cd464.1458893643@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/14065/ |
| State | Changes Requested |
| Delegated to: | Yuya Nishihara |
| Headers | show |
Comments
On Fri, Mar 25, 2016 at 4:14 AM, Mads Kiilerich <mads@kiilerich.com> wrote: > Instead, preserve parents that are outside the current convert range but > already has been converted. > it doesn't work to return all parents there. Instead, pass the converted > revisions to getcommit() so it freely can return parents that already has been already has => already have (x2)
On Fri, 25 Mar 2016 01:14:03 -0700, Mads Kiilerich wrote: > # HG changeset patch > # User Mads Kiilerich <madski@unity3d.com> > # Date 1458860864 25200 > # Thu Mar 24 16:07:44 2016 -0700 > # Node ID ede9dd4902ae4a5cd464811532acdcd2fd02728f > # Parent 0dd592daa18caead5b26cfc1f438b5d885d8b8ce > convert: keep converted parents that are outside convert.hg.revs (BC) > > Before, when converting revisions without also converting their parents, > parents that already had been converted 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 has been converted. I think the direction is good. > --- a/hgext/convert/filemap.py > +++ b/hgext/convert/filemap.py > @@ -259,7 +259,7 @@ class filemap_source(common.converter_so > def getheads(self): > return self.base.getheads() > > - def getcommit(self, rev): > + def getcommit(self, rev, revmap=None): > # We want to save a reference to the commit objects to be able > # to rewrite their parents later on. > c = self.commits[rev] = self.base.getcommit(rev) Perhaps filemap_source should forward revmap to base source, but I got IndexError if I changed it in that way. Can we pass a copy of the known revmap by constructor? I think it will be simpler because revmap isn't necessary to be updated dynamically.
On 03/27/2016 09:07 AM, Yuya Nishihara wrote: > On Fri, 25 Mar 2016 01:14:03 -0700, Mads Kiilerich wrote: >> # HG changeset patch >> # User Mads Kiilerich <madski@unity3d.com> >> # Date 1458860864 25200 >> # Thu Mar 24 16:07:44 2016 -0700 >> # Node ID ede9dd4902ae4a5cd464811532acdcd2fd02728f >> # Parent 0dd592daa18caead5b26cfc1f438b5d885d8b8ce >> convert: keep converted parents that are outside convert.hg.revs (BC) >> >> Before, when converting revisions without also converting their parents, >> parents that already had been converted 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 has been converted. > I think the direction is good. > >> --- a/hgext/convert/filemap.py >> +++ b/hgext/convert/filemap.py >> @@ -259,7 +259,7 @@ class filemap_source(common.converter_so >> def getheads(self): >> return self.base.getheads() >> >> - def getcommit(self, rev): >> + def getcommit(self, rev, revmap=None): >> # We want to save a reference to the commit objects to be able >> # to rewrite their parents later on. >> c = self.commits[rev] = self.base.getcommit(rev) > Perhaps filemap_source should forward revmap to base source, but I got > IndexError if I changed it in that way. > > Can we pass a copy of the known revmap by constructor? I think it will be > simpler because revmap isn't necessary to be updated dynamically. I don't see a good way to do that with the current convert architecture. The source and destination are created independently before both are passed to convert(). The same result could also be achieved with other more or less intrusive changes to how parents generally are handled or to the conversion framework in general. I am not sure which one would be better. /Mads
On Sun, 27 Mar 2016 13:23:46 -0700 Mads Kiilerich <mads@kiilerich.com> wrote: > On 03/27/2016 09:07 AM, Yuya Nishihara wrote: > > On Fri, 25 Mar 2016 01:14:03 -0700, Mads Kiilerich wrote: > >> # HG changeset patch > >> # User Mads Kiilerich <madski@unity3d.com> > >> # Date 1458860864 25200 > >> # Thu Mar 24 16:07:44 2016 -0700 > >> # Node ID ede9dd4902ae4a5cd464811532acdcd2fd02728f > >> # Parent 0dd592daa18caead5b26cfc1f438b5d885d8b8ce > >> convert: keep converted parents that are outside convert.hg.revs (BC) > >> > >> Before, when converting revisions without also converting their parents, > >> parents that already had been converted 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 has been converted. > > I think the direction is good. > > > >> --- a/hgext/convert/filemap.py > >> +++ b/hgext/convert/filemap.py > >> @@ -259,7 +259,7 @@ class filemap_source(common.converter_so > >> def getheads(self): > >> return self.base.getheads() > >> > >> - def getcommit(self, rev): > >> + def getcommit(self, rev, revmap=None): > >> # We want to save a reference to the commit objects to be able > >> # to rewrite their parents later on. > >> c = self.commits[rev] = self.base.getcommit(rev) > > Perhaps filemap_source should forward revmap to base source, but I got > > IndexError if I changed it in that way. > > > > Can we pass a copy of the known revmap by constructor? I think it will be > > simpler because revmap isn't necessary to be updated dynamically. > > I don't see a good way to do that with the current convert architecture. > The source and destination are created independently before both are > passed to convert(). I don't have a good idea. It should be doable, but it doesn't look nice. destc = convertsink(...) revmap = mapfile(ui, destc.revmapfile()) srcc = convertsource(..., dict(revmap)) converter(ui, srcc, destc, revmap, ...) # transfer ownership of revmap file > The same result could also be achieved with other more or less intrusive > changes to how parents generally are handled or to the conversion > framework in general. I am not sure which one would be better. That sounds better as the convertsource won't need a revmap trick. But I don't know how much hard work would be necessary to achieve that.
Patch
diff --git a/hgext/convert/bzr.py b/hgext/convert/bzr.py --- a/hgext/convert/bzr.py +++ b/hgext/convert/bzr.py @@ -155,7 +155,7 @@ class bzr_source(common.converter_source files, changes = self._gettreechanges(self._revtree, prevtree) return files, changes, set() - def getcommit(self, version): + def getcommit(self, version, revmap=None): rev = self.sourcerepo.get_revision(version) # populate parent id cache if not rev.parent_ids: diff --git a/hgext/convert/common.py b/hgext/convert/common.py --- a/hgext/convert/common.py +++ b/hgext/convert/common.py @@ -132,7 +132,7 @@ class converter_source(object): """ raise NotImplementedError - def getcommit(self, version): + def getcommit(self, version, revmap=None): """Return the commit object for version""" raise NotImplementedError diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py --- a/hgext/convert/convcmd.py +++ b/hgext/convert/convcmd.py @@ -439,7 +439,7 @@ class converter(object): afile.close() def cachecommit(self, rev): - commit = self.source.getcommit(rev) + commit = self.source.getcommit(rev, revmap=self.map) commit.author = self.authors.get(commit.author, commit.author) commit.branch = mapbranch(commit.branch, self.branchmap) self.commitcache[rev] = commit diff --git a/hgext/convert/cvs.py b/hgext/convert/cvs.py --- a/hgext/convert/cvs.py +++ b/hgext/convert/cvs.py @@ -284,7 +284,7 @@ class convert_cvs(converter_source): self._parse() return sorted(self.files[rev].iteritems()), {}, set() - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): self._parse() return self.changeset[rev] diff --git a/hgext/convert/darcs.py b/hgext/convert/darcs.py --- a/hgext/convert/darcs.py +++ b/hgext/convert/darcs.py @@ -146,7 +146,7 @@ class darcs_source(common.converter_sour def getheads(self): return self.parents[None] - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): elt = self.changes[rev] date = util.strdate(elt.get('local_date'), '%a %b %d %H:%M:%S %Z %Y') desc = elt.findtext('name') + '\n' + elt.findtext('comment', '') diff --git a/hgext/convert/filemap.py b/hgext/convert/filemap.py --- a/hgext/convert/filemap.py +++ b/hgext/convert/filemap.py @@ -259,7 +259,7 @@ class filemap_source(common.converter_so def getheads(self): return self.base.getheads() - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): # We want to save a reference to the commit objects to be able # to rewrite their parents later on. c = self.commits[rev] = self.base.getcommit(rev) diff --git a/hgext/convert/git.py b/hgext/convert/git.py --- a/hgext/convert/git.py +++ b/hgext/convert/git.py @@ -301,7 +301,7 @@ class convert_git(common.converter_sourc changes.append(('.hgsubstate', '')) return (changes, copies, set()) - def getcommit(self, version): + def getcommit(self, version, revmap=None): c = self.catfile(version, "commit") # read the commit hash end = c.find("\n\n") message = c[end + 2:] diff --git a/hgext/convert/gnuarch.py b/hgext/convert/gnuarch.py --- a/hgext/convert/gnuarch.py +++ b/hgext/convert/gnuarch.py @@ -181,7 +181,7 @@ class gnuarch_source(common.converter_so self.lastrev = rev return sorted(set(changes)), copies, set() - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): changes = self.changes[rev] return common.commit(author=changes.author, date=changes.date, desc=changes.summary, parents=self.parents[rev], diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py --- a/hgext/convert/hg.py +++ b/hgext/convert/hg.py @@ -501,8 +501,10 @@ class mercurial_source(common.converter_ self.lastrev = rev return self.lastctx - def _parents(self, ctx): - return [p for p in ctx.parents() if p and self.keep(p.node())] + def _parents(self, ctx, revmap=None): + return [p for p in ctx.parents() + if p and (self.keep(p.node()) or + revmap is not None and nodemod.hex(p.node()) in revmap)] def getheads(self): return [nodemod.hex(h) for h in self._heads if self.keep(h)] @@ -579,9 +581,9 @@ class mercurial_source(common.converter_ self.ui.warn(_('ignoring: %s\n') % e) return copies - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): ctx = self._changectx(rev) - parents = [p.hex() for p in self._parents(ctx)] + parents = [p.hex() for p in self._parents(ctx, revmap)] crev = rev return common.commit(author=ctx.user(), diff --git a/hgext/convert/monotone.py b/hgext/convert/monotone.py --- a/hgext/convert/monotone.py +++ b/hgext/convert/monotone.py @@ -303,7 +303,7 @@ class monotone_source(common.converter_s node, attr = self.files.get(name, (None, "")) return data, attr - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): extra = {} certs = self.mtngetcerts(rev) if certs.get('suspend') == certs["branch"]: diff --git a/hgext/convert/p4.py b/hgext/convert/p4.py --- a/hgext/convert/p4.py +++ b/hgext/convert/p4.py @@ -286,7 +286,7 @@ class p4_source(common.converter_source) raise error.Abort(_("convert from p4 does not support --full")) return self.files[rev], self.copies[rev], set() - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): return self.changeset[rev] def gettags(self): diff --git a/hgext/convert/subversion.py b/hgext/convert/subversion.py --- a/hgext/convert/subversion.py +++ b/hgext/convert/subversion.py @@ -507,7 +507,7 @@ class svn_source(converter_source): self._changescache = (rev, (files, copies)) return [f[0] for f in files] - def getcommit(self, rev): + def getcommit(self, rev, revmap=None): if rev not in self.commits: uuid, module, revnum = revsplit(rev) self.module = module 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