Merge lp:~luoyonggang/bzr-svn/perfect-layout into lp:bzr-svn/1.1

[Yonggang Luo]

There is indent fixup.

Mainly focused on get perfect working with bzr-svn's layout!

[Jelmer Vernooij (jelmer)]

Hi,

On 14/06/11 12:57, Yonggang Luo wrote:
> Yonggang Luo has proposed merging lp:~luoyonggang/bzr-svn/perfect-layout into lp:bzr-svn.
>
> Requested reviews:
> Jelmer Vernooij (jelmer)
>
> For more details, see:
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
>
> There is indent fixup.
>
> Mainly focused on get perfect working with bzr-svn's layout!
In general, please submit merge proposals for specific changes. That
makes it possible to merge those specific changes without having to
reject the entire MP (like this one).

You seem to be deleting a few things that are there for good reasons.
Please keep the following around:

* The --layout option in svn-import - removing it breaks existing users
who rely on it, or might have scripts that rely on it
* Guessing of layouts in "bzr svn-import". It's there for a reason - to
prevent users from shooting themselves in the foot.

I don't see why prefix isn't relevant for PerfectLayout; shouldn't users
be able to fetch just a part of the repository?

Please don't call your custom layout "perfect". It may be perfect for
you, it's not a good fit for everybody and that name doesn't really
explain how it works. What about "copytracking" ?

As we discussed earlier, please don't register your layout in the layout
registry as it needs to have the LogWalker passed in. Instead, please
add a configuration option for it, look for that option in
Repository.get_layout() and return PerfectLayout(self._log) from there.

Cheers,

Jelmer

[Jelmer Vernooij (jelmer)]

review: Disapprove

[Yonggang Luo (luoyonggang)]

2011/6/14 Jelmer Vernooij <email address hidden>

> Hi,
>
> On 14/06/11 12:57, Yonggang Luo wrote:
> > Yonggang Luo has proposed merging lp:~luoyonggang/bzr-svn/perfect-layout
> into lp:bzr-svn.
> >
> > Requested reviews:
> > Jelmer Vernooij (jelmer)
> >
> > For more details, see:
> >
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
> >
> > There is indent fixup.
> >
> > Mainly focused on get perfect working with bzr-svn's layout!
> In general, please submit merge proposals for specific changes. That
> makes it possible to merge those specific changes without having to
> reject the entire MP (like this one).
>
I don't know how to do that.

>
> You seem to be deleting a few things that are there for good reasons.
> Please keep the following around:
>
> * The --layout option in svn-import - removing it breaks existing users
> who rely on it, or might have scripts that rely on it
> * Guessing of layouts in "bzr svn-import". It's there for a reason - to
> prevent users from shooting themselves in the foot.
>
Did not affecting of bzr svn-import . I've tested.
Why I ignore prefix? I am not ignore of it, just because of PerfectLayout
need using of it
PerfectLayout need manage of prefix by its self.

>
> I don't see why prefix isn't relevant for PerfectLayout; shouldn't users
> be able to fetch just a part of the repository?
>
> Please don't call your custom layout "perfect". It may be perfect for
> you, it's not a good fit for everybody and that name doesn't really
> explain how it works. What about "copytracking" ?
>
Err.. you should tell me earlier, such as the last discussion!

> As we discussed earlier, please don't register your layout in the layout
>
NO。。。it's not working, when I was trying, all kinds of problem is appeared..

> registry as it needs to have the LogWalker passed in. Instead, please
> add a configuration option for it, look for that option in
> Repository.get_layout() and return PerfectLayout(self._log) from there.
>
> Cheers,
>
> Jelmer
>
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
> You are the owner of lp:~luoyonggang/bzr-svn/perfect-layout.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Jelmer Vernooij (jelmer)]

On 14/06/11 13:43, Yonggang Luo wrote:
> 2011/6/14 Jelmer Vernooij <email address hidden>
>
>
>> In general, please submit merge proposals for specific changes. That
>> makes it possible to merge those specific changes without having to
>> reject the entire MP (like this one).
>>
> I don't know how to do that.
I mean create different branches and propose them for merging against
lp:bzr separately.
>> You seem to be deleting a few things that are there for good reasons.
>> Please keep the following around:
>>
>> * The --layout option in svn-import - removing it breaks existing users
>> who rely on it, or might have scripts that rely on it
>> * Guessing of layouts in "bzr svn-import". It's there for a reason - to
>> prevent users from shooting themselves in the foot.
>>
> Did not affecting of bzr svn-import . I've tested.
> Why I ignore prefix? I am not ignore of it, just because of PerfectLayout
> need using of it
> PerfectLayout need manage of prefix by its self.
prefix is used to restrict which paths are imported. If I specify:

bzr svn-import svn://svn.llvm.org/llvm-gcc and svn://svn.llvm.org/ is
the repository root I expect only the llvm-gcc part of the repository to
be imported, and not any of the other projects. Ignoring prefix means
all projects in the repository will be imported.

