[RFC][Merge] repository support
John A Meinel
john at arbash-meinel.com
Wed Feb 15 17:12:19 GMT 2006
Robert Collins wrote:
> This is the data level support for repositories:
> * finding a repository from a branch
> * using a repository when making a branch if desired
> (bzrdir.BzrDir.create_branch_convenience(URL, force_new_repo=False,
> force_new_tree=None))
>
>
> It does not have any UI yet, I've moved onto 'upgrade' support as part
> of bzrdir phase 3. If this gets a +1 I'll merge it and hopefully someone
> can do a UI :). Or I'll come back and do a minimal UI to get us going.
>
> If you want to make a repository and play:
> import bzrlib.bzrdir
> repo = bzrdir.BzrDir.create_repository('.', shared=True)
> shared_branch = bzrdir.BzrDir.create_branch_convenience('subdir')
>
> To toggle the creation of new trees inside a repo (i.e. for repos that
> you are publishing):
> repo.set_make_new_trees(True|False)
>
> To copy an existing branch into a repo so that it uses the shared
> storage:
> dir = bzrdir.BzrDir.open('old_branch')
> dir.clone('path/in/repo')
>
> After that ui commands should just work.
>
> As you can see below, this is primarily corner case testing of new code
> paths. I'm asking for a review on this so that we dont get another
> mega-branch accumulating :)
>
review in progress....
...
>
> - def clone(self, url, revision_id=None, basis=None):
> + def clone(self, url, revision_id=None, basis=None, force_new_repo=False):
> """Clone this bzrdir and its contents to url verbatim.
>
> If urls last component does not exist, it will be created.
>
> if revision_id is not None, then the clone operation may tune
> itself to download less data.
> + :param force_new_repo: Do not use a shared repository for the target
> + even if one is available.
> """
I want to make sure I understand the objects here...
'local_repo' is the repository local to the branch we are copying from.
'basis_repo' is something given by --basis
'result_repo' is the target repository.
> self._make_tail(url)
> + basis_repo, basis_branch, basis_tree = self._get_basis_components(basis)
> result = self._format.initialize(url)
> - basis_repo, basis_branch, basis_tree = self._get_basis_components(basis)
> - try:
> - self.open_repository().clone(result, revision_id=revision_id, basis=basis_repo)
> + try:
> + local_repo = self.find_repository()
> except errors.NoRepositoryPresent:
> - pass
> + local_repo = None
> + if local_repo:
> + # may need to copy content in
> + if force_new_repo:
> + local_repo.clone(result, revision_id=revision_id, basis=basis_repo)
> + else:
> + try:
> + result_repo = result.find_repository()
> + # fetch content this dir needs.
> + if basis_repo:
> + # XXX FIXME RBC 20060214 need tests for this when the basis
> + # is incomplete
> + result_repo.fetch(basis_repo, revision_id=revision_id)
> + result_repo.fetch(local_repo, revision_id=revision_id)
> + except errors.NoRepositoryPresent:
> + # needed to make one anyway.
> + local_repo.clone(result, revision_id=revision_id, basis=basis_repo)
> + # 1 if there is a branch present
> + # make sure its content is available in the target repository
> + # clone it.
Wouldn't this be cleaner as (paraphrased):
if local_repo:
if force_new_repo:
result_repo = None
else:
try:
result_repo = result.find_repository()
except
result_repo = None
if not result_repo:
result_repo.fetch(...)
else:
local_repo.clone(result, revision_id=revision_id, basis=basis_repo)
It just seems less nested, with less redundancy.
It also seems like code would be cleaner if "find_repository" returned
None if it didn't find any, instead of throwing an exception. Then you
could just do:
if force_new_repo:
result_repo = None
else:
result_repo = result.find_repository()
if result_repo is None:
...
You have this same pattern later where you have:
if force:
do X
else:
try:
find
do Y
except
do X
> try:
> self.open_branch().clone(result, revision_id=revision_id)
> except errors.NotBranchError:
> @@ -140,26 +161,73 @@
> raise NotImplementedError(self.create_branch)
>
> @staticmethod
> - def create_branch_and_repo(base):
> + def create_branch_and_repo(base, force_new_repo=False):
> """Create a new BzrDir, Branch and Repository at the url 'base'.
>
> This will use the current default BzrDirFormat, and use whatever
> repository format that that uses via bzrdir.create_branch and
> - create_repository.
> + create_repository. If a shared repository is available that is used
> + preferentially.
>
> The created Branch object is returned.
> +
> + :param base: The URL to create the branch at.
> + :param force_new_repo: If True a new repository is always created.
> """
> bzrdir = BzrDir.create(base)
> - bzrdir.create_repository()
> + if force_new_repo:
> + bzrdir.create_repository()
> + try:
> + repo = bzrdir.find_repository()
> + except errors.NoRepositoryPresent:
> + bzrdir.create_repository()
> return bzrdir.create_branch()
>
> @staticmethod
> - def create_repository(base):
> + def create_branch_convenience(base, force_new_repo=False, force_new_tree=None):
> + """Create a new BzrDir, Branch and Repository at the url 'base'.
> +
> + This is a convenience function - it will use an existing repository
> + if possible, can be told explicitly whether to create a working tree or
> + nor.
not
> +
> + This will use the current default BzrDirFormat, and use whatever
> + repository format that that uses via bzrdir.create_branch and
> + create_repository. If a shared repository is available that is used
> + preferentially. Whatever repository is used, its tree creation policy
> + is followed.
> +
> + The created Branch object is returned.
> + If a working tree cannot be made due to base not being a file:// url,
> + no error is raised.
> +
> + :param base: The URL to create the branch at.
> + :param force_new_repo: If True a new repository is always created.
> + :param force_new_tree: If True or False force creation of a tree or
> + prevent such creation respectively.
> + """
> + bzrdir = BzrDir.create(base)
> + if force_new_repo:
> + bzrdir.create_repository()
Isn't this incorrect? You have create, and then find, and then create
again. Shouldn't it be:
if force:
repo = create
else:
try:
repo = find
except:
repo = create
> + try:
> + repo = bzrdir.find_repository()
> + except errors.NoRepositoryPresent:
> + repo = bzrdir.create_repository()
> + result = bzrdir.create_branch()
> + if force_new_tree or (repo.make_working_trees() and
> + force_new_tree is None):
> + bzrdir.create_workingtree()
> + return result
> +
> + @staticmethod
> + def create_repository(base, shared=False):
> """Create a new BzrDir and Repository at the url 'base'.
>
> This will use the current default BzrDirFormat, and use whatever
> repository format that that uses for bzrdirformat.create_repository.
>
> + ;param shared: Create a shared repository rather than a standalone
> + repository.
> The Repository object is returned.
>
> This must be overridden as an instance method in child classes, where
> @@ -184,7 +252,8 @@
> t = get_transport(safe_unicode(base))
> if not isinstance(t, LocalTransport):
> raise errors.NotLocalUrl(base)
> - bzrdir = BzrDir.create_branch_and_repo(safe_unicode(base)).bzrdir
> + bzrdir = BzrDir.create_branch_and_repo(safe_unicode(base),
> + force_new_repo=True).bzrdir
> return bzrdir.create_workingtree()
>
> def create_workingtree(self, revision_id=None):
> @@ -193,6 +262,36 @@
> revision_id: create it as of this revision id.
> """
> raise NotImplementedError(self.create_workingtree)
> +
> + def find_repository(self):
> + """Find the repository that should be used for a_bzrdir.
> +
> + This does not require a branch as we use it to find the repo for
> + new branches as well as to hook existing branches up to their
> + repository.
> + """
> + try:
> + return self.open_repository()
> + except errors.NoRepositoryPresent:
> + pass
> + next_transport = self.root_transport.clone('..')
> + while True:
> + try:
> + found_bzrdir = BzrDir.open_containing_transport(
> + next_transport)[0]
> + except errors.NotBranchError:
> + raise errors.NoRepositoryPresent(self)
> + try:
> + repository = found_bzrdir.open_repository()
> + except errors.NoRepositoryPresent:
> + next_transport = found_bzrdir.root_transport.clone('..')
> + continue
> + if ((found_bzrdir.root_transport.base ==
> + self.root_transport.base) or repository.is_shared()):
> + return repository
> + else:
> + raise errors.NoRepositoryPresent(self)
> + raise errors.NoRepositoryPresent(self)
>
> def get_branch_transport(self, branch_format):
> """Get the transport for use by branch format in this BzrDir.
> @@ -283,7 +382,16 @@
> def open_containing(url):
> """Open an existing branch which contains url.
>
> - This probes for a branch at url, and searches upwards from there.
> + :param url: url to search from.
> + See open_containing_transport for more detail.
> + """
> + return BzrDir.open_containing_transport(get_transport(url))
> +
> + @staticmethod
> + def open_containing_transport(a_transport):
> + """Open an existing branch which contains a_transport.base
> +
> + This probes for a branch at a_transport, and searches upwards from there.
can you name this "open_containing_from_transport"? Otherwise it sounds
like you are returning a transport not a branch (or bzr dir).
...
...
> + if force_new_repo:
> + result_repo = None
> + else:
> + try:
> + result_repo = result.find_repository()
> + except errors.NoRepositoryPresent:
> + result_repo = None
Again we have this pattern. Mabye it would be better to factor it into a
function:
def find_or_create_repository(force=False):
> + if source_repository is None and result_repo is not None:
> + pass
> + elif source_repository is None and result_repo is None:
> + # no repo available, make a new one
> + result.create_repository()
> + elif source_repository is not None and result_repo is None:
> + # have soure, and want to make a new target repo
> source_repository.clone(result,
> revision_id=revision_id,
> basis=basis_repo)
> else:
> - # no repo available, make a new one
> - result.create_repository()
> + # fetch needed content into target.
> + if basis_repo:
> + # XXX FIXME RBC 20060214 need tests for this when the basis
> + # is incomplete
> + result_repo.fetch(basis_repo, revision_id=revision_id)
> + result_repo.fetch(source_repository, revision_id=revision_id)
> if source_branch is not None:
> source_branch.sprout(result, revision_id=revision_id)
> else:
I didn't go through all of the tests very thoroughly. But generally
things look good. My only comment is the repeating paradigm of
find_or_create. (+1 from me)
John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060215/f5077e65/attachment.pgp
More information about the bazaar
mailing list