want "reconfigure --unstacked" or --stacked-on

This branch <https://code.edge.launchpad.net/~mbp/bzr/391411-reconfigure-stacked> does add the requested options, but in an ad hoc test against Launchpad it fails with LockNotHeld: Lock not held: RemoteRepository(bzr+ssh://bazaar.launchpad.net/~bzr-core/bzr/devnotes/.bzr/)

29.145 Traceback (most recent call last):
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 835, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/builtins.py", line 5288, in run
    branch.unlock()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/remote.py", line 2250, in unlock
    self.repository.unlock()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/remote.py", line 1028, in unlock
    raise errors.LockNotHeld(self)
LockNotHeld: Lock not held: RemoteRepository(bzr+ssh://bazaar.launchpad.net/~bzr-core/bzr/devnotes/.bzr/)

Some people have commented that perhaps these should be in separate commands from reconfigure, because it's not the same as the other types of reconfiguration that command does. It is similar at one level in being a type of setting that moves data around, rather than just changing a setting.

The error is probably because when the branch was opened it had a stacked-on branch, but by the time it's unlocked that's no longer true, and it's not quite matching up.

This also needs a change in the user-guide.

In interactive testing this fails even locally with a traceback like this:

mbp@grace% ./bzr reconfigure --unstacked /tmp/foo
/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/lockable_files.py:61: UserWarning: 'LockableFiles(<bzrlib.transport.local.LocalTransport url=file:///home/mbp/bzr/.bzr/repository/>)' was gc'd while locked
  warnings.warn("%r was gc'd while locked" % self.repr)
bzr: ERROR: bzrlib.errors.NoSuchRevision: KnitPackRepository('file:///home/mbp/bzr/.bzr/repository/') has no revision <email address hidden>

Traceback (most recent call last):
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 835, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/builtins.py", line 5282, in run
    branch.set_stacked_on_url(None)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/branch.py", line 686, in set_stacked_on_url
    find_ghosts=True)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 1544, in fetch
    find_ghosts=find_ghosts, fetch_spec=fetch_spec)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 3188, in fetch
    pb=pb, find_ghosts=find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 79, in __init__
    self.__fetch()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 101, in __fetch
    search = self._revids_to_fetch()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 170, in _revids_to_fetch
    find_ghosts=self.find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 1377, in search_missing_revision_ids
    revision_id, find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 3258, in search_missing_revision_ids
    source_ids = self.source.get_ancestry(revision_id)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 2405, in get_ancestry
    raise errors.NoSuchRevision(self, revision_id)
NoSuchRevision: KnitPackRepository('file:///home/mbp/bzr/.bzr/repository/') has no revision <email address hidden>

The NoSuchRevision can occur if there's a tag pointing to a revision not present in either of the revisions originally. We normally tolerate this, but here we're trying to explicitly fetch it and we don't ignore the error.

I'm not sure that we should ignore it.

But we should make it consistent with what bzr branch does.

bzr branch only brings across tagged revisions present in the branch's history closure. Arguably it should be otherwise but that's a different issue.

With that fixed, local reconfigure works well in the current branch, but the issue across the network still exists, probably because of different lock lifetimes.

The failure seen over hpss in ad hoc testing seems to be because it's trying to fetch the tip of the stacked branch from the fallback repository. It shouldn't be even trying if that revision is already in the destination, it should be just bringing across its closure.

This perhaps needs to be tested at the set_stacked_on_url api level.

I improved my test and it now fails locally in the same way. The problem is that it's trying to fetch the branch tip from the fallback repository, but it typically doesn't actually contain the tip of the stacked branch.

Traceback (most recent call last):
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/tests/blackbox/test_reconfigure.py", line 213, in test_reconfigure_stacking
    out, err = self.run_bzr('reconfigure --unstacked b2')
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/tests/__init__.py", line 1651, in run_bzr
    working_dir=working_dir,
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/tests/__init__.py", line 1565, in _run_bzr_autosplit
    encoding=encoding, stdin=stdin, working_dir=working_dir,
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/tests/__init__.py", line 1595, in _run_bzr_core
    args)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/tests/__init__.py", line 1885, in apply_redirected
    return a_callable(*args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 1131, in run_bzr_catch_user_errors
    return run_bzr(argv)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/builtins.py", line 5282, in run
    branch.set_stacked_on_url(None)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/branch.py", line 673, in set_stacked_on_url
    self._unstack()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/branch.py", line 713, in _unstack
    find_ghosts=True)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 1550, in fetch
    find_ghosts=find_ghosts, fetch_spec=fetch_spec)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 192, in write_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 3194, in fetch
    pb=pb, find_ghosts=find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 79, in __init__
    self.__fetch()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 101, in __fetch
    search = self._revids_to_fetch()
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/fetch.py", line 170, in _revids_to_fetch
    find_ghosts=self.find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 1383, in search_missing_revision_ids
    revision_id, find_ghosts)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/repository.py", line 3264, in search_missing_revision_ids
    source_ids = self.source.get_ancestry(revision_id)
  File "/home/mbp/bzr/391411-reconfigure-stacked/bzrlib/decorators.py", line 138, in read_locked
    result = unbound(self, *args, **kwargs)
  File "/home/mbp/b...

Talking this over with Robert:

1 - Rather than copying all history when fetching, we could fetch the tip from a Repository configured with the fallback to a Repository with no fallbacks, asking for ghosts to be filled.

2 - We may need to specially check that fetching in this case brings across all the needed CHKMap inventory pages; that's probably not covered by my current test because the inventory is too small. (That could be split to a separate bug.)

One other issue: streaming from the fallback repository into the newly unstacked repository possibly requires opening a second transport connection where previously we had only one; if this isn't done we'll probably see a smart transport "connection in use" error.

> 2 - We may need to specially check that fetching in this case brings across all the needed CHKMap inventory pages; that's probably not covered by my current test because the inventory is too small. (That could be split to a separate bug.)

bug 397286 for this.

Running the current branch code against launchpad doesn't actually copy the right content when unstacking because Remote fetch shortcuts when seeing they're apparently the same repository.

The per-branch test_unstack_fetches fails, but incorrectly, because it wasn't actually configuring stacking in a Remote branch.

With that fixed, and the fetch rules adjusted, the tests now pass in general, but fail remotely, I think because it needs two transports and only has one.

I think this is now fixed in the attached branch, and working for nontrivial branches on Launchpad.

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

Martin Pool wrote:
> Some people have commented that perhaps these should be in separate
> commands from reconfigure, because it's not the same as the other types
> of reconfiguration that command does.

I think that --unstacked and --stacked-on are very similar to
- --standalone and --use-shared.

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

iEYEARECAAYFAkpf/pAACgkQ0F+nu1YWqI0EUwCfehEcLmusnheYD7ePM50xJoKr
0sYAn3N3WR9LAqXWEODgQoTC+EJTvyYp
=I6gQ
-----END PGP SIGNATURE-----