This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6643

convert: add a config option to help doing identity hg->hg conversion
ClosedPublic

Authored by valentin.gatienbaron on Jul 14 2019, 3:15 PM.

Details

Reviewers
mharbison72
durin42
Group Reviewers
hg-reviewers
Commits
rHGd98ec36be808: convert: add a config option to help doing identity hg->hg conversion
rHG7a44de54490a: convert: add a config option to help doing identity hg->hg conversion
Summary

I want to change the computation of the list of files modified by a
commit. In principle, this would simply change a cache. But since this
information is stored in commits rather than a cache, changing it
means changing commit hashes (going forward).

Some users rely on the convert extension from hg to hg not changing
hashes when nothing changes (usually). Allow these users to preserve
hashes despite changes to the changelog files computation by reusing
these files lists when the manifest is unchanged (since these files
list are derived from the manifest).

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mharbison72 requested changes to this revision.Jul 15 2019, 12:20 PM
mharbison72 added a subscriber: mharbison72.

Don't take this as a complete list, but it looks like repository.py, remotefilelog, eol, and probably keyword need to be taught the new kwarg. Note that the keyword wrapper is called kwcommitctx, so it isn't enough to grep for def commitctx.

This revision now requires changes to proceed.Jul 15 2019, 12:20 PM
valentin.gatienbaron added a comment.Jul 15 2019, 6:11 PM
In D6643#97171, @mharbison72 wrote:

Don't take this as a complete list, but it looks like repository.py, remotefilelog, eol, and probably keyword need to be taught the new kwarg. Note that the keyword wrapper is called kwcommitctx, so it isn't enough to grep for def commitctx.

I found the same places by grepping for commitctx, plus a minor one in a test, which I fixed.

mharbison72 accepted this revision.Jul 16 2019, 9:33 PM

The update worked for me, thanks.

