Page MenuHomePhabricator

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

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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

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.

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.

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