>> I don't see why prefix isn't relevant for PerfectLayout; shouldn't users
>> be able to fetch just a part of the repository?
>>
>> Please don't call your custom layout "perfect". It may be perfect for
>> you, it's not a good fit for everybody and that name doesn't really
>> explain how it works. What about "copytracking" ?
>>
> Err.. you should tell me earlier, such as the last discussion!
? We never discussed the name of your layout before as far as I
remember, and we never looked at any code.

>> As we discussed earlier, please don't register your layout in the layout
>>
> NO。。。it's not working, when I was trying, all kinds of problem is appeared..
Can you be more specific, what exactly didn't work?

Cheers,

Jelmer

[Yonggang Luo (luoyonggang)]

2011/6/14 Jelmer Vernooij <email address hidden>

> On 14/06/11 13:43, Yonggang Luo wrote:
> > 2011/6/14 Jelmer Vernooij <email address hidden>
> >
> >
> >> In general, please submit merge proposals for specific changes. That
> >> makes it possible to merge those specific changes without having to
> >> reject the entire MP (like this one).
> >>
> > I don't know how to do that.
> I mean create different branches and propose them for merging against
> lp:bzr separately.
>

That's too much complicated. It's just leading the development progress too
depression.
Is there any other way?

>> You seem to be deleting a few things that are there for good reasons.
> >> Please keep the following around:
> >>
> >> * The --layout option in svn-import - removing it breaks existing users
> >> who rely on it, or might have scripts that rely on it
> >> * Guessing of layouts in "bzr svn-import". It's there for a reason - to
> >> prevent users from shooting themselves in the foot.
> >>
> > Did not affecting of bzr svn-import . I've tested.
> > Why I ignore prefix? I am not ignore of it, just because of PerfectLayout
> > need using of it
> > PerfectLayout need manage of prefix by its self.
> prefix is used to restrict which paths are imported. If I specify:
>
> bzr svn-import svn://svn.llvm.org/llvm-gcc and svn://svn.llvm.org/ is
> the repository root I expect only the llvm-gcc part of the repository to
> be imported, and not any of the other projects. Ignoring prefix means
> all projects in the repository will be imported.
>
>
> >> I don't see why prefix isn't relevant for PerfectLayout; shouldn't users
> >> be able to fetch just a part of the repository?
> >>
> >> Please don't call your custom layout "perfect". It may be perfect for
> >> you, it's not a good fit for everybody and that name doesn't really
> >> explain how it works. What about "copytracking" ?
> >>
> > Err.. you should tell me earlier, such as the last discussion!
> ? We never discussed the name of your layout before as far as I
> remember, and we never looked at any code.
>
> You mentioned PerfectLayout at freenode, so I was think you were accepted
of it.

> >> As we discussed earlier, please don't register your layout in the
> layout
> >>
> > NO。。。it's not working, when I was trying, all kinds of problem is
> appeared..
> Can you be more specific, what exactly didn't work?
>
> The code in commands.py that I removed.

> Cheers,
>
> Jelmer
>
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
> You are the owner of lp:~luoyonggang/bzr-svn/perfect-layout.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[lp:~luoyonggang/bzr-svn/perfect-layout]

3734. By Yonggang Luo on 2011-06-14

Add it first!

[Jelmer Vernooij (jelmer)]

On 14/06/11 14:07, Yonggang Luo wrote:
> 2011/6/14 Jelmer Vernooij <email address hidden>
>
>> On 14/06/11 13:43, Yonggang Luo wrote:
>>> 2011/6/14 Jelmer Vernooij <email address hidden>
>>>
>>>
>>>> In general, please submit merge proposals for specific changes. That
>>>> makes it possible to merge those specific changes without having to
>>>> reject the entire MP (like this one).
>>>>
>>> I don't know how to do that.
>> I mean create different branches and propose them for merging against
>> lp:bzr separately.
>>
> That's too much complicated. It's just leading the development progress too
> depression.
> Is there any other way?
It's fine with me if you want to submit bigger branches, I was just
suggesting smaller branches since they can be landed independently.
>>>> I don't see why prefix isn't relevant for PerfectLayout; shouldn't users
>>>> be able to fetch just a part of the repository?
>>>>
>>>> Please don't call your custom layout "perfect". It may be perfect for
>>>> you, it's not a good fit for everybody and that name doesn't really
>>>> explain how it works. What about "copytracking" ?
>>>>
>>> Err.. you should tell me earlier, such as the last discussion!
>> ? We never discussed the name of your layout before as far as I
>> remember, and we never looked at any code.

> You mentioned PerfectLayout at freenode, so I was think you were accepted
> of it.
I mean we didn't explicitly discuss the name - I don't like it for the
reasons specified earlier.

Cheers,

Jelmer

[Yonggang Luo (luoyonggang)]

Another problem, because of repo already have an function named with
get_layout
Why another get_layout resident at command.py?

I think they must be at the same place. It's easier to management.

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Yonggang Luo (luoyonggang)]

