I noticed that duplicatecopies() can end up marking a file as a copy
even if the file already exists in the parent. At least when using
overlayworkingctx for creating the commit, it ended up in the commit's
"files" field, which means it showed as a modified in `hg status
--change .`. It doesn't seem like we produced any such commit in core,
but we did end up producing some empty commits, as can be seen in the
updated tests.
Details
- Reviewers
- None
- Group Reviewers
hg-reviewers
Diff Detail
- Repository
- rHG Mercurial
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
- a/tests/test-issue1175.t
+++ b/tests/test-issue1175.t
@@ -82,7 +82,6 @@continue: hg graft --continue $ hg graft --continue grafting 1:5974126fad84 "b1"
- warning: can't find ancestor for 'b' copied from 'a'! $ hg log -f b -T 'changeset: {rev}:{node|short}\nsummary: {desc}\n\n' changeset: 3:376d30ccffc0 summary: b1
This test was added at a43fdf33a6be, which might need bad copy information.
- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -810,10 +810,11 @@
- of the function is much faster (and is required for carrying copy
- metadata across the rebase anyway). exclude = pathcopies(repo[fromrev], repo[skiprev])
+ pctx = wctx.p1()
for dst, src in pycompat.iteritems(pathcopies(repo[fromrev], repo[rev])): if dst in exclude: continue
- if dst in wctx:
+ if dst in wctx and dst not in pctx:
wctx[dst].markcopied(src)
Seems fine as we don't support hg rm b; hg cp a b case.
Ah, so maybe the bug used to be incorrectly suppressed by the logic for finding the source in an old version. Thanks for digging that up. I somehow didn't think to check.
- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -810,10 +810,11 @@
- of the function is much faster (and is required for carrying copy
- metadata across the rebase anyway). exclude = pathcopies(repo[fromrev], repo[skiprev])
+ pctx = wctx.p1()
for dst, src in pycompat.iteritems(pathcopies(repo[fromrev], repo[rev])): if dst in exclude: continue
- if dst in wctx:
+ if dst in wctx and dst not in pctx:
wctx[dst].markcopied(src)Seems fine as we don't support hg rm b; hg cp a b case.
Yes, exactly. I should have mentioned that.
I was working on something unrelated just now and it turned out to be a little related to this. I was reminded that we have hg cp --force to "forcibly copy over an existing managed file". If you don't use that and try to copy onto an existing file, you'll get this hint: "'hg copy --force' to replace the file by recording a copy". It sounds like the plan was to allow you to copy a file onto an existing one to record it as a copy. We also have tests in test-copy.t showing the status of that. So I think I should rethink this patch.