{"thread":{"id":"33601","subject":"[PATCH] remote-bzr: use proper push method","startedAt":"2013-04-25T11:25:37Z","lastAt":"2013-04-25T22:32:26Z","messageCount":6,"participants":["Felipe Contreras","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"215439","messageId":"1366889137-19700-1-git-send-email-felipe.contreras@gmail.com","threadId":"33601","inReplyTo":null,"subject":"[PATCH] remote-bzr: use proper push method","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-25T11:25:37Z","receivedAt":"2013-04-25T11:25:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Not just randomly synchronize the revisions with no checks at all. This\nis the way bazaar's UI does it.\n\nAlso, add a non-ff check.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n\nThis patch should probably go to maint, as the results of pushing the way we\ncurrently push are not really understood. Perhaps it's similar to a 'git push\n --force', or perhaps it can potentially screw the repository.\n\nIt's better to be safe and just do what bazaar does.\n\n contrib/remote-helpers/git-remote-bzr | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr\nindex 8c316fe..cb8d081 100755\n--- a/contrib/remote-helpers/git-remote-bzr\n+++ b/contrib/remote-helpers/git-remote-bzr\n@@ -639,12 +639,12 @@ def do_export(parser):\n     for ref, revid in parsed_refs.iteritems():\n         if ref == 'refs/heads/master':\n             repo.generate_revision_history(revid, marks.get_tip('master'))\n-            revno, revid = repo.last_revision_info()\n             if peer:\n-                if hasattr(peer, \"import_last_revision_info_and_tags\"):\n-                    peer.import_last_revision_info_and_tags(repo, revno, revid)\n-                else:\n-                    peer.import_last_revision_info(repo.repository, revno, revid)\n+                try:\n+                    repo.push(peer, stop_revision=revid)\n+                except bzrlib.errors.DivergedBranches:\n+                    print \"error %s non-fast forward\" % ref\n+                    continue\n             else:\n                 wt = repo.bzrdir.open_workingtree()\n                 wt.update()\n-- \n1.8.2.1\n"},{"id":"215445","messageId":"87haiu7jgn.fsf@linux-k42r.v.cablecom.net","threadId":"33601","inReplyTo":"1366889137-19700-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] remote-bzr: use proper push method","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-25T14:50:32Z","receivedAt":"2013-04-25T14:50:32Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Not just randomly synchronize the revisions with no checks at all. This\n> is the way bazaar's UI does it.\n>\n> Also, add a non-ff check.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>\n> This patch should probably go to maint, as the results of pushing the way we\n> currently push are not really understood. Perhaps it's similar to a 'git push\n>  --force', or perhaps it can potentially screw the repository.\n>\n> It's better to be safe and just do what bazaar does.\n\nOther than \"this patch should probably go to maint\", this should be in\nthe commit message.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215509","messageId":"7v1u9y2u4q.fsf@alter.siamese.dyndns.org","threadId":"33601","inReplyTo":"87haiu7jgn.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH] remote-bzr: use proper push method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-25T21:11:17Z","receivedAt":"2013-04-25T21:11:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@inf.ethz.ch> writes:\n\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Not just randomly synchronize the revisions with no checks at all. This\n>> is the way bazaar's UI does it.\n>>\n>> Also, add a non-ff check.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>\n>> This patch should probably go to maint, as the results of pushing the way we\n>> currently push are not really understood. Perhaps it's similar to a 'git push\n>>  --force', or perhaps it can potentially screw the repository.\n>>\n>> It's better to be safe and just do what bazaar does.\n>\n> Other than \"this patch should probably go to maint\", this should be in\n> the commit message.\n\nHmph, should it?\n\nI do not quite understand what \"... are not really understood.\nPerhaps...\" wants to say.  Understood by whom?  By the author of the\npatch?  By the author of the original code?\n\nThe log would end up saying \"Doing the same as bazaar should be the\nright thing to do(TM), but don't ask me why.  I do not know what I\nam doing, or why checking is better than not checking, but it seems\nto work\".  That _could_ be the truth, but it won't help people who\nare reading the code later, will it?\n"},{"id":"215514","messageId":"CAMP44s2-QZxuV-bXc_0zeqxJDy=Y6AAC0iwgbjNDNuCaaMcErA@mail.gmail.com","threadId":"33601","inReplyTo":"7v1u9y2u4q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote-bzr: use proper push method","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-25T21:41:47Z","receivedAt":"2013-04-25T21:41:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Apr 25, 2013 at 4:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Thomas Rast <trast@inf.ethz.ch> writes:\n>\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> Not just randomly synchronize the revisions with no checks at all. This\n>>> is the way bazaar's UI does it.\n>>>\n>>> Also, add a non-ff check.\n>>>\n>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>>> ---\n>>>\n>>> This patch should probably go to maint, as the results of pushing the way we\n>>> currently push are not really understood. Perhaps it's similar to a 'git push\n>>>  --force', or perhaps it can potentially screw the repository.\n>>>\n>>> It's better to be safe and just do what bazaar does.\n>>\n>> Other than \"this patch should probably go to maint\", this should be in\n>> the commit message.\n>\n> Hmph, should it?\n>\n> I do not quite understand what \"... are not really understood.\n> Perhaps...\" wants to say.  Understood by whom?  By the author of the\n> patch?  By the author of the original code?\n>\n> The log would end up saying \"Doing the same as bazaar should be the\n> right thing to do(TM), but don't ask me why.  I do not know what I\n> am doing, or why checking is better than not checking, but it seems\n> to work\".  That _could_ be the truth, but it won't help people who\n> are reading the code later, will it?\n\nIn addition to that, it's doubtful there's anybody who would actually\nknow. Bazaar is basically a dead and unmaintained project now, and it\nhas been for quite some time. It seems the people that had any idea\nare long gone.\n\nMy gut feeling is that we should do it the way the Bazaar UI does it,\nI don't have any evidence that there's anything wrong with the current\ncode, which Bazaar seems to but have, but for different purposes which\nare hard to explain. I would rather avoid surprises.\n\n-- \nFelipe Contreras\n"},{"id":"215516","messageId":"CAMP44s3S2A2ymEjGZVQjYx8k08x11WYTuy=y_e-MHa_qM78Zxw@mail.gmail.com","threadId":"33601","inReplyTo":"CAMP44s2-QZxuV-bXc_0zeqxJDy=Y6AAC0iwgbjNDNuCaaMcErA@mail.gmail.com","subject":"Re: [PATCH] remote-bzr: use proper push method","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-25T21:50:51Z","receivedAt":"2013-04-25T21:50:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Apr 25, 2013 at 4:41 PM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> On Thu, Apr 25, 2013 at 4:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Thomas Rast <trast@inf.ethz.ch> writes:\n>>\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> Not just randomly synchronize the revisions with no checks at all. This\n>>>> is the way bazaar's UI does it.\n>>>>\n>>>> Also, add a non-ff check.\n>>>>\n>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>>>> ---\n>>>>\n>>>> This patch should probably go to maint, as the results of pushing the way we\n>>>> currently push are not really understood. Perhaps it's similar to a 'git push\n>>>>  --force', or perhaps it can potentially screw the repository.\n>>>>\n>>>> It's better to be safe and just do what bazaar does.\n>>>\n>>> Other than \"this patch should probably go to maint\", this should be in\n>>> the commit message.\n>>\n>> Hmph, should it?\n>>\n>> I do not quite understand what \"... are not really understood.\n>> Perhaps...\" wants to say.  Understood by whom?  By the author of the\n>> patch?  By the author of the original code?\n>>\n>> The log would end up saying \"Doing the same as bazaar should be the\n>> right thing to do(TM), but don't ask me why.  I do not know what I\n>> am doing, or why checking is better than not checking, but it seems\n>> to work\".  That _could_ be the truth, but it won't help people who\n>> are reading the code later, will it?\n>\n> In addition to that, it's doubtful there's anybody who would actually\n> know. Bazaar is basically a dead and unmaintained project now, and it\n> has been for quite some time. It seems the people that had any idea\n> are long gone.\n>\n> My gut feeling is that we should do it the way the Bazaar UI does it,\n> I don't have any evidence that there's anything wrong with the current\n> code, which Bazaar seems to but have, but for different purposes which\n> are hard to explain. I would rather avoid surprises.\n\nEr,which Bazaar seems to use, but for different purposes...\n\n-- \nFelipe Contreras\n"},{"id":"215518","messageId":"7vobd21bt1.fsf@alter.siamese.dyndns.org","threadId":"33601","inReplyTo":"CAMP44s2-QZxuV-bXc_0zeqxJDy=Y6AAC0iwgbjNDNuCaaMcErA@mail.gmail.com","subject":"Re: [PATCH] remote-bzr: use proper push method","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-25T22:32:26Z","receivedAt":"2013-04-25T22:32:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> My gut feeling is that we should do it the way the Bazaar UI does it,\n> I don't have any evidence that there's anything wrong with the current\n> code, which Bazaar seems to but have, but for different purposes which\n> are hard to explain. I would rather avoid surprises.\n\nThat is actually a very fine description for this change ;-).\n\ncommit 473b7aac9f542dc22cfff0c264e96d8dbbd9d895\nAuthor: Felipe Contreras <felipe.contreras@gmail.com>\nDate:   Thu Apr 25 06:25:37 2013 -0500\n\n    remote-bzr: use proper push method\n    \n    Do not just randomly synchronize the revisions with no checks at\n    all.\n    \n    I don't have any evidence that there's anything wrong with the\n    current code, which Bazaar seems to use, but for different purposes.\n    Let's use the logic Bazaar UI uses to avoid surprises.\n    \n    Also, add a non-ff check.\n    \n    Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"}]}