Why this error arising, it's looks weird.. I have no idea on this, on
another computer, it's works fine.
ssh -v luoyonggang@<email address hidden>
OpenSSH_5.8p1, OpenSSL 0.9.8r 8 Feb 2011
debug1: Reading configuration data /cygdrive/d/CI/docs/home/lyg/.ssh/config
debug1: Applying options for bazaar.launchpad.net
debug1: Connecting to bazaar.launchpad.net [91.189.90.11] port 22.
debug1: Connection established.
debug1: permanently_set_uid: 0/0
debug1: identity file /cygdrive/d/CI/docs/home/lyg/.ssh/id_rsa type 1
debug1: identity file /cygdrive/d/CI/docs/home/lyg/.ssh/id_rsa-cert type -1
debug1: Remote protocol version 2.0, remote software version Twisted
debug1: no match: Twisted
debug1: Enabling compatibility mode for protocol 2.0
      4 [main] ssh 5584 exception::handle: Exception:
STATUS_ACCESS_VIOLATION
   3075 [main] ssh 5584 open_stackdumpfile: Dumping stack trace to
ssh.exe.stack
dump

[Jelmer Vernooij (jelmer)]

On 14/06/11 15:58, Yonggang Luo wrote:
> Another problem, because of repo already have an function named with
> get_layout
> Why another get_layout resident at command.py?
I don't think that's a problem, they have a different scope. get_layout
in commands.py could be renamed to get_layout_by_name or something like
that, but I don't see any reason to do so.
>
> I think they must be at the same place. It's easier to management.
They do different things; one is a UI layer helper for getting a layout by name. The other is for finding the layout to use for a repository.

Cheers,

Jelmer

[Jelmer Vernooij (jelmer)]

On 14/06/11 15:59, Yonggang Luo wrote:
> Why this error arising, it's looks weird.. I have no idea on this, on
> another computer, it's works fine.
> ssh -v luoyonggang@<email address hidden>
> OpenSSH_5.8p1, OpenSSL 0.9.8r 8 Feb 2011
> debug1: Reading configuration data /cygdrive/d/CI/docs/home/lyg/.ssh/config
> debug1: Applying options for bazaar.launchpad.net
> debug1: Connecting to bazaar.launchpad.net [91.189.90.11] port 22.
> debug1: Connection established.
> debug1: permanently_set_uid: 0/0
> debug1: identity file /cygdrive/d/CI/docs/home/lyg/.ssh/id_rsa type 1
> debug1: identity file /cygdrive/d/CI/docs/home/lyg/.ssh/id_rsa-cert type -1
> debug1: Remote protocol version 2.0, remote software version Twisted
> debug1: no match: Twisted
> debug1: Enabling compatibility mode for protocol 2.0
> 4 [main] ssh 5584 exception::handle: Exception:
> STATUS_ACCESS_VIOLATION
> 3075 [main] ssh 5584 open_stackdumpfile: Dumping stack trace to
> ssh.exe.stack
> dump
Looks like a segfaulting ssh client; the cygwin bug tracker is probably
a more appropriate place for that.

Cheers,

jelmer

[Yonggang Luo (luoyonggang)]

>
>
> > debug1: Enabling compatibility mode for protocol 2.0
> > 4 [main] ssh 5584 exception::handle: Exception:
> > STATUS_ACCESS_VIOLATION
> > 3075 [main] ssh 5584 open_stackdumpfile: Dumping stack trace to
> > ssh.exe.stack
> > dump
> Looks like a segfaulting ssh client; the cygwin bug tracker is probably
> a more appropriate place for that.
>
> Thanks, by using of putty, its working, ssh.exe still not working, I found
that problem appeared before under cygwin mailling list. but they didn't
resolved at last:)

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Yonggang Luo (luoyonggang)]

2011/6/14 Jelmer Vernooij <email address hidden>

> On 14/06/11 15:58, Yonggang Luo wrote:
> > Another problem, because of repo already have an function named with
> > get_layout
> > Why another get_layout resident at command.py?
> I don't think that's a problem, they have a different scope. get_layout
> in commands.py could be renamed to get_layout_by_name or something like
> that, but I don't see any reason to do so.
> >
> > I think they must be at the same place. It's easier to management.
> They do different things; one is a UI layer helper for getting a layout by
> name. The other is for finding the layout to use for a repository.
>
> I know they were different things, I means the UI only provide the
layoutname, and the creating of Layout will at the single point. so we can
separate it up. Right place do the right things, UI's main function is
providing the parameters, and these parameters are used by repository:)

> Cheers,
>
> Jelmer
>
>
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
> You are the owner of lp:~luoyonggang/bzr-svn/perfect-layout.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Jelmer Vernooij (jelmer)]

