{"thread":{"id":"33722","subject":"[PATCH v2 1/2] remote-bzr: convert all unicode keys to str","startedAt":"2013-05-04T00:31:05Z","lastAt":"2013-05-05T21:54:55Z","messageCount":10,"participants":["Felipe Contreras","Stefano Lattarini","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"216393","messageId":"1367627467-15132-1-git-send-email-felipe.contreras@gmail.com","threadId":"33722","inReplyTo":null,"subject":"[PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-04T00:31:05Z","receivedAt":"2013-05-04T00:31:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nThe previous version had an indentation bug (did I mention I hate python?).\n\nA few fixes to be applied on top of the massive changes already queued. Nothing\nmajor.\n\nFelipe Contreras (2):\n remote-bzr: convert all unicode keys to str\n remote-bzr: avoid bad refs\n\n contrib/remote-helpers/git-remote-bzr | 42 +++++++++++++++++++++--------------\n 1 file changed, 25 insertions(+), 17 deletions(-)\n\n-- \n1.8.3.rc0.401.g45bba44\n"},{"id":"216392","messageId":"1367627467-15132-2-git-send-email-felipe.contreras@gmail.com","threadId":"33722","inReplyTo":"1367627467-15132-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 1/2] remote-bzr: convert all unicode keys to str","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-04T00:31:06Z","receivedAt":"2013-05-04T00:31:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise some versions of bazaar might barf.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/remote-helpers/git-remote-bzr | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\nindex 161f831..bbaaa8f 100755\n--- a/contrib/remote-helpers/git-remote-bzr\n+++ b/contrib/remote-helpers/git-remote-bzr\n@@ -95,7 +95,7 @@ class Marks:\n return self.marks[rev]\n \n def to_rev(self, mark):\n- return self.rev_marks[mark]\n+ return str(self.rev_marks[mark])\n \n def next_mark(self):\n self.last_mark += 1\n@@ -621,7 +621,7 @@ def parse_commit(parser):\n files[path] = f\n \n committer, date, tz = committer\n- parents = [str(mark_to_rev(p)) for p in parents]\n+ parents = [mark_to_rev(p) for p in parents]\n revid = bzrlib.generate_ids.gen_revision_id(committer, date)\n props = {}\n props['branch-nick'] = branch.nick\n-- \n1.8.3.rc0.401.g45bba44\n"},{"id":"216394","messageId":"1367627467-15132-3-git-send-email-felipe.contreras@gmail.com","threadId":"33722","inReplyTo":"1367627467-15132-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 2/2] remote-bzr: avoid bad refs","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-04T00:31:07Z","receivedAt":"2013-05-04T00:31:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Turns out fast-export throws bad 'reset' commands because of a behavior\nin transport-helper that is not even needed.\n\nWe should ignore them, otherwise we will threat them as branches and\nfail.\n\nThis was fixed in v1.8.2, but some people use this script in older\nversions of git.\n\nAlso, check if the ref was a tag, and skip it for now.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/remote-helpers/git-remote-bzr | 38 +++++++++++++++++++++--------------\n 1 file changed, 23 insertions(+), 15 deletions(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\nindex bbaaa8f..0ef30f8 100755\n--- a/contrib/remote-helpers/git-remote-bzr\n+++ b/contrib/remote-helpers/git-remote-bzr\n@@ -682,23 +682,31 @@ def do_export(parser):\n die('unhandled export command: %s' % line)\n \n for ref, revid in parsed_refs.iteritems():\n- name = ref[len('refs/heads/'):]\n- branch = bzrlib.branch.Branch.open(branches[name])\n- branch.generate_revision_history(revid, marks.get_tip(name))\n+ if ref.startswith('refs/heads/'):\n+ name = ref[len('refs/heads/'):]\n+ branch = bzrlib.branch.Branch.open(branches[name])\n+ branch.generate_revision_history(revid, marks.get_tip(name))\n \n- if name in peers:\n- peer = bzrlib.branch.Branch.open(peers[name])\n- try:\n- peer.bzrdir.push_branch(branch, revision_id=revid)\n- except bzrlib.errors.DivergedBranches:\n- print \"error %s non-fast forward\" % ref\n- continue\n+ if name in peers:\n+ peer = bzrlib.branch.Branch.open(peers[name])\n+ try:\n+ peer.bzrdir.push_branch(branch, revision_id=revid)\n+ except bzrlib.errors.DivergedBranches:\n+ print \"error %s non-fast forward\" % ref\n+ continue\n \n- try:\n- wt = branch.bzrdir.open_workingtree()\n- wt.update()\n- except bzrlib.errors.NoWorkingTree:\n- pass\n+ try:\n+ wt = branch.bzrdir.open_workingtree()\n+ wt.update()\n+ except bzrlib.errors.NoWorkingTree:\n+ pass\n+ elif ref.startswith('refs/tags/'):\n+ # TODO: implement tag push\n+ print \"error %s pushing tags not supported\" % ref\n+ continue\n+ else:\n+ # transport-helper/fast-export bugs\n+ continue\n \n print \"ok %s\" % ref\n \n-- \n1.8.3.rc0.401.g45bba44\n"},{"id":"216408","messageId":"5184C939.4080505@gmail.com","threadId":"33722","inReplyTo":"1367627467-15132-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 2/2] remote-bzr: avoid bad refs","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2013-05-04T08:39:21Z","receivedAt":"2013-05-04T08:39:21Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 05/04/2013 02:31 AM, Felipe Contreras wrote:\n> Turns out fast-export throws bad 'reset' commands because of a behavior\n> in transport-helper that is not even needed.\n> \n> We should ignore them, otherwise we will threat\n>\ns/threat/treat/\n\n> them as branches and fail.\n> \n> This was fixed in v1.8.2, but some people use this script in older\n> versions of git.\n> \n> Also, check if the ref was a tag, and skip it for now.\n\nRegards,\n Stefano\n"},{"id":"216458","messageId":"7vd2t5uvi2.fsf@alter.siamese.dyndns.org","threadId":"33722","inReplyTo":"1367627467-15132-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-05T18:33:41Z","receivedAt":"2013-05-05T18:33:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> The previous version had an indentation bug (did I mention I hate python?).\n>\n> A few fixes to be applied on top of the massive changes already queued. Nothing\n> major.\n\n[2/2] may not matter much in the context of my tree (people would\nuse post 1.8.2 fast-export if they are using remote-bzr from 1.8.3\nfrom my tree ;-), but [1/2] sounds like it is a good thing to have\nin 1.8.3 (not \"on top of that 'massive' series\").\n\nAssuming the \"otherwise some version of bzr might barf\" problem is\nthat repo.generate_revision_history() in those versions may not\napply str() to its first parameter and the caller is expected to\npass a string there, or something?\n\nThanks.\n\n>\n> Felipe Contreras (2):\n> remote-bzr: convert all unicode keys to str\n> remote-bzr: avoid bad refs\n>\n> contrib/remote-helpers/git-remote-bzr | 42 +++++++++++++++++++++--------------\n> 1 file changed, 25 insertions(+), 17 deletions(-)\n"},{"id":"216459","messageId":"CAMP44s1D7LOhDGkZguosPiXyuJ5cP2hmgq4AWagwadrJYK1Pgg@mail.gmail.com","threadId":"33722","inReplyTo":"7vd2t5uvi2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-05T18:42:22Z","receivedAt":"2013-05-05T18:42:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> The previous version had an indentation bug (did I mention I hate python?).\n>>\n>> A few fixes to be applied on top of the massive changes already queued. Nothing\n>> major.\n>\n> [2/2] may not matter much in the context of my tree (people would\n> use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3\n> from my tree ;-),\n\nMaybe, but if even if they have the latest git, pushing a tag will\nfail miserably, and with the patch it would fail nicely :)\n\n> but [1/2] sounds like it is a good thing to have\n> in 1.8.3 (not \"on top of that 'massive' series\").\n>\n> Assuming the \"otherwise some version of bzr might barf\" problem is\n> that repo.generate_revision_history() in those versions may not\n> apply str() to its first parameter and the caller is expected to\n> pass a string there, or something?\n\nNo, there's no change to repo.generate_revision_history(), because we\nalready convert the elements of the array to strings, it's the other\ncallers of Marks::to_rev() that see a change, namely code that pushes\nto a remote, I think.\n\nAnd BTW, they are already strings, but unicode strings, because they\ncome from a json file, somehow bazaar doesn't like that, but it works\nfine in my machine without the patch. Shrugs.\n\nAlso, the emacs developers seem to be fine with all these changes,\nthere's only one patch pending that I need to cleanup.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216461","messageId":"7v4nehuu3o.fsf@alter.siamese.dyndns.org","threadId":"33722","inReplyTo":"CAMP44s1D7LOhDGkZguosPiXyuJ5cP2hmgq4AWagwadrJYK1Pgg@mail.gmail.com","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-05T19:03:55Z","receivedAt":"2013-05-05T19:03:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> The previous version had an indentation bug (did I mention I hate python?).\n>>>\n>>> A few fixes to be applied on top of the massive changes already queued. Nothing\n>>> major.\n>>\n>> [2/2] may not matter much in the context of my tree (people would\n>> use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3\n>> from my tree ;-),\n>\n> Maybe, but if even if they have the latest git, pushing a tag will\n> fail miserably, and with the patch it would fail nicely :)\n>\n>> but [1/2] sounds like it is a good thing to have\n>> in 1.8.3 (not \"on top of that 'massive' series\").\n>>\n>> Assuming the \"otherwise some version of bzr might barf\" problem is\n>> that repo.generate_revision_history() in those versions may not\n>> apply str() to its first parameter and the caller is expected to\n>> pass a string there, or something?\n>\n> No, there's no change to repo.generate_revision_history(), because we\n> already convert the elements of the array to strings, it's the other\n> callers of Marks::to_rev() that see a change, namely code that pushes\n> to a remote, I think.\n>\n> And BTW, they are already strings, but unicode strings, because they\n> come from a json file, somehow bazaar doesn't like that, but it works\n> fine in my machine without the patch. Shrugs.\n>\n> Also, the emacs developers seem to be fine with all these changes,\n> there's only one patch pending that I need to cleanup.\n\nSo do you want to queue these on top of the \"massive\" in 'next', not\ndirectly on 'master'?\n"},{"id":"216462","messageId":"CAMP44s3JtLzE0vne5VH+bHrLvSuOwaWwuGa7DFggjEOt6ixgTA@mail.gmail.com","threadId":"33722","inReplyTo":"7v4nehuu3o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-05T20:24:48Z","receivedAt":"2013-05-05T20:24:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 5, 2013 at 2:03 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> The previous version had an indentation bug (did I mention I hate python?).\n>>>>\n>>>> A few fixes to be applied on top of the massive changes already queued. Nothing\n>>>> major.\n>>>\n>>> [2/2] may not matter much in the context of my tree (people would\n>>> use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3\n>>> from my tree ;-),\n>>\n>> Maybe, but if even if they have the latest git, pushing a tag will\n>> fail miserably, and with the patch it would fail nicely :)\n>>\n>>> but [1/2] sounds like it is a good thing to have\n>>> in 1.8.3 (not \"on top of that 'massive' series\").\n>>>\n>>> Assuming the \"otherwise some version of bzr might barf\" problem is\n>>> that repo.generate_revision_history() in those versions may not\n>>> apply str() to its first parameter and the caller is expected to\n>>> pass a string there, or something?\n>>\n>> No, there's no change to repo.generate_revision_history(), because we\n>> already convert the elements of the array to strings, it's the other\n>> callers of Marks::to_rev() that see a change, namely code that pushes\n>> to a remote, I think.\n>>\n>> And BTW, they are already strings, but unicode strings, because they\n>> come from a json file, somehow bazaar doesn't like that, but it works\n>> fine in my machine without the patch. Shrugs.\n>>\n>> Also, the emacs developers seem to be fine with all these changes,\n>> there's only one patch pending that I need to cleanup.\n>\n> So do you want to queue these on top of the \"massive\" in 'next', not\n> directly on 'master'?\n\nIf they apply on master, master. But I'm confused, are the massive\nchanges not going to graduate to master? Because if not, I should\ncherry-pick the safest changes, as there's a lot of good stuff there.\n\n-- \nFelipe Contreras\n"},{"id":"216463","messageId":"7vzjw9ta8u.fsf@alter.siamese.dyndns.org","threadId":"33722","inReplyTo":"CAMP44s3JtLzE0vne5VH+bHrLvSuOwaWwuGa7DFggjEOt6ixgTA@mail.gmail.com","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-05T20:58:09Z","receivedAt":"2013-05-05T20:58:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> So do you want to queue these on top of the \"massive\" in 'next', not\n>> directly on 'master'?\n>\n> If they apply on master, master. But I'm confused, are the massive\n> changes not going to graduate to master? Because if not, I should\n> cherry-pick the safest changes, as there's a lot of good stuff there.\n\nI think we discussed and agreed that we would ship it in 1.8.3 if we\nhear positive feedback from Emacs folks, and my understanding is\nthat I was waiting for you to give me a go-ahead once that happens.\n\nIt is entirely up to you to add these two on top of that \"massive\"\nstuff, their fate decided by feedback from Emacs folks, or apply\nthese as \"much safer than those we need to hear from them; we can\nverify their validity and safety ourselves without knowing the real\nworld projects that use the program\" patches.\n\nThe impression I was getting from your response \"I hear it breaks\nfor some of them without the patch but I haven't seen the breakage\nmyself\" is that it is safer to group 2/2 as part of the rest of the\nseries, but as I heard in the same message that you heard Emacs\nfolks are happy with the entire series, so it wouldn't make much of\na difference either way.\n\nWill apply these two to the tip of the \"massive\" stuff, and merge\nthe result before the next -rc.\n\nThanks.\n"},{"id":"216464","messageId":"CAMP44s3W79ZY_s2jq8-KtdMSqFRvCo3RC7ojcTzgGYJkzoDVnA@mail.gmail.com","threadId":"33722","inReplyTo":"7vzjw9ta8u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/2] remote-bzr: couple of fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-05T21:54:55Z","receivedAt":"2013-05-05T21:54:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 5, 2013 at 3:58 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>>> So do you want to queue these on top of the \"massive\" in 'next', not\n>>> directly on 'master'?\n>>\n>> If they apply on master, master. But I'm confused, are the massive\n>> changes not going to graduate to master? Because if not, I should\n>> cherry-pick the safest changes, as there's a lot of good stuff there.\n>\n> I think we discussed and agreed that we would ship it in 1.8.3 if we\n> hear positive feedback from Emacs folks, and my understanding is\n> that I was waiting for you to give me a go-ahead once that happens.\n\nYeah, and I just said everything seems to be fine. There's only one\nmore patch that would be good to have that I still haven't cleaned up.\n\n> It is entirely up to you to add these two on top of that \"massive\"\n> stuff, their fate decided by feedback from Emacs folks, or apply\n> these as \"much safer than those we need to hear from them; we can\n> verify their validity and safety ourselves without knowing the real\n> world projects that use the program\" patches.\n>\n> The impression I was getting from your response \"I hear it breaks\n> for some of them without the patch but I haven't seen the breakage\n> myself\" is that it is safer to group 2/2 as part of the rest of the\n> series, but as I heard in the same message that you heard Emacs\n> folks are happy with the entire series, so it wouldn't make much of\n> a difference either way.\n>\n> Will apply these two to the tip of the \"massive\" stuff, and merge\n> the result before the next -rc.\n\nCool, I think that's the best approach. I'll send the last patch later today.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}