{"thread":{"id":"36223","subject":"[PATCH 3/3] remote-hg: add test cases for null bookmarks","startedAt":"2014-03-19T12:33:17Z","lastAt":"2014-03-19T15:18:58Z","messageCount":6,"participants":["Max Horn","Antoine Pelisse"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"237067","messageId":"1395232399-60112-1-git-send-email-max@quendi.de","threadId":"36223","inReplyTo":null,"subject":"[PATCH 1/3] remote-hg: do not fail on invalid bookmarks","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2014-03-19T12:33:17Z","receivedAt":"2014-03-19T12:33:17Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"From: Antoine Pelisse \n\nMercurial can have bookmarks pointing to \"nullid\" (the empty root\nrevision), while Git can not have references to it.\nWhen cloning or fetching from a Mercurial repository that has such a\nbookmark, the import will fail because git-remote-hg will not be able to\ncreate the corresponding reference.\n\nWarn the user about the invalid reference, and continue the import,\ninstead of stopping right away.\n\nSigned-off-by: Antoine Pelisse \nSigned-off-by: Max Horn \n---\n contrib/remote-helpers/git-remote-hg | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\nindex eb89ef6..12d850e 100755\n--- a/contrib/remote-helpers/git-remote-hg\n+++ b/contrib/remote-helpers/git-remote-hg\n@@ -625,6 +625,9 @@ def list_head(repo, cur):\n def do_list(parser):\n repo = parser.repo\n for bmark, node in bookmarks.listbookmarks(repo).iteritems():\n+ if node == '0000000000000000000000000000000000000000':\n+ warn(\"Ignoring invalid bookmark '%s'\", bmark)\n+ continue\n bmarks[bmark] = repo[node]\n \n cur = repo.dirstate.branch()\n-- \n1.9.0.7.ga299b13\n"},{"id":"237066","messageId":"1395232399-60112-2-git-send-email-max@quendi.de","threadId":"36223","inReplyTo":"1395232399-60112-1-git-send-email-max@quendi.de","subject":"[PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2014-03-19T12:33:18Z","receivedAt":"2014-03-19T12:33:18Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"Fix the previous commit to workaround issues with edge cases: Specifically,\nremote-hg inserts a fake 'master' branch, unless the cloned hg repository\nalready contains a 'master' bookmark. If that 'master' bookmark happens\nto reference the 'null' commit, the preceding fix ignores it. This\nwould leave us in an inconsistent state. Avoid this by NOT ignoring\nnull bookmarks named 'master' or 'default' under suitable circumstances.\n\nSigned-off-by: Max Horn \n---\n contrib/remote-helpers/git-remote-hg | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\nindex 12d850e..49b2c2e 100755\n--- a/contrib/remote-helpers/git-remote-hg\n+++ b/contrib/remote-helpers/git-remote-hg\n@@ -626,8 +626,11 @@ def do_list(parser):\n repo = parser.repo\n for bmark, node in bookmarks.listbookmarks(repo).iteritems():\n if node == '0000000000000000000000000000000000000000':\n- warn(\"Ignoring invalid bookmark '%s'\", bmark)\n- continue\n+ if fake_bmark == 'default' and bmark == 'master':\n+ pass\n+ else:\n+ warn(\"Ignoring invalid bookmark '%s'\", bmark)\n+ continue\n bmarks[bmark] = repo[node]\n \n cur = repo.dirstate.branch()\n-- \n1.9.0.7.ga299b13\n"},{"id":"237065","messageId":"1395232399-60112-3-git-send-email-max@quendi.de","threadId":"36223","inReplyTo":"1395232399-60112-1-git-send-email-max@quendi.de","subject":"[PATCH 3/3] remote-hg: add test cases for null bookmarks","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2014-03-19T12:33:19Z","receivedAt":"2014-03-19T12:33:19Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"Signed-off-by: Max Horn \n---\n contrib/remote-helpers/test-hg.sh | 48 +++++++++++++++++++++++++++++++++++++++\n 1 file changed, 48 insertions(+)\n\ndiff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh\nindex a933b1e..8d01b32 100755\n--- a/contrib/remote-helpers/test-hg.sh\n+++ b/contrib/remote-helpers/test-hg.sh\n@@ -772,4 +772,52 @@ test_expect_success 'remote double failed push' '\n \t)\n '\n \n+test_expect_success 'clone remote with master null bookmark' '\n+\ttest_when_finished \"rm -rf gitrepo* hgrepo*\" &&\n+\n+\t(\n+\thg init hgrepo &&\n+\tcd hgrepo &&\n+\techo a >a &&\n+\thg add a &&\n+\thg commit -m a &&\n+\thg bookmark -r null master\n+\t) &&\n+\n+\tgit clone \"hg::hgrepo\" gitrepo &&\n+\tcheck gitrepo HEAD a\n+'\n+\n+test_expect_success 'clone remote with default null bookmark' '\n+\ttest_when_finished \"rm -rf gitrepo* hgrepo*\" &&\n+\n+\t(\n+\thg init hgrepo &&\n+\tcd hgrepo &&\n+\techo a >a &&\n+\thg add a &&\n+\thg commit -m a &&\n+\thg bookmark -r null -f default\n+\t) &&\n+\n+\tgit clone \"hg::hgrepo\" gitrepo &&\n+\tcheck gitrepo HEAD a\n+'\n+\n+test_expect_success 'clone remote with generic null bookmark' '\n+\ttest_when_finished \"rm -rf gitrepo* hgrepo*\" &&\n+\n+\t(\n+\thg init hgrepo &&\n+\tcd hgrepo &&\n+\techo a >a &&\n+\thg add a &&\n+\thg commit -m a &&\n+\thg bookmark -r null bmark\n+\t) &&\n+\n+\tgit clone \"hg::hgrepo\" gitrepo &&\n+\tcheck gitrepo HEAD a\n+'\n+\n test_done\n-- \n1.9.0.7.ga299b13\n"},{"id":"237069","messageId":"CALWbr2yb9_Kumm697w2c68xE6JPpkF9OfxvP2acsPjPFq=zboQ@mail.gmail.com","threadId":"36223","inReplyTo":"1395232399-60112-2-git-send-email-max@quendi.de","subject":"Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2014-03-19T13:07:38Z","receivedAt":"2014-03-19T13:07:38Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Hi Max,\n\nThank you for working on this.\nI believe it would be fair that you forget about patch 1/3 as you fix\nit in this patch (2/3).\nAlso, I think it would be best NOT to integrate a patch (mine) that\nbreaks a test, as it\nwould make bisect harder to use.\n\nThanks,\nAntoine\n\nOn Wed, Mar 19, 2014 at 1:33 PM, Max Horn wrote:\n> Fix the previous commit to workaround issues with edge cases: Specifically,\n> remote-hg inserts a fake 'master' branch, unless the cloned hg repository\n> already contains a 'master' bookmark. If that 'master' bookmark happens\n> to reference the 'null' commit, the preceding fix ignores it. This\n> would leave us in an inconsistent state. Avoid this by NOT ignoring\n> null bookmarks named 'master' or 'default' under suitable circumstances.\n>\n> Signed-off-by: Max Horn \n> ---\n> contrib/remote-helpers/git-remote-hg | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\n> index 12d850e..49b2c2e 100755\n> --- a/contrib/remote-helpers/git-remote-hg\n> +++ b/contrib/remote-helpers/git-remote-hg\n> @@ -626,8 +626,11 @@ def do_list(parser):\n> repo = parser.repo\n> for bmark, node in bookmarks.listbookmarks(repo).iteritems():\n> if node == '0000000000000000000000000000000000000000':\n> - warn(\"Ignoring invalid bookmark '%s'\", bmark)\n> - continue\n> + if fake_bmark == 'default' and bmark == 'master':\n> + pass\n> + else:\n> + warn(\"Ignoring invalid bookmark '%s'\", bmark)\n> + continue\n> bmarks[bmark] = repo[node]\n>\n> cur = repo.dirstate.branch()\n> --\n> 1.9.0.7.ga299b13\n>\n"},{"id":"237074","messageId":"CDB4DDFC-FF7F-4BE0-A0B5-0933A506F690@quendi.de","threadId":"36223","inReplyTo":"CALWbr2yb9_Kumm697w2c68xE6JPpkF9OfxvP2acsPjPFq=zboQ@mail.gmail.com","subject":"Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2014-03-19T15:00:53Z","receivedAt":"2014-03-19T15:00:53Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"Hi Antoine,\n\nOn 19.03.2014, at 14:07, Antoine Pelisse wrote:\n\n> Hi Max,\n> \n> Thank you for working on this.\n> I believe it would be fair that you forget about patch 1/3 as you fix\n> it in this patch (2/3).\n> Also, I think it would be best NOT to integrate a patch (mine) that\n> breaks a test, as it\n> would make bisect harder to use.\n\n\nOK, makes sense. I didn't want to step on anybodies feet by hijacking previously made work (however small or big it might be -- I've been burned by this before). Anyway, so I'll squash the first two commits together (or all three even?), and edit the message. But I'd like to properly attribute that you discovered the issue, so perhaps I can add something like \"Reported-by: Antoine Pelisse\" or so?\n\nMax\n\n> \n> Thanks,\n> Antoine\n> \n> On Wed, Mar 19, 2014 at 1:33 PM, Max Horn wrote:\n>> Fix the previous commit to workaround issues with edge cases: Specifically,\n>> remote-hg inserts a fake 'master' branch, unless the cloned hg repository\n>> already contains a 'master' bookmark. If that 'master' bookmark happens\n>> to reference the 'null' commit, the preceding fix ignores it. This\n>> would leave us in an inconsistent state. Avoid this by NOT ignoring\n>> null bookmarks named 'master' or 'default' under suitable circumstances.\n>> \n>> Signed-off-by: Max Horn \n>> ---\n>> contrib/remote-helpers/git-remote-hg | 7 +++++--\n>> 1 file changed, 5 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg\n>> index 12d850e..49b2c2e 100755\n>> --- a/contrib/remote-helpers/git-remote-hg\n>> +++ b/contrib/remote-helpers/git-remote-hg\n>> @@ -626,8 +626,11 @@ def do_list(parser):\n>> repo = parser.repo\n>> for bmark, node in bookmarks.listbookmarks(repo).iteritems():\n>> if node == '0000000000000000000000000000000000000000':\n>> - warn(\"Ignoring invalid bookmark '%s'\", bmark)\n>> - continue\n>> + if fake_bmark == 'default' and bmark == 'master':\n>> + pass\n>> + else:\n>> + warn(\"Ignoring invalid bookmark '%s'\", bmark)\n>> + continue\n>> bmarks[bmark] = repo[node]\n>> \n>> cur = repo.dirstate.branch()\n>> --\n>> 1.9.0.7.ga299b13\n>> \n> \n\n"},{"id":"237076","messageId":"CALWbr2xa9pJ5wXJGB8Q6ZL9CWsVCPdhW5n-VbGZpTsmgjd6XhQ@mail.gmail.com","threadId":"36223","inReplyTo":"CDB4DDFC-FF7F-4BE0-A0B5-0933A506F690@quendi.de","subject":"Re: [PATCH 2/3] remote-hg: allow invalid bookmarks in a few edge cases","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2014-03-19T15:18:58Z","receivedAt":"2014-03-19T15:18:58Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Mar 19, 2014 at 4:00 PM, Max Horn wrote:\n>> Thank you for working on this.\n>> I believe it would be fair that you forget about patch 1/3 as you fix\n>> it in this patch (2/3).\n>> Also, I think it would be best NOT to integrate a patch (mine) that\n>> breaks a test, as it\n>> would make bisect harder to use.\n>\n> OK, makes sense. I didn't want to step on anybodies feet by hijacking previously made work (however small or big it might be -- I've been burned by this before). Anyway, so I'll squash the first two commits together (or all three even?), and edit the message. But I'd like to properly attribute that you discovered the issue, so perhaps I can add something like \"Reported-by: Antoine Pelisse\" or so?\n\nYes,\nI think you can squash all three commits into one, and use the\nreported-by line that you mentioned.\n\nThanks,\nAntoine\n"}]}