On 14/06/11 16:36, Yonggang Luo wrote:
> 2011/6/14 Jelmer Vernooij <email address hidden>
>
>> On 14/06/11 15:58, Yonggang Luo wrote:
>>> Another problem, because of repo already have an function named with
>>> get_layout
>>> Why another get_layout resident at command.py?
>> I don't think that's a problem, they have a different scope. get_layout
>> in commands.py could be renamed to get_layout_by_name or something like
>> that, but I don't see any reason to do so.
>>> I think they must be at the same place. It's easier to management.
>> They do different things; one is a UI layer helper for getting a layout by
>> name. The other is for finding the layout to use for a repository.

> I know they were different things, I means the UI only provide the
> layoutname, and the creating of Layout will at the single point. so we can
> separate it up. Right place do the right things, UI's main function is
> providing the parameters, and these parameters are used by repository:)
That's already the case - the actual looking up is happening with this
single line of code in both cases:

ret = layout_registry.get(layoutname)()

The UI helper function has some logic to cope with the fact that command
line options can be unicode (whereas layout names are strictly ascii at
the moment), and it has some code to display a command line user error
if the specific layout was not found.

Cheers,

Jelmer

[Yonggang Luo (luoyonggang)]

2011/6/14 Jelmer Vernooij <email address hidden>

> On 14/06/11 16:36, Yonggang Luo wrote:
> > 2011/6/14 Jelmer Vernooij <email address hidden>
> >
> >> On 14/06/11 15:58, Yonggang Luo wrote:
> >>> Another problem, because of repo already have an function named with
> >>> get_layout
> >>> Why another get_layout resident at command.py?
> >> I don't think that's a problem, they have a different scope. get_layout
> >> in commands.py could be renamed to get_layout_by_name or something like
> >> that, but I don't see any reason to do so.
> >>> I think they must be at the same place. It's easier to management.
> >> They do different things; one is a UI layer helper for getting a layout
> by
> >> name. The other is for finding the layout to use for a repository.
>
> > I know they were different things, I means the UI only provide the
> > layoutname, and the creating of Layout will at the single point. so we
> can
> > separate it up. Right place do the right things, UI's main function is
> > providing the parameters, and these parameters are used by repository:)
> That's already the case - the actual looking up is happening with this
> single line of code in both cases:
>
> ret = layout_registry.get(layoutname)()
>
> That's why I mentioned to merge this two function, it's implement the same
function at two different place and get the things
to be complicated? Don't you realized of this?

Compare the codebase bzr-svn with hgsubversion, one is 600kb+, one is 150kb,
I don't think bzr-svn implement much more function than hgsubversion, but
why the codebase is so large? Even though the framework is better, but the
codebase is really redundant， this is one of the example, I was try to let
you realize of it, but you just ignore of that.
Python is a script language, but you using it like a c language. That's I
want to say.

Sorry for the critical, but that's an fact.

> The UI helper function has some logic to cope with the fact that command
> line options can be unicode (whereas layout names are strictly ascii at
> the moment), and it has some code to display a command line user error
> if the specific layout was not found.
>
> That's what I was accepted, but I think should implement an function
get_layoutname, but not get_layout, because get_layout is the working of
repository... Do don't implement the same function here and there. that's
make no sense.

> Cheers,
>
> Jelmer
>
>
>
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/perfect-layout/+merge/64530
> You are the owner of lp:~luoyonggang/bzr-svn/perfect-layout.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Jelmer Vernooij (jelmer)]

On 14/06/11 17:31, Yonggang Luo wrote:
> 2011/6/14 Jelmer Vernooij <email address hidden>
>
>> On 14/06/11 16:36, Yonggang Luo wrote:
>>> 2011/6/14 Jelmer Vernooij <email address hidden>
>>>
>>>> On 14/06/11 15:58, Yonggang Luo wrote:
>>>>> Another problem, because of repo already have an function named with
>>>>> get_layout
>>>>> Why another get_layout resident at command.py?
>>>> I don't think that's a problem, they have a different scope. get_layout
>>>> in commands.py could be renamed to get_layout_by_name or something like
>>>> that, but I don't see any reason to do so.
>>>>> I think they must be at the same place. It's easier to management.
>>>> They do different things; one is a UI layer helper for getting a layout
>> by
>>>> name. The other is for finding the layout to use for a repository.
>>> I know they were different things, I means the UI only provide the
>>> layoutname, and the creating of Layout will at the single point. so we
>> can
>>> separate it up. Right place do the right things, UI's main function is
>>> providing the parameters, and these parameters are used by repository:)
>> That's already the case - the actual looking up is happening with this
>> single line of code in both cases:
>>
>> ret = layout_registry.get(layoutname)()
>>

> That's why I mentioned to merge this two function, it's implement the same
> function at two different place and get the things
> to be complicated? Don't you realized of this?
These two functions do really different things. One could be called
get_layout_for_repository and the other could be called
get_layout_by_name. If you think they can be merged, I'm happy to review
a merge proposal to that extend, but they only really have one line in
common (the one I mentioned earlier). I'm not sure how that could be
factored out.

