threads / patch / 33601

patchremote-bzr: use proper push method

Subject: [PATCH] remote-bzr: use proper push method

## tl;dr

6 messages between Apr 25, 2013 and Apr 25, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Felipe Contreras· Apr 25, 2013, 11:25 UTC · lore

Not just randomly synchronize the revisions with no checks at all. This is the way bazaar's UI does it.

Also, add a non-ff check.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
This patch should probably go to maint, as the results of pushing the way we
currently push are not really understood. Perhaps it's similar to a 'git push
 --force', or perhaps it can potentially screw the repository.
It's better to be safe and just do what bazaar does.
 contrib/remote-helpers/git-remote-bzr | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Show changes to contrib/remote-helpers/git-remote-bzr +5 −5
diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 8c316fe..cb8d081 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -639,12 +639,12 @@ def do_export(parser):
     for ref, revid in parsed_refs.iteritems():
         if ref == 'refs/heads/master':
             repo.generate_revision_history(revid, marks.get_tip('master'))
-            revno, revid = repo.last_revision_info()
             if peer:
-                if hasattr(peer, "import_last_revision_info_and_tags"):
-                    peer.import_last_revision_info_and_tags(repo, revno, revid)
-                else:
-                    peer.import_last_revision_info(repo.repository, revno, revid)
+                try:
+                    repo.push(peer, stop_revision=revid)
+                except bzrlib.errors.DivergedBranches:
+                    print "error %s non-fast forward" % ref
+                    continue
             else:
                 wt = repo.bzrdir.open_workingtree()
                 wt.update()

-- 
1.8.2.1
Thomas Rast· Apr 25, 2013, 14:50 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-bzr: use proper push method

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 13 quoted lines
> Not just randomly synchronize the revisions with no checks at all. This
> is the way bazaar's UI does it.
>
> Also, add a non-ff check.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>
> This patch should probably go to maint, as the results of pushing the way we
> currently push are not really understood. Perhaps it's similar to a 'git push
>  --force', or perhaps it can potentially screw the repository.
>
> It's better to be safe and just do what bazaar does.

Other than "this patch should probably go to maint", this should be in the commit message.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Junio C Hamano· Apr 25, 2013, 21:11 UTC · re: Thomas Rast · lore

Re: [PATCH] remote-bzr: use proper push method

Thomas Rast <trast@inf.ethz.ch> writes:
Show 18 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Not just randomly synchronize the revisions with no checks at all. This
>> is the way bazaar's UI does it.
>>
>> Also, add a non-ff check.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> This patch should probably go to maint, as the results of pushing the way we
>> currently push are not really understood. Perhaps it's similar to a 'git push
>>  --force', or perhaps it can potentially screw the repository.
>>
>> It's better to be safe and just do what bazaar does.
>
> Other than "this patch should probably go to maint", this should be in
> the commit message.
Hmph, should it?

I do not quite understand what "... are not really understood. Perhaps..." wants to say. Understood by whom? By the author of the patch? By the author of the original code?

The log would end up saying "Doing the same as bazaar should be the right thing to do(TM), but don't ask me why. I do not know what I am doing, or why checking is better than not checking, but it seems to work". That _could_ be the truth, but it won't help people who are reading the code later, will it?

Felipe Contreras· Apr 25, 2013, 21:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] remote-bzr: use proper push method

On Thu, Apr 25, 2013 at 4:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 32 quoted lines
> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Not just randomly synchronize the revisions with no checks at all. This
>>> is the way bazaar's UI does it.
>>>
>>> Also, add a non-ff check.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>>
>>> This patch should probably go to maint, as the results of pushing the way we
>>> currently push are not really understood. Perhaps it's similar to a 'git push
>>>  --force', or perhaps it can potentially screw the repository.
>>>
>>> It's better to be safe and just do what bazaar does.
>>
>> Other than "this patch should probably go to maint", this should be in
>> the commit message.
>
> Hmph, should it?
>
> I do not quite understand what "... are not really understood.
> Perhaps..." wants to say.  Understood by whom?  By the author of the
> patch?  By the author of the original code?
>
> The log would end up saying "Doing the same as bazaar should be the
> right thing to do(TM), but don't ask me why.  I do not know what I
> am doing, or why checking is better than not checking, but it seems
> to work".  That _could_ be the truth, but it won't help people who
> are reading the code later, will it?

In addition to that, it's doubtful there's anybody who would actually know. Bazaar is basically a dead and unmaintained project now, and it has been for quite some time. It seems the people that had any idea are long gone.

My gut feeling is that we should do it the way the Bazaar UI does it, I don't have any evidence that there's anything wrong with the current code, which Bazaar seems to but have, but for different purposes which are hard to explain. I would rather avoid surprises.

-- 
Felipe Contreras
Felipe Contreras· Apr 25, 2013, 21:50 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-bzr: use proper push method

On Thu, Apr 25, 2013 at 4:41 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 43 quoted lines
> On Thu, Apr 25, 2013 at 4:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Thomas Rast <trast@inf.ethz.ch> writes:
>>
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> Not just randomly synchronize the revisions with no checks at all. This
>>>> is the way bazaar's UI does it.
>>>>
>>>> Also, add a non-ff check.
>>>>
>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>>> ---
>>>>
>>>> This patch should probably go to maint, as the results of pushing the way we
>>>> currently push are not really understood. Perhaps it's similar to a 'git push
>>>>  --force', or perhaps it can potentially screw the repository.
>>>>
>>>> It's better to be safe and just do what bazaar does.
>>>
>>> Other than "this patch should probably go to maint", this should be in
>>> the commit message.
>>
>> Hmph, should it?
>>
>> I do not quite understand what "... are not really understood.
>> Perhaps..." wants to say.  Understood by whom?  By the author of the
>> patch?  By the author of the original code?
>>
>> The log would end up saying "Doing the same as bazaar should be the
>> right thing to do(TM), but don't ask me why.  I do not know what I
>> am doing, or why checking is better than not checking, but it seems
>> to work".  That _could_ be the truth, but it won't help people who
>> are reading the code later, will it?
>
> In addition to that, it's doubtful there's anybody who would actually
> know. Bazaar is basically a dead and unmaintained project now, and it
> has been for quite some time. It seems the people that had any idea
> are long gone.
>
> My gut feeling is that we should do it the way the Bazaar UI does it,
> I don't have any evidence that there's anything wrong with the current
> code, which Bazaar seems to but have, but for different purposes which
> are hard to explain. I would rather avoid surprises.
Er,which Bazaar seems to use, but for different purposes...
-- 
Felipe Contreras
Junio C Hamano· Apr 25, 2013, 22:32 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-bzr: use proper push method

Felipe Contreras <felipe.contreras@gmail.com> writes:
> My gut feeling is that we should do it the way the Bazaar UI does it,
> I don't have any evidence that there's anything wrong with the current
> code, which Bazaar seems to but have, but for different purposes which
> are hard to explain. I would rather avoid surprises.
That is actually a very fine description for this change ;-).
commit 473b7aac9f542dc22cfff0c264e96d8dbbd9d895
Author: Felipe Contreras <felipe.contreras@gmail.com>
Date:   Thu Apr 25 06:25:37 2013 -0500
    remote-bzr: use proper push method
    
    Do not just randomly synchronize the revisions with no checks at
    all.
    
    I don't have any evidence that there's anything wrong with the
    current code, which Bazaar seems to use, but for different purposes.
    Let's use the logic Bazaar UI uses to avoid surprises.
    
    Also, add a non-ff check.
    
    Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

← back to recent threads