'bzr push' copies the entire repository if there is a BzrDir but not a Branch

As near as I can tell, it is trying to copy my entire repository, rather than *just* copying the ancestry of the branch. Poking some more to figure out why.

So, I tried pushing again, this time to a *different* directory, and I got:
fetching revisions: 5
streaming inventories: 5
streaming chk, excluding: 2
streamed 19 chk pages
found text keys: 12

Which is what I expected to get the first time. Now, I've had some troubles with these pushes, so the remote 'branch' may not be in a perfect state.

Specifically, it appears that there *is* a .bzr directory, and it has a .bzr/branch-format but it does *not* have a .bzr/branch directory.

So perhaps this can be downgraded to:
  bzr push to existing bzrdir (without branch) copies entire repository to remote

The ensure real should only happen if the server is old: what bzr
version is on babune?

-Rob

Looking at the code, it appears that if the bzrdir exists but there is no branch, but there *is* a repository, we end up in this code path:

        if br_to is None:
            # We have a repository but no branch, copy the revisions, and then
            # create a branch.
            repository_to.fetch(source.repository, revision_id=revision_id)
            br_to = source.clone(self, revision_id=revision_id)
            if source.get_push_location() is None or remember:
                source.set_push_location(br_to.base)
            push_result.stacked_on = None
            push_result.branch_push_result = None
            push_result.old_revno = None
            push_result.old_revid = _mod_revision.NULL_REVISION
            push_result.target_branch = br_to
            push_result.master_branch = None
            push_result.workingtree_updated = False

If we *do* have a branch we use:
                push_result.branch_push_result = source.push(br_to,
                    overwrite, stop_revision=revision_id)

This ends up going through InterBranch.push() which winds around again through '_basic_push' and seems to end up in InterBranch.update_revisions (only now having swapped source and target, so it is target.update_revisions(source)), which seems to indicate that we call
 remote_branch.fetch(local_branch, stop_revision)

(note that I'm quite horribly confused by the double indirection through InterBranch, and the fact that the default .update_revisions does this:
    def update_revisions(self, other, stop_revision=None, overwrite=False,
                         graph=None):
...
        return InterBranch.get(other, self).update_revisions(stop_revision,
            overwrite, graph)

^- Which as near as I can tell makes 'other' the self.source attribute for InterBranch, which looking at update_revisions hints that it would then be fetching "remote => local" which certainly doesn't make any sense.

And if we don't have a bzrdir we use:
            br_to = br_from.create_clone_on_transport(to_transport,
                revision_id=revision_id, stacked_on=stacked_on,
                create_prefix=create_prefix, use_existing_dir=use_existing_dir)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> The ensure real should only happen if the server is old: what bzr
> version is on babune?
>
> -Rob
>

2.0.1 as near as I can tell.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrrYZIACgkQJdeBCYSNAAOyQQCfbcdgKAlXSOv/G3CuXnsLLh8e
ib0AoJAR9krd/F5BjBhLkX+6DqOqQROb
=q497
-----END PGP SIGNATURE-----

Downgrading from critical because it only triggers in somewhat exotic circumstances. (canceling a push into a shared repo could create the target bzrdir but not the target branch, pushing again will trigger this sort of bug.)
I'd still probably like to see the fix in the 2.0 series and preferably for 2.0.2

I think I understand it. I haven't probed deep enough to be sure but this line:

if br_to is None:
            # We have a repository but no branch, copy the revisions, and then
            # create a branch.
            repository_to.fetch(source.repository, revision_id=revision_id)

Looks like it is doing the right thing, but I think it is subtly wrong. Namely, 'revision_id' is a string only if the user specifies "bzr push -r XXXX", otherwise it is just "None".

This handled in the other code paths with a:

  if stop_revision is None:
    stop_revision = the_branch.last_revision()

But that isn't being done in this code path.

I've taken the milestone off because it doesn't sound like this will be done in 2.1, and certainly in a release that's already out.

Accepted bzr into natty-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Hello John, or anyone else affected,

Accepted bzr into natty-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Verified by running the bzr testsuite from the package in a clean natty install.

This bug was fixed in the package bzr - 2.3.4-0ubuntu1

---------------
bzr (2.3.4-0ubuntu1) natty-proposed; urgency=low

  * New upstream release.
   + Fix bzr version number in deprecation warnings. LP: #794960
   + Prevent write attemps on remote branch during "bzr up". LP: #786980
   + Fix conflict handling when two trees involved in a merge have different
     root ids. LP: #805809

bzr (2.3.3-0ubuntu1) natty-proposed; urgency=low

  * New upstream release.
   + Fixes deprecation warning on newer versions of Python. LP: #760435
   + Stops 'bzr push' from copying entire repository if a .bzr directory is
     present without a branch. LP: #465517
   + Fixes undefined local variable error when waiting for lock. LP: #733136
   + Fixes lock contention issues pushing to a bound branch. LP: #733350
   + Transfers less data creating a new stacked branch. LP: #737234
   + Several fixes to the test suite, making it more robust. LP: #654733,
      LP: #751824
   + 'bzr merge --pull --preview' actually shows a preview rather than
     actually merging. LP: #760152
   + bzr smart server now supports UTF-8 user names. LP: #659763
   + user identity can now be set based on username and /etc/mailname, not
     requiring it to be set manually. LP: #616878
   + stacking is now fully transitive. LP: #715000
   + makes in-terminal crash report of plugins much shorter. LP: #716389
 -- Jelmer Vernooij <email address hidden> Thu, 14 Jul 2011 21:12:58 +0200