>
> Compare the codebase bzr-svn with hgsubversion, one is 600kb+, one is 150kb,
> I don't think bzr-svn implement much more function than hgsubversion, but
> why the codebase is so large? Even though the framework is better, but the
> codebase is really redundant， this is one of the example, I was try to let
> you realize of it, but you just ignore of that.
> Python is a script language, but you using it like a c language. That's I
> want to say.
Unlike hgsubversion, bzr-svn allows random access to svn repositories,
it doesn't just allow pushing and pulling. It also supports
roundtripping push (push that preserves all metadata). I'm not sure if
that qualifies the larger size of the project, but that's definitely one
of the reasons.

I'm happy to merge branches that remove redundancy where it is present.
However, so far you haven't mentioned anything that would actually
simplify the code base *without* throwing away existing features.

How is the codebase C like?

Cheers,

jelmer

[Jelmer Vernooij (jelmer)]

Marking as rejected for the moment, please change back to "needs review" when applicable.

Unmerged revisions

3734. By Yonggang Luo on 2011-06-14

Add it first!

3733. By Yonggang Luo on 2011-06-14

No prefix for perfect.
We need all history:!

3732. By Yonggang Luo on 2011-06-14

Create layout on an single place!

=== modified file 'commands.py'
--- commands.py	2010-12-26 14:29:55 +0000
+++ commands.py	2011-06-14 12:09:26 +0000
@@ -30,21 +30,14 @@
     )
 
 
