Submitter | Pierre-Yves David |
---|---|
Date | Aug. 5, 2019, 4:36 p.m. |
Message ID | <71edc0b44b7108cddee3.1565022992@nodosa.octopoid.net> |
Download | mbox | patch |
Permalink | /patch/41153/ |
State | Accepted |
Headers | show |
Comments
On Mon, Aug 5, 2019 at 9:55 AM Pierre-Yves David < pierre-yves.david@ens-lyon.org> wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@octobus.net> > # Date 1564509524 -7200 > # Tue Jul 30 19:58:44 2019 +0200 > # Node ID 71edc0b44b7108cddee33d99f0ec586a0b0405c9 > # Parent aa19f478cfdfe781acc15cd26339644757156355 > # EXP-Topic upgrade-select > # Available At https://bitbucket.org/octobus/mercurial-devel/ > # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r > 71edc0b44b71 > upgrade: introduce a _copyrevlog method > > This function copies a revlog from the old store to the new one, without > re-adding all deltas manually. This will eventually save a lot of time > when some > revlog does not needs any conversions. > > Code actually using this will be introduced in later changesets. > > diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py > --- a/mercurial/upgrade.py > +++ b/mercurial/upgrade.py > @@ -533,6 +533,35 @@ def _revlogfrompath(repo, path): > #reverse of "/".join(("data", path + ".i")) > return filelog.filelog(repo.svfs, path[5:-2]) > > +def _copyrevlog(tr, destrepo, oldrl, unencodedname): > + """copy all relevant files for `oldrl` into `destrepo` store > I'd like to point out that low-level code like this is a violation of storage abstractions. In this case, the upgrade code is making assumptions about which revlogs exist and what they are named. The proper way to implement this feature is to implement a `copystorage()` method or some such on the per-primitive storage interfaces (changelog, manifestlog, filelog, etc) and to have the upgrade code call it when copying between compatible repo types. That being said, the storage interfaces aren't fully flushed out yet and my recollection is the upgrade code is full of cases that assume revlogs. So it is probably acceptable to take this technical debt until we can devise a way to better handle repo upgrades in the storage interfaces. > + > + Files are copied "as is" without any transformation. The copy is > performed > + without extra checks. Callers are responsible for making sure the > copied > + content is compatible with format of the destination repository. > + """ > + oldrl = getattr(oldrl, '_revlog', oldrl) > + newrl = _revlogfrompath(destrepo, unencodedname) > + newrl = getattr(newrl, '_revlog', newrl) > + > + oldvfs = oldrl.opener > + newvfs = newrl.opener > + oldindex = oldvfs.join(oldrl.indexfile) > + newindex = newvfs.join(newrl.indexfile) > + olddata = oldvfs.join(oldrl.datafile) > + newdata = newvfs.join(newrl.datafile) > + > + newdir = newvfs.dirname(newrl.indexfile) > + newvfs.makedirs(newdir) > + > + util.copyfile(oldindex, newindex) > + if oldrl.opener.exists(olddata): > + util.copyfile(olddata, newdata) > + > + if not (unencodedname.endswith('00changelog.i') > + or unencodedname.endswith('00manifest.i')): > + destrepo.svfs.fncache.add(unencodedname) > + > def _clonerevlogs(ui, srcrepo, dstrepo, tr, deltareuse, > forcedeltabothparents): > """Copy revlogs between 2 repos.""" > revcount = 0 > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
On 8/6/19 4:33 AM, Gregory Szorc wrote: > On Mon, Aug 5, 2019 at 9:55 AM Pierre-Yves David > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> > wrote: > > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@octobus.net > <mailto:pierre-yves.david@octobus.net>> > # Date 1564509524 -7200 > # Tue Jul 30 19:58:44 2019 +0200 > # Node ID 71edc0b44b7108cddee33d99f0ec586a0b0405c9 > # Parent aa19f478cfdfe781acc15cd26339644757156355 > # EXP-Topic upgrade-select > # Available At https://bitbucket.org/octobus/mercurial-devel/ > # hg pull > https://bitbucket.org/octobus/mercurial-devel/ -r 71edc0b44b71 > upgrade: introduce a _copyrevlog method > > This function copies a revlog from the old store to the new one, without > re-adding all deltas manually. This will eventually save a lot of > time when some > revlog does not needs any conversions. > > Code actually using this will be introduced in later changesets. > > diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py > --- a/mercurial/upgrade.py > +++ b/mercurial/upgrade.py > @@ -533,6 +533,35 @@ def _revlogfrompath(repo, path): > #reverse of "/".join(("data", path + ".i")) > return filelog.filelog(repo.svfs, path[5:-2]) > > +def _copyrevlog(tr, destrepo, oldrl, unencodedname): > + """copy all relevant files for `oldrl` into `destrepo` store > > > I'd like to point out that low-level code like this is a violation of > storage abstractions. In this case, the upgrade code is making > assumptions about which revlogs exist and what they are named. The > proper way to implement this feature is to implement a `copystorage()` > method or some such on the per-primitive storage interfaces (changelog, > manifestlog, filelog, etc) and to have the upgrade code call it when > copying between compatible repo types. I tried that route first, but I lacked access to a couple of important bit at the storage layer. So I took a simpler approach. (Not saying it is impossible, but that was not trivial when I tried) > That being said, the storage interfaces aren't fully flushed out yet and > my recollection is the upgrade code is full of cases that assume > revlogs. So it is probably acceptable to take this technical debt until > we can devise a way to better handle repo upgrades in the storage > interfaces. Yeah, the upgrade code is already assuming revlog all over the place, so it seems fine to add another one. For example `_revlogfrompath` has a similar logic than the matchrevlog function.
Patch
diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py --- a/mercurial/upgrade.py +++ b/mercurial/upgrade.py @@ -533,6 +533,35 @@ def _revlogfrompath(repo, path): #reverse of "/".join(("data", path + ".i")) return filelog.filelog(repo.svfs, path[5:-2]) +def _copyrevlog(tr, destrepo, oldrl, unencodedname): + """copy all relevant files for `oldrl` into `destrepo` store + + Files are copied "as is" without any transformation. The copy is performed + without extra checks. Callers are responsible for making sure the copied + content is compatible with format of the destination repository. + """ + oldrl = getattr(oldrl, '_revlog', oldrl) + newrl = _revlogfrompath(destrepo, unencodedname) + newrl = getattr(newrl, '_revlog', newrl) + + oldvfs = oldrl.opener + newvfs = newrl.opener + oldindex = oldvfs.join(oldrl.indexfile) + newindex = newvfs.join(newrl.indexfile) + olddata = oldvfs.join(oldrl.datafile) + newdata = newvfs.join(newrl.datafile) + + newdir = newvfs.dirname(newrl.indexfile) + newvfs.makedirs(newdir) + + util.copyfile(oldindex, newindex) + if oldrl.opener.exists(olddata): + util.copyfile(olddata, newdata) + + if not (unencodedname.endswith('00changelog.i') + or unencodedname.endswith('00manifest.i')): + destrepo.svfs.fncache.add(unencodedname) + def _clonerevlogs(ui, srcrepo, dstrepo, tr, deltareuse, forcedeltabothparents): """Copy revlogs between 2 repos.""" revcount = 0