Recording where branches that are absent are

[Aaron Bentley]

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

Robert Collins wrote:
| I'm entirely comfortable with them being separate. I'm not comfortable
| with designing in failure modes we dont want.

That's not a failure mode.  It will work perfectly in that state.

| The .bzr/branch/format file would be different yes, and the fact they
| both occupy .bzr/branch/* would have that effect.

I think all working trees should specify the location of their branch,
even if it's just '.'.  This breaks that regularity.

| Yes you do need a working tree object, because reading anything other
| than the format file from within .bzr/checkout is an abstraction
| violation.

If there's the will, there's a way to do it without violating the
abstraction boundary.  Say a WorkingTreeBase class, which would be
unique to each format, but wouldn't be a WorkingTree.  Or perhaps the
WorkingTreeFormat object itself.

|>>with respect to potential cycles, I would not expect this to forward,
|>>that is a reference could not be a reference to a reference.
|>
|>In order to implement that, you will need Branch.open_really.  I would
|>prefer that Branch.open_really was named Branch.open.
|
|
| Huh? I don't understand why you would need that.
| bzrdir = BzrDir.open(referenced_url)
| format = BranchFormat.find_format(bzrdir)
| if isinstance(format, BranchPointerFormat):
|     raise ChainedBranchReferences(self)

This is an implementation of Branch.open?  So you've now turned
BzrDir.open into Branch.open_really.  I do not consider this intuitive.

This is ugly.  Assigns unwanted properties to branch formats.  You now
have two entirely different kinds of things living at the same location
which is why you have to use isinstance.

I think this is much better:

class Branch()
....
~   @static
~   def open_any(url):
~       try:
~           return Branch.open(url)
~       except NotBranch, e:
~           ref_url = Branch.branch_reference(url)
~           if ref_url is None:
~               raise e
~           return Branch.open(ref_url)

Though I continue to believe that .bzr/workingtree is the right place to
store the branch location of a working tree.

Actually, I think it might be nicer to stick it in a config file,
instead of putting one-line files everywhere.

|>I don't think it makes sense to have a .bzr/branch directory, if it does
|>not contain a branch.  I don't think you've shown that we should never
|>have both a branch and a branch reference.  Even if you do, it is quite
|>easy to enforce that rule without messing up the conceptual layout.
|
|
| I think its much harder to enforce that rule when the data storage
| permits it to be broken.

Impasse.

| I think the damage of having a working tree pointing to a branch that is
| not the one present locally is pretty clear - code that grabs branches
| directly will consistently have different information than the working
| tree does.

Any command that is attempting to use a WorkingTree's branch, but
opening it directly, is broken, and this will be easily diagnosed as
soon as we start using checkouts.

| I dont understand your objection to having the branch api manage whether
| the branch is local or not. I know you dont think it makes sense - but I
| dont know *why* you think that.

You're breaking the filesystem structure.  You're reducing code
regularity.  You're trying to tangle things back together now that we've
finally got them separated.

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

iD8DBQFD4Wtm0F+nu1YWqI0RAu1lAJ96+wXvqprUj1JIhtNatWKdeg2ZoQCdHzWM
HmUsLeVivquMVdLmhI4s6zM=
=4Vlw
-----END PGP SIGNATURE-----