-def get_layout(layoutname):
-    """Parse layout name and return a layout.
+def get_layoutname(layoutname):
+    """Parse layout name and return a normal layout name.
 
     :param layout: Name of the layout to retrieve.
     """
     if isinstance(layoutname, unicode):
         layoutname = layoutname.encode("ascii")
-    from bzrlib.plugins.svn.layout import layout_registry
-    from bzrlib.errors import BzrCommandError
-
-    try:
-        ret = layout_registry.get(layoutname)()
-    except KeyError:
-        raise BzrCommandError('No such repository layout %r' % layoutname)
-    return ret
+    return layoutname
 
 
 class cmd_svn_import(Command):
@@ -70,7 +63,7 @@
                      Option('all',
                          help='Convert all revisions, even those not in '
                               'current branch history.'),
-                     Option('layout', type=get_layout,
+                     Option('layoutname', type=get_layoutname,
                          help='Repository layout (none, trunk, etc). '
                               'Default: auto.'),
                      Option('keep',
@@ -87,7 +80,7 @@
                     ]
 
     def run(self, from_location, to_location=None, format=None, trees=False,
-            standalone=False, layout=None, all=False, prefix=None, keep=False,
+            standalone=False, layoutname=None, all=False, prefix=None, keep=False,
             restore=False, until=None):
         from bzrlib import (
             osutils,
@@ -152,25 +145,12 @@
         from_repos.lock_read()
         try:
             if prefix is not None:
-                if layout is None:
-                    overall_layout = from_repos.get_guessed_layout()
-                else:
-                    overall_layout = layout
                 prefix = prefix.strip("/") + "/"
-                if overall_layout.is_branch(prefix):
-                    raise BzrCommandError("%s appears to contain a branch. "
-                            "For individual branches, use 'bzr branch'." %
-                            from_location)
-                # FIXME: Hint about is_tag()
-                elif overall_layout.is_branch_parent(prefix):
-                    self.outf.write("Importing branches with prefix /%s\n" %
-                        urlutils.unescape_for_display(prefix, self.outf.encoding))
-                else:
-                    raise BzrCommandError("The specified path is inside a branch. "
-                        "Specify a different URL or a different repository layout (see also 'bzr help svn-layout').")
 
             if (prefix is not None and
                 from_repos.transport.check_path(prefix, to_revnum) == NODE_NONE):
+                #TODO:Is this really necessary? because once a branch removed in SVN.
+                #Then SVN won't see it anymore, but it's sill in the history!  
                 raise BzrCommandError("Prefix %s does not exist" % prefix)
 
             def filter_branch(branch):
@@ -179,8 +159,10 @@
                     return False
                 return True
 
-            trace.note("Using repository layout: %s",
-                       layout or from_repos.get_layout())
+            layout = from_repos.get_layout(layoutname, prefix)
+            if layoutname is 'perfect':
+                prefix = None
+            trace.note("Using repository layout: %s", layout)
             convert_repository(from_repos, to_location, layout,
                 not standalone, trees, all, format=format,
                 filter_branch=filter_branch, keep=keep,

=== modified file 'layout/__init__.py'
--- layout/__init__.py	2011-06-07 00:39:20 +0000
+++ layout/__init__.py	2011-06-14 12:09:26 +0000
@@ -306,6 +306,9 @@
 layout_registry.register_lazy("itrunk3", "bzrlib.plugins.svn.layout.standard",
     "InverseTrunkLayout3")
 
+layout_registry.register_lazy("perfect", "bzrlib.plugins.svn.layout.perfect",
+    "PerfectLayout")
+
 class RepositoryRegistry(registry.Registry):
 
     def get(self, name):

=== added file 'layout/perfect.py'
--- layout/perfect.py	1970-01-01 00:00:00 +0000
+++ layout/perfect.py	2011-06-14 12:09:26 +0000
@@ -0,0 +1,345 @@
+from subvertpy import NODE_DIR
+
+from bzrlib import urlutils
+from bzrlib.plugins.svn import errors as svn_errors
+from bzrlib.plugins.svn.layout import (
+    RepositoryLayout,
+    get_root_paths,
+    wildcard_matches,
+    )
+
+class SVNBranches(object):
+    def __init__(self, _subdir):
+
+
+    def localname_tag(self, path):
+        if self.meta.layout == 'single':
+            return 'tip'
+        default_tag_prefix = self.subdir + 'tags/'
+        if path == self.subdir + 'trunk' or path is None:
+            return 'tip'
+        elif path.startswith(default_tag_prefix):
+            return path[len(default_tag_prefix):]
+        return path
+
+    def remotename_tag(self, tag):
+        if self.meta.layout == 'single':
+            return '/'
+        if tag == 'tip' or tag is None:
+            return self.subdir + 'trunk'
+        elif tag.startswith('/'):
+            return tag
+        return self.subdir + 'tags/' + tag
+
+    def get_tag_name(self, branch_info = None):
+        if self.meta.layout == 'single':
+            return None
+        if not branch_info:
+            return None
+        if not self.exist_branch(branch_info[2], branch_info[3]):
+            return None
+        return self.is_tag(branch_info[0])
+
+    def is_tag(self, branch_path):
+        for tagspath in self.tag_locations:
+            tag_prefix = self.subdir + tagspath + '/'
+            if branch_path.startswith(tag_prefix):
+                return self.localname_tag(branch_path)
+        return None 
+
+    def get_paths(self, revision):
+        paths = {}
+        for p,item in revision.paths.iteritems():
+            paths[p] = svn_changes(item)
+        return paths
+
+    def update(self, svn, revision):
+        """
+        Given a revision object, determine changes to branches.
+        """
+        self.added_tags, self.deleted_tags = {},{}
+        self.deleted_branches = set()
+        self.modified = {}
+        if self.meta.layout == 'single':
+            self.modified['/'] = 1
+            self.all_branches = {'/':{-1:['/', 0, 1]}}
+            return
+
+        paths, revnum = self.get_paths(revision), revision.revnum
+        print(paths, revnum)
+        sorted_paths = sorted(paths.keys())
+        current_test_branches = frozenset(self.all_branches.keys())
+        for p in sorted_paths:
+            if paths[p].action != 'D' and  paths[p].action != 'R':
+                continue
+            np = p + '/'
+            for branch_path in current_test_branches:
+                if not self.exist_branch(branch_path, revnum):
+                    continue
+                if not (branch_path + '/').startswith(np):
+                    continue
+                self.remove_branch(branch_path, revnum)
+
+        for p in sorted_paths:
+            copy_from_path = paths[p].copyfrom_path
+            if (copy_from_path is None):
+                continue
+            copy_from_path += '/'
+            parent_rev = paths[p].copyfrom_rev
+            for parent_path in current_test_branches:
+                rn = parent_path + '/'
+                if not rn.startswith(copy_from_path):
+                    continue
+                if not self.exist_branch(parent_path, parent_rev):
+                    continue
+                suffix = ''
+                if (rn != copy_from_path):
+                    suffix = parent_path[len(copy_from_path) - 1:]
+                new_branch_path = p + suffix
+                if self.real_branch(new_branch_path) == new_branch_path:
+                    self.add_branch(new_branch_path, revnum, parent_path, parent_rev)
+
+        for p in sorted_paths:
+            new_branch_path = None
+            branch_info = self.get_file_branch(p, revnum)[1]
+            if branch_info is None:
+                new_branch_path = self.create_branch(p)
+            else:
+                self.modified[branch_info[0]] = 1
+            if (paths[p].action == 'D'):
+                continue
+            if self.contains_branch(p, revnum):
+                continue
+
+            # If p is an exist file at in some current opened branch 
+
+            #Now, /llvm/branches/Apple/williamson processed at the begining!
+            #Carefully handle there is copyfrom, but the from palce didn't have any branches!
+            cp_from = paths[p].copyfrom_path
+            if ( (cp_from and not cp_from.startswith(self.subdir)
+                and self.real_branch(p) == p
+                and (branch_info == None or branch_info[3] == 0)) #This means copies outside the current subdir
+                or self.exist_branch(p, revnum, 2) != None): #This means there is an branch at path p before:)
+                new_branch_path = p
+
+            if new_branch_path and svn.checkpath(p.lstrip('/'), revnum) == 'd':
+                sub_path = self.get_file_path(p, new_branch_path)
+                parent_path, parent_rev = self.subdir + 'trunk', 0
+                if cp_from:
+                    parent_path = cp_from[:len(cp_from) - len(sub_path)]
+                    parent_rev = paths[p].copyfrom_rev
+                if sub_path != '':
+                    self.modified[new_branch_path] = 1
+                print("Try add branch with ",p , str(new_branch_path) +':' + str(revnum), sub_path, parent_path + ':' + str(parent_rev))
+                self.add_branch(new_branch_path, revnum, parent_path, parent_rev)
+
+    def contains_branch(self, path, revnum):
+        for branch_path, revlist in self.all_branches.iteritems():
+            if not branch_path.startswith(path):
+                continue
+            if -1 in revlist:
+                return True
+        return False
+
+    def real_branch(self, path, ret_original = True):
+        new_path = path + '/'
+        #TODO: We can fitler more, user can specify which folder can't 
+        #contains newly created branch:)
+        trunk_suffix = '/trunk/'
+        index = new_path.find("/trunk/")
+        if index == -1:
+            if ret_original:
+                return path
+            return None
+        index = index + len(trunk_suffix)
+        path = new_path[0:index - 1]
+        return path
+
+    def create_branch(self, path):
+        if self.meta.layout == 'single':
+            return ''
+
+        elts = [p for p in path.split('/') if p]
+        branch_path = self.real_branch(path, False)
+        if branch_path != None:
+            return branch_path
+        elif path.startswith(self.branch_prefix):
+            branch_path = '/'.join(elts[:3])
+        else:
+            branch_path = '/'.join(elts[:-1])
+        branch_path = '/' + branch_path
+        if branch_path.startswith(self.branch_prefix):
+            return branch_path
+        return None
+
+    def add_tag(self, hash, tag, revnum):
+        if tag in self.meta.tagmap and not self.meta.tagmap[tag]:
+            return
+        print("Add tag", tag, revnum)
+        parent_id = self.meta.get_tag_parent(hash, revnum)
+        self.added_tags[tag] = parent_id, hash
+
+    def add_branch(self, branch_path, open_revision, parent_path, parent_rev, must_add = True):
+        tag_name = self.is_tag(branch_path)
+        branch_info = [parent_path, parent_rev, open_revision, True]
+        from_info = self.exist_branch(branch_path, parent_rev)
+        if tag_name and from_info is None:
+            print("Try to create an tag from nothing!", tag_name, branch_info, from_info)
+            return
+
+        self.modified[branch_path] = 0
+        if branch_path in self.all_branches:
+            revlist = self.all_branches[branch_path]
+            if -1 in revlist:
+                revlist[-1][3] = branch_info[3]
+            else:
+                revlist[-1] = branch_info
+        else:
+            self.all_branches[branch_path] = {-1:branch_info}
+
+    def remove_branch(self, branch_path, close_revision):
+        self.modified[branch_path] = 0
+        if branch_path in self.all_branches:
+            if (-1 not in self.all_branches[branch_path]):
+                raise Exception('Should contains key -1 in', self.all_branches[branch_path])
+            branch_info = self.all_branches[branch_path].pop(-1)
+            self.all_branches[branch_path][close_revision] = branch_info
+            branch_info = self.exist_branch(branch_path, close_revision, 1)
+            tag_name = self.get_tag_name(branch_info)
+            print('Close branch or tag with info', ['tag name:' + str(tag_name), branch_path, close_revision] + branch_info)
+            if tag_name:
+                self.update_tags[branch_path] = 'D'
+                parent_path = self.get_parent(branch_path, close_revision - 1)[0]
+                self.deleted_tags[tag_name] = self.meta.localname(parent_path), None
+            else:
+                branch = self.meta.localname(branch_path)
+                self.deleted_branches.add(branch)
+        else:
+            raise Exception('Remove an unexist branch',branch_path, 'in', self.all_branches)
+
+    def exist_branch(self, branch_path, branch_rev, include_state = 0):
+        '''
+        include_state = 0 means there is an internval [open,close) contains branch_rev
+        include_state = 1 means there is an internval [open,close] contains branch_rev
+        include_state = 2 means if there is an interval [open,close), then just return it.
+            else return the interval that got the maximal close and also the close <= branch_rev
+        '''
+        if branch_path not in self.all_branches.iterkeys():
+            return None
+        revlist = self.all_branches[branch_path]
+        result_revision = None
+        if include_state == 1 and branch_rev in revlist:
+            result_revision = branch_rev
+        elif -1 in revlist and (branch_rev == -1 or revlist[-1][2] <= branch_rev):
+            result_revision = -1
+        else:
+            for close_revision in revlist:
+                open_revision = revlist[close_revision][2]
+                if (open_revision <= branch_rev and branch_rev < close_revision):
+                    result_revision = close_revision
+                    break
+        if result_revision is None and include_state == 2:
+            result_revision = -2
+            for close_revision in revlist:
+                if close_revision <= branch_rev and close_revision > result_revision:
+                    result_revision = close_revision
+            if result_revision == -2:
+                result_revision = None
+        if result_revision is None:
+            return None
+        branch_info = list(revlist[result_revision])
+        branch_info.insert(3, result_revision)
+        return [branch_path, branch_rev] + branch_info
+
+    def get_file_path(self, path, branch_path):
+        if path == None or branch_path == None:
+            return ''
+        return path[len(branch_path) + 1:]
+
+
+class PerfectLayout(RepositoryLayout):
+    def __init__(self, _log = None, _prefix = None):
+        if (_log is None or _prefix is None):
+            return
+        self.log = _log
+        self.added_tags, self.deleted_tags = {},{}
+        self.update_tags = {}
+        self.deleted_branches = set()
+        self.subdir = _prefix
+        self.tag_prefixes = self.subdir + 'tags/'
+        self.branch_prefixes = self.subdir + 'branches/'
+        self.init_trunk = self.subdir + 'trunk/'
+        print('PerfectLayout with log', self.log, self.prefix)
+
+    def get_project_prefixes(self, project):
+        return [""]
+
+    def supports_tags(self):
+        return True
+
+    def get_tag_path(self, name, project=""):
+        """Return the path at which the tag with specified name should be found.
+
+        :param name: Name of the tag.
+        :param project: Optional name of the project the tag is for. Can include slashes.
+        :return: Path of the tag.
+        """
+        raise NotImplementedError
+
+    def get_tag_name(self, path, project=""):
+        """Determine the tag name from a tag path.
+
+        :param path: Path inside the repository.
+        """
+        raise NotImplementedError
+
+    def parse(self, path):
+        """Parse a path.
+
+        :return: Tuple with type ('tag', 'branch'), project name, branch path and path
+            inside the branch
+        """
+
+        raise svn_errors.NotSvnBranchPath(path)
+
+    def get_branches(self, repository, revnum, project=None, pb=None):
+        """Retrieve a list of paths that refer to branches in a specific revision.
+
+        :return: Iterator over tuples with (project, branch path)
+        """
+        raise NotImplementedError
+
+    def get_tags(self, repository, revnum, project=None, pb=None):
+        """Retrieve a list of paths that refer to tags in a specific revision.
+
+        :return: Iterator over tuples with (project, branch path)
+        """
+        raise NotImplementedError
+
+    def __repr__(self):
+        return "%s" % (self.__class__.__name__)
+
+    def __str__(self):
+        return "perfect"
+
+    def _is_prefix(self, branches, path, project=None):
+        if (path == self.prefix):
+            return True
+        print('branches', branches, path, project)
+        for branch in branches:
+            if branch.startswith("%s/" % path):
+                return True
+        return False
+    
+    def _get_branches(self):
+        return []
+
+    def _get_tags(self):
+        return []
+
+    def is_branch_parent(self, path, project=None):
+        return self._is_prefix(self._get_branches(), path, project)
+
+    def is_tag_parent(self, path, project=None):
+        print(path)
+        return self._is_prefix(self._get_tags(), path, project)

=== modified file 'layout/standard.py'
--- layout/standard.py	2011-05-11 12:48:05 +0000
+++ layout/standard.py	2011-06-14 12:09:26 +0000
@@ -330,7 +330,7 @@
 
     def is_branch(self, path, project=None):
         for bp in self.branches:
-           if wildcard_matches(path, bp):
+            if wildcard_matches(path, bp):
                 return True
         return False
 

=== modified file 'repository.py'
--- repository.py	2011-06-09 11:29:06 +0000
+++ repository.py	2011-06-14 12:09:26 +0000
@@ -659,23 +659,29 @@
         self.get_config().set_layout(layout)
         self._layout_source = LAYOUT_SOURCE_CONFIG
 
-    def get_layout(self):
+    def get_layout(self, layoutname = None, prefix = None):
         """Determine layout to use for this repository.
 
         This will use whatever layout the user has specified, or
         otherwise the layout that was guessed by bzr-svn.
         """
-        return self.get_layout_source()[0]
+        return self.get_layout_source(layoutname, prefix)[0]
 
-    def get_layout_source(self):
+    def get_layout_source(self, layoutname = None, prefix = None):
         if self._layout is None:
             self._layout_source = LAYOUT_SOURCE_MAPPING_MANDATED
             self._layout = self.get_mapping().get_mandated_layout(self)
         if self._layout is None:
-            layoutname = self.get_config().get_layout()
+            layoutname = layoutname or self.get_config().get_layout()
             if layoutname is not None:
                 self._layout_source = LAYOUT_SOURCE_CONFIG
-                self._layout = layout.layout_registry.get(layoutname)()
+                try:
+                    if layoutname == 'perfect':
+                        self._layout = layout.layout_registry.get(layoutname)(self._log, prefix)
+                    else: 
+                        self._layout = layout.layout_registry.get(layoutname)()
+                except KeyError:
+                    raise bzr_errors.BzrCommandError('No such repository layout %r' % layoutname)
         if self._layout is None:
             branches = self.get_config().get_branches()
             tags = self.get_config().get_tags()