I ran this on the repo that was giving me fits last summer, and it stayed consistent up until just after the point where there was an octopus fixup merge. I suspect the logic in that part of convert is too loose (i.e. it should be dropping files in the fixup merge that were already in the first merge if there weren't changes in the 3rd branch), but wasn't able to write a simple test. (The original repo was in bzr, converted to hg, and then I had to reconvert to lfs.) I've been swamped at work, so I don't have time to dig into it. But no sense in holding this up, as there doesn't seem to be a regression here.

Do you think there are other hash preserving things that can be added? (Not asking you to do that.) Or should we save this config name for such functionality? I don't have any better name ideas, and don't have a problem adding more to this config option.

valentin.gatienbaron added a comment.Jul 16 2019, 10:50 PM
In D6643#97247, @mharbison72 wrote:

The update worked for me, thanks.

Cool, thanks.

I ran this on the repo that was giving me fits last summer, and it stayed consistent up until just after the point where there was an octopus fixup merge. I suspect the logic in that part of convert is too loose (i.e. it should be dropping files in the fixup merge that were already in the first merge if there weren't changes in the 3rd branch), but wasn't able to write a simple test. (The original repo was in bzr, converted to hg, and then I had to reconvert to lfs.) I've been swamped at work, so I don't have time to dig into it. But no sense in holding this up, as there doesn't seem to be a regression here.

I didn't quite understand. Are you saying your convert call cause hashes to change after this octopus merge, but it's not caused by my change, i.e. hg behaves this way even before?

Do you think there are other hash preserving things that can be added? (Not asking you to do that.) Or should we save this config name for such functionality? I don't have any better name ideas, and don't have a problem adding more to this config option.

I don't plan on making more changes that impact hashes at least (other than more perhaps tweaking the changelog files list some more).
Changes to the choice of p1/p2 in manifestlog/filelogs revisions would cause spurious hash changes. Changes in this area don't seem that likely though.
But if such a change did happen, you'd probably want to preserve hashes regardless of the specific source of the hash change. And if not, more specialized options can be minted.

mharbison72 added a comment.Jul 17 2019, 12:17 AM
In D6643#97250, @valentin.gatienbaron wrote:
In D6643#97247, @mharbison72 wrote:

I ran this on the repo that was giving me fits last summer, and it stayed consistent up until just after the point where there was an octopus fixup merge. I suspect the logic in that part of convert is too loose (i.e. it should be dropping files in the fixup merge that were already in the first merge if there weren't changes in the 3rd branch), but wasn't able to write a simple test. (The original repo was in bzr, converted to hg, and then I had to reconvert to lfs.) I've been swamped at work, so I don't have time to dig into it. But no sense in holding this up, as there doesn't seem to be a regression here.

I didn't quite understand. Are you saying your convert call cause hashes to change after this octopus merge, but it's not caused by my change, i.e. hg behaves this way even before?

Yes, it was a pre-existing issue. It's the problem that I referenced here, which your patch got to without any of the hacking that I did:

https://www.mercurial-scm.org/pipermail/mercurial-devel/2018-July/118896.html

Do you think there are other hash preserving things that can be added? (Not asking you to do that.) Or should we save this config name for such functionality? I don't have any better name ideas, and don't have a problem adding more to this config option.

I don't plan on making more changes that impact hashes at least (other than more perhaps tweaking the changelog files list some more).
Changes to the choice of p1/p2 in manifestlog/filelogs revisions would cause spurious hash changes. Changes in this area don't seem that likely though.
But if such a change did happen, you'd probably want to preserve hashes regardless of the specific source of the hash change. And if not, more specialized options can be minted.

Correct. I just wanted to make sure that nobody has a problem with this config morphing into a general "try really hard to keep hashes" setting in the future, if other tweaks need to be made.

durin42 accepted this revision.Jul 17 2019, 6:23 PM
This revision is now accepted and ready to land.Jul 17 2019, 6:23 PM

Revision Contents
Changeset List

Diff 15904

hgext/convert/__init__.py

``default``. ``default``.
:convert.hg.usebranchnames: preserve branch names. The default is :convert.hg.usebranchnames: preserve branch names. The default is
True. True.
:convert.hg.sourcename: records the given string as a 'convert_source' extra :convert.hg.sourcename: records the given string as a 'convert_source' extra
value on each commit made in the target repository. The default is None. value on each commit made in the target repository. The default is None.
:convert.hg.preserve-hash: only works with mercurial sources. Make convert
prevent performance improvement to the list of modified files in commits
when such an improvement would cause the hash of a commit to change.
The default is False.
All Destinations All Destinations
################ ################
All destination types accept the following options: All destination types accept the following options:
:convert.skiptags: does not convert tags from the source repo to the target :convert.skiptags: does not convert tags from the source repo to the target
repo. The default is False. repo. The default is False.
""" """

hgext/convert/common.py

class NoRepo(Exception): class NoRepo(Exception):
pass pass
SKIPREV = 'SKIP' SKIPREV = 'SKIP'
class commit(object): class commit(object):
def __init__(self, author, date, desc, parents, branch=None, rev=None, def __init__(self, author, date, desc, parents, branch=None, rev=None,
extra=None, sortkey=None, saverev=True, phase=phases.draft, extra=None, sortkey=None, saverev=True, phase=phases.draft,
optparents=None): optparents=None, ctx=None):
self.author = author or 'unknown' self.author = author or 'unknown'
self.date = date or '0 0' self.date = date or '0 0'
self.desc = desc self.desc = desc
self.parents = parents # will be converted and used as parents self.parents = parents # will be converted and used as parents
self.optparents = optparents or [] # will be used if already converted self.optparents = optparents or [] # will be used if already converted
self.branch = branch self.branch = branch
self.rev = rev self.rev = rev
self.extra = extra or {} self.extra = extra or {}
self.sortkey = sortkey self.sortkey = sortkey
self.saverev = saverev self.saverev = saverev
self.phase = phase self.phase = phase
self.ctx = ctx # for hg to hg conversions
class converter_source(object): class converter_source(object):
"""Conversion source interface""" """Conversion source interface"""
def __init__(self, ui, repotype, path=None, revs=None): def __init__(self, ui, repotype, path=None, revs=None):
"""Initialize conversion source (or raise NoRepo("message") """Initialize conversion source (or raise NoRepo("message")
exception if path is not a valid repository)""" exception if path is not a valid repository)"""
self.ui = ui self.ui = ui

hgext/convert/hg.py

getfilectx, commit.author, commit.date, extra) getfilectx, commit.author, commit.date, extra)
# We won't know if the conversion changes the node until after the # We won't know if the conversion changes the node until after the
# commit, so copy the source's phase for now. # commit, so copy the source's phase for now.
self.repo.ui.setconfig('phases', 'new-commit', self.repo.ui.setconfig('phases', 'new-commit',
phases.phasenames[commit.phase], 'convert') phases.phasenames[commit.phase], 'convert')
with self.repo.transaction("convert") as tr: with self.repo.transaction("convert") as tr:
node = nodemod.hex(self.repo.commitctx(ctx)) if self.repo.ui.config('convert', 'hg.preserve-hash'):
origctx = commit.ctx
else:
origctx = None
node = nodemod.hex(self.repo.commitctx(ctx, origctx=origctx))
# If the node value has changed, but the phase is lower than # If the node value has changed, but the phase is lower than
# draft, set it back to draft since it hasn't been exposed # draft, set it back to draft since it hasn't been exposed
# anywhere. # anywhere.
if commit.rev != node: if commit.rev != node:
ctx = self.repo[node] ctx = self.repo[node]
if ctx.phase() < phases.draft: if ctx.phase() < phases.draft:
phases.registernew(self.repo, tr, phases.draft, phases.registernew(self.repo, tr, phases.draft,
desc=ctx.description(), desc=ctx.description(),
rev=crev, rev=crev,
parents=parents, parents=parents,
optparents=optparents, optparents=optparents,
branch=ctx.branch(), branch=ctx.branch(),
extra=ctx.extra(), extra=ctx.extra(),
sortkey=ctx.rev(), sortkey=ctx.rev(),
saverev=self.saverev, saverev=self.saverev,
phase=ctx.phase()) phase=ctx.phase(),
ctx=ctx)
def numcommits(self): def numcommits(self):
return len(self.repo) return len(self.repo)
def gettags(self): def gettags(self):
# This will get written to .hgtags, filter non global tags out. # This will get written to .hgtags, filter non global tags out.
tags = [t for t in self.repo.tagslist() tags = [t for t in self.repo.tagslist()
if self.repo.tagtype(t[0]) == 'global'] if self.repo.tagtype(t[0]) == 'global']

hgext/lfs/__init__.py

if not repo.local(): if not repo.local():
return return
repo.svfs.lfslocalblobstore = blobstore.local(repo) repo.svfs.lfslocalblobstore = blobstore.local(repo)
repo.svfs.lfsremoteblobstore = blobstore.remote(repo) repo.svfs.lfsremoteblobstore = blobstore.remote(repo)
class lfsrepo(repo.__class__): class lfsrepo(repo.__class__):
@localrepo.unfilteredmethod @localrepo.unfilteredmethod
def commitctx(self, ctx, error=False): def commitctx(self, ctx, error=False, origctx=None):
repo.svfs.options['lfstrack'] = _trackedmatcher(self) repo.svfs.options['lfstrack'] = _trackedmatcher(self)
return super(lfsrepo, self).commitctx(ctx, error) return super(lfsrepo, self).commitctx(ctx, error, origctx=origctx)
repo.__class__ = lfsrepo repo.__class__ = lfsrepo
if 'lfs' not in repo.requirements: if 'lfs' not in repo.requirements:
def checkrequireslfs(ui, repo, **kwargs): def checkrequireslfs(ui, repo, **kwargs):
if 'lfs' in repo.requirements: if 'lfs' in repo.requirements:
return 0 return 0

mercurial/configitems.py

default=False, default=False,
) )
coreconfigitem('convert', 'hg.clonebranches', coreconfigitem('convert', 'hg.clonebranches',
default=False, default=False,
) )
coreconfigitem('convert', 'hg.ignoreerrors', coreconfigitem('convert', 'hg.ignoreerrors',
default=False, default=False,
) )
coreconfigitem('convert', 'hg.preserve-hash',
default=False,
)
coreconfigitem('convert', 'hg.revs', coreconfigitem('convert', 'hg.revs',
default=None, default=None,
) )
coreconfigitem('convert', 'hg.saverev', coreconfigitem('convert', 'hg.saverev',
default=False, default=False,
) )
coreconfigitem('convert', 'hg.sourcename', coreconfigitem('convert', 'hg.sourcename',
default=None, default=None,

mercurial/localrepo.py

# temporary commit got stripped before hook release # temporary commit got stripped before hook release
if self.changelog.hasnode(ret): if self.changelog.hasnode(ret):
self.hook("commit", node=hex(ret), parent1=hookp1, self.hook("commit", node=hex(ret), parent1=hookp1,
parent2=hookp2) parent2=hookp2)
self._afterlock(commithook) self._afterlock(commithook)
return ret return ret
@unfilteredmethod @unfilteredmethod
def commitctx(self, ctx, error=False): def commitctx(self, ctx, error=False, origctx=None):
"""Add a new revision to current repository. """Add a new revision to current repository.
Revision information is passed via the context argument. Revision information is passed via the context argument.
ctx.files() should list all files involved in this commit, i.e. ctx.files() should list all files involved in this commit, i.e.
modified/added/removed files. On merge, it may be wider than the modified/added/removed files. On merge, it may be wider than the
ctx.files() to be committed, since any file nodes derived directly ctx.files() to be committed, since any file nodes derived directly
from p1 or p2 are excluded from the committed ctx.files(). from p1 or p2 are excluded from the committed ctx.files().
origctx is for convert to work around the problem that bug
fixes to the files list in changesets change hashes. For
convert to be the identity, it can pass an origctx and this
function will use the same files list when it makes sense to
do so.
""" """
p1, p2 = ctx.p1(), ctx.p2() p1, p2 = ctx.p1(), ctx.p2()
user = ctx.user() user = ctx.user()
writecopiesto = self.ui.config('experimental', 'copies.write-to') writecopiesto = self.ui.config('experimental', 'copies.write-to')
writefilecopymeta = writecopiesto != 'changeset-only' writefilecopymeta = writecopiesto != 'changeset-only'
writechangesetcopy = (writecopiesto in writechangesetcopy = (writecopiesto in
# no entry should be written. If writing to both, write an empty # no entry should be written. If writing to both, write an empty
# entry to prevent the reader from falling back to reading # entry to prevent the reader from falling back to reading
# filelogs. # filelogs.
p1copies = p1copies or None p1copies = p1copies or None
p2copies = p2copies or None p2copies = p2copies or None
filesadded = filesadded or None filesadded = filesadded or None
filesremoved = filesremoved or None filesremoved = filesremoved or None
if origctx and origctx.manifestnode() == mn:
files = origctx.files()
# update changelog # update changelog
self.ui.note(_("committing changelog\n")) self.ui.note(_("committing changelog\n"))
self.changelog.delayupdate(tr) self.changelog.delayupdate(tr)
n = self.changelog.add(mn, files, ctx.description(), n = self.changelog.add(mn, files, ctx.description(),
trp, p1.node(), p2.node(), trp, p1.node(), p2.node(),
user, ctx.date(), ctx.extra().copy(), user, ctx.date(), ctx.extra().copy(),
p1copies, p2copies, filesadded, filesremoved) p1copies, p2copies, filesadded, filesremoved)
xp1, xp2 = p1.hex(), p2 and p2.hex() or '' xp1, xp2 = p1.hex(), p2 and p2.hex() or ''

tests/test-convert-identity.t

  • This file was added.
Testing that convert.hg.preserve-hash=true can be used to make hg
convert from hg repo to hg repo preserve hashes, even if the
computation of the files list in commits change slightly between hg
versions.
$ cat <<'EOF' >> "$HGRCPATH"
> [extensions]
> convert =
> EOF
$ cat <<'EOF' > changefileslist.py
> from mercurial import (changelog, extensions)
> def wrap(orig, clog, manifest, files, *args, **kwargs):
> return orig(clog, manifest, ["a"], *args, **kwargs)
> def extsetup(ui):
> extensions.wrapfunction(changelog.changelog, 'add', wrap)
> EOF
$ hg init repo
$ cd repo
$ echo a > a; hg commit -qAm a
$ echo b > a; hg commit -qAm b
$ hg up -qr 0; echo c > c; hg commit -qAm c
$ hg merge -qr 1
$ hg commit -m_ --config extensions.x=../changefileslist.py
$ hg log -r . -T '{node|short} {files|json}\n'
c085bbe93d59 ["a"]
Now that we have a commit with a files list that's not what the
current hg version would create, check that convert either fixes it or
keeps it depending on config:
$ hg convert -q . ../convert
$ hg --cwd ../convert log -r tip -T '{node|short} {files|json}\n'
b7c4d4bbacd3 []
$ rm -rf ../convert
$ hg convert -q . ../convert --config convert.hg.preserve-hash=true
$ hg --cwd ../convert log -r tip -T '{node|short} {files|json}\n'
c085bbe93d59 ["a"]
$ rm -rf ../convert

tests/test-convert.t

convert.hg.tagsbranch convert.hg.tagsbranch
branch name for tag revisions, defaults to "default". branch name for tag revisions, defaults to "default".
convert.hg.usebranchnames convert.hg.usebranchnames
preserve branch names. The default is True. preserve branch names. The default is True.
convert.hg.sourcename convert.hg.sourcename
records the given string as a 'convert_source' extra value records the given string as a 'convert_source' extra value
on each commit made in the target repository. The default is on each commit made in the target repository. The default is
None. None.
convert.hg.preserve-hash
only works with mercurial sources. Make convert prevent
performance improvement to the list of modified files in
commits when such an improvement would cause the hash of a
commit to change. The default is False.
All Destinations All Destinations
################ ################
All destination types accept the following options: All destination types accept the following options:
convert.skiptags convert.skiptags
does not convert tags from the source repo to the target does not convert tags from the source repo to the target