[merge] Fix push output (bug 49742)

[Michael Ellerman]

On 6/17/06, John Arbash Meinel <john at arbash-meinel.com> wrote:
> https://launchpad.net/bugs/49742
>
> After the encoding changes, I missed a final newline as part of writing
> out the saved push location. The attached patch changes the test suite
> so that it actually checks the output.
>
> Also, this changes 'push' so that it only remembers the push location
> after connecting to the branch (but before it actually pushes
> revisions). That way if you fail to push because of a typo, it isn't
> remembered. But if you fail because you are out of date, you can fix
> that and not have to type it again.
>
> That was the basic conclusion from the bug.
>
> Seeking a +1.
>
> John
> =:->
>
>
> # Bazaar revision bundle v0.7
> #
> # message:
> #   NEWS, this fixes bug #49742
> # committer: John Arbash Meinel <john at arbash-meinel.com>
> # date: Fri 2006-06-16 19:51:49.000586033 -0500
>
> === modified file NEWS
> --- NEWS
> +++ NEWS
> @@ -70,6 +70,9 @@
>      * Fix unnecessary requirement of sign-my-commits that it be run from
>        a working directory.  (Martin Pool, Robert Collins)
>
> +    * 'bzr push location' will only remember the push location if it succeeds
> +      in connecting to the remote location. (#49742, John Arbash Meinel)
> +
>    INTERNALS:
>
>      * Combine the ignore rules into a single regex rather than looping over
>
> === modified file bzrlib/builtins.py // last-changed:john at arbash-meinel.com-200
> ... 60617004934-7eb765919f2a4de5
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -530,13 +530,11 @@
>              else:
>                  display_url = urlutils.unescape_for_display(stored_loc,
>                          self.outf.encoding)
> -                self.outf.write("Using saved location: %s" % display_url)
> +                self.outf.write("Using saved location: %s\n" % display_url)
>                  location = stored_loc
>
>          transport = get_transport(location)
>          location_url = transport.base
> -        if br_from.get_push_location() is None or remember:
> -            br_from.set_push_location(location_url)
>
>          old_rh = []
>          try:
> @@ -572,7 +570,14 @@
>                  revision_id=br_from.last_revision())
>              br_to = dir_to.open_branch()
>              count = len(br_to.revision_history())
> +            # We successfully created the target, remember it
> +            if br_from.get_push_location() is None or remember:
> +                br_from.set_push_location(br_to.base)
>          else:
> +            # We were able to connect to the remote location, so remember it
> +            # we don't need to successfully push because of possible divergence.
> +            if br_from.get_push_location() is None or remember:
> +                br_from.set_push_location(br_to.base)
>              old_rh = br_to.revision_history()
>              try:
>                  try:

It's a pity this has to be there twice, but I can't see how to easily
fix it. That whole function needs to be refactored mercilessly.

It'd probably be good to update the help to mention that --remember
only works if the operation completes successfully, just to set
people's expectations correctly. Looks ok to me otherwise.

cheers