Here is the repro: #!/bin/bash cat << EOF > /tmp/lolomghgrc [ui] username = omg wtf lol interactive = True [extensions] record= [committemplate] changeset = {desc}\n\n HG: {extramsg} HG: user: {author}\n{ifeq(p2rev, "-1", "", "HG: branch merge\n") }HG: branch '{branch}'\n{if(currentbookmark, "HG: bookmark '{currentbookmark}'\n") }{subrepos % "HG: subrepo {subrepo}\n" } {splitlines(diff()) % 'HG: {line}\n'} EOF export HGRCPATH=/tmp/lolomghgrc hg init lol cd lol echo a > a echo b > b hg addr hg ci -m 'init' hg rm b hg ci -m 'rm b' echo a >> a echo -e "y\nA" | hg record --amend unset HGRCPATH cd .. This results in the following incorrect error message: transaction abort! rollback completed abort: template filter 'splitlines' is not compatible with keyword '[]' The problem is actually that in the following line of memctx: man[f] = revlog.hash(self[f].data(), p1node, p2node) (mercurial/context.py:1643 at rev 406dfc63a1ad: http://inversethought.com/hg/mercurial-crew/file/406dfc63a1ad/mercurial/context.py#l1635 ) self[f] will be None if f is a removed file. This exception then gets swallowed by the templater who concludes the problem is with the combination of splitlines() and `diff`.
Oops, record's got nothing to do with it. Here is a simpler repro, without record: #!/bin/bash cat << EOF > /tmp/lolomghgrc [ui] username = omg wtf lol [committemplate] changeset = {desc}\n\n HG: {extramsg} HG: user: {author}\n{ifeq(p2rev, "-1", "", "HG: branch merge\n") }HG: branch '{branch}'\n{if(currentbookmark, "HG: bookmark '{currentbookmark}'\n") }{subrepos % "HG: subrepo {subrepo}\n" } {splitlines(diff()) % 'HG: {line}\n'} EOF export HGRCPATH=/tmp/lolomghgrc hg init lol cd lol echo a > a echo b > b hg addr hg ci -m 'init' hg rm b hg ci -m 'rm b' echo a >> a hg commit --amend unset HGRCPATH cd ..
I tried converting your test to a .t-test, and it doesn't fail for me. Will try your repro script next. # HG changeset patch # User Augie Fackler <raf@durin42.com> # Date 1418673654 18000 # Mon Dec 15 15:00:54 2014 -0500 # Node ID 35fd4b7f803c33d71c2f4469193fb4ac25a58061 # Parent 65c854f92d6ba8861414ff3191182fba28777a82 test-commit: try to expose a bug with memctx and showing removed files This isn't failing for me, but maybe I've done something wrong. diff --git a/tests/test-commit.t b/tests/test-commit.t --- a/tests/test-commit.t +++ b/tests/test-commit.t @@ -429,6 +429,37 @@ specific template keywords work well abort: empty commit message [255] +prove that we can show a diff of an amend using committemplate: + + $ cat >> .hg/hgrc <<EOF + > [committemplate] + > changeset = {desc}\n\n + > HG: {extramsg} + > HG: user: {author}\n{ifeq(p2rev, "-1", "", + > "HG: branch merge\n") + > }HG: branch '{branch}'\n{if(currentbookmark, + > "HG: bookmark '{currentbookmark}'\n") }{subrepos % + > "HG: subrepo {subrepo}\n" } + > {splitlines(diff()) % 'HG: {line}\n'} + > EOF + $ hg init lol + $ cd lol + $ echo a > a + $ echo b > b + $ hg addr + adding a + adding b + $ hg ci -m 'init' + $ hg rm b + $ hg ci -m 'rm b' + $ echo a >> a + $ hg commit --amend + saved backup bundle to * (glob) + $ cd .. + $ rm -r lol + + +cleanup $ cat >> .hg/hgrc <<EOF > # disable customizing for subsequent tests > [committemplate]
When run-tests.py runs, what does it do about bringing up an editor to edit commit messages? If the editor isn't being loaded, then the problematic codepath won't get activated.
Oh, I see the problem. You're writing the config to an hgrc that doesn't actually get used to the test. I moved the hg init above the modification to .hg/hgrc so that it's actually used. Here's the correct test. Feel free to push it yourself, if you want. # HG changeset patch # User Augie Fackler <raf@durin42.com> # Date 1418673654 18000 # Mon Dec 15 15:00:54 2014 -0500 # Node ID 8f1b2a9eec51fd9320f091b28eca6fdcc87bf9dd # Parent 495bc1b65d25872324a0220354f048b220304bd1 test-commit: try to expose a bug with memctx and showing removed files Changing HGEDITOR=cat here should also show the commit diff in the commit message, but even if the memctx bug is fixed (issue4470), there's a separate bug about not showing the correct diff. diff --git a/tests/test-commit.t b/tests/test-commit.t --- a/tests/test-commit.t +++ b/tests/test-commit.t @@ -429,6 +429,37 @@ specific template keywords work well abort: empty commit message [255] +prove that we can show a diff of an amend using committemplate: + + $ hg init lol + $ cd lol + $ cat >> .hg/hgrc <<EOF + > [committemplate] + > changeset = {desc}\n\n + > HG: {extramsg} + > HG: user: {author}\n{ifeq(p2rev, "-1", "", + > "HG: branch merge\n") + > }HG: branch '{branch}'\n{if(currentbookmark, + > "HG: bookmark '{currentbookmark}'\n") }{subrepos % + > "HG: subrepo {subrepo}\n" } + > {splitlines(diff()) % 'HG: {line}\n'} + > EOF + $ echo a > a + $ echo b > b + $ hg addr + adding a + adding b + $ hg ci -m 'init' + $ hg rm b + $ hg ci -m 'rm b' + $ echo a >> a + $ HGEDITOR=cat hg commit --amend + saved backup bundle to * (glob) + $ cd .. + $ rm -r lol + + +cleanup $ cat >> .hg/hgrc <<EOF > # disable customizing for subsequent tests > [committemplate]
+ transaction abort! + rollback completed + abort: template filter 'splitlines' is not compatible with keyword '[]' + [255] Is that the failure I should be expecting?
> Is that the failure I should be expecting? Yes. But like I said originally, that's actually the templater swallowing the AttributeError exception from memctx.
Fixed by http://selenic.com/repo/hg/rev/db03ed8cbfa3 Augie Fackler <augie@google.com> memctx: fix manifest for removed files (issue4470) filectxfn returns None for removed files, so we have to check for None before computing the new file content hash for the manifest. Includes a test that proves this works, by demonstrating that we can show the diff of an amended commit in the committemplate. (please test the fix)
Fixed by http://selenic.com/repo/hg/rev/200215cdf7aa FUJIWARA Katsunori <foozy@lares.dti.ne.jp> memctx: calculate manifest correctly with newly-removed files (issue4470) Before this patch, "memctx._manifest" tries to get (and use normally) filectx also for newly-removed files, even though "memctx.filectx()" returns None for such files. To calculate manifest correctly even with newly-removed files, this patch does: - replace "man.iteritems()" for the loop by "self._status.modified" to avoid accessing itself to newly removed files this also reduces loop cost for large manifest. - remove files in "self._status.removed" from the manifest In this patch, amending is confirmed twice to examine both (1) newly removed files and (2) ones already removed in amended revision. (please test the fix)
Fixed by http://selenic.com/repo/hg/rev/d74eb8d477d5 FUJIWARA Katsunori <foozy@lares.dti.ne.jp> memctx: calculate manifest more efficiently Before this patch, "memctx._manifest" updates all entries in the (parent) manifest. But this is inefficiency, because almost all files may be clean in that context. On the other hand, just updating entries for changed "files" specified at construction causes unexpected abortion, when there is at least one newly removed file (see issue4470 for detail). To calculate manifest more efficiently, this patch replaces "pman.iteritems()" for the loop by "self._status.modified" to avoid updating entries for clean or removed files Examination of removal is also omitted, because removed files aren't treated in this loop (= "self[f]" returns not None always). (please test the fix)
Fixed by http://selenic.com/repo/hg/rev/bb304f9b05d0 FUJIWARA Katsunori <foozy@lares.dti.ne.jp> memctx: remove redundant test for issue4470 from test-commit.t Because: - the test to avoid regression for issue4470 was already added to test-commit-amend.t by previous patch It is also a part of test series about manifest calculation issues of memctx in test-commit-amend.t. - this is the only test using "commit --amend" in test-commit.t (please test the fix)
Bulk testing -> fixed