[BUG] transport/memory.py doesn't handle strings
John A Meinel
john at arbash-meinel.com
Wed Oct 19 03:57:20 BST 2005
Robert Collins wrote:
> On Tue, 2005-10-18 at 16:45 -0500, John A Meinel wrote:
>
>>There are a couple of changes that should be made to the
>>transport/memory.py implementation.
>>First, most of the "put" functions can take either a file-like object, or
>>a string. Which means that MemoryTransport.put() shouldn't just call
>>read(), it should also check to see if it is already a string. (Either
>>that, or we can change the api if you really prefer).
>
>
> I think that the ambigious api is very strange. I'd much rather have
> explict put_stream, put_string etc.
>
>
>>Also, I would recommend including MemoryTransport into the test suite.
>
>
> What do you mean by that ? (There are comprehensive tests for
> MemoryTransport).
When I first wrote this, I thought it wasn't being tested for some
foolish reason, then I saw the test and wondered why it wasn't the same,
because the test you wrote doesn't cover quite as much of the api as the
tests I wrote (using generators vs lists, files vs strings, etc).
But then ...
>
>
>>I already wrote a suite for all Transports which you just have to inherit
>>from. I see that there is a test suite, but all you really had to do was
>>inherit from the TestTransportMixin which makes sure to test the complete
>>API. (See how TestLocalTransport and TestHttpTransport are implemented).
>
>
> Yes, and they do stuff to local disk and check that it worked. Its uhm,
> difficult to do that with an in memory only transport.
You are completely right that you can't use my tests. I feel quite
foolish about this.
>
>
>>I'm also concerned that MemoryTransport.clone() returns the exact copy of
>>itself. Because one of the fundamental bits of Transport objects is that
>>they have a "base" where all paths are given relative to that base.
>>
>>Since all files and directories are stored inside the dictionary as
>>relative paths, clone(offset=".bzr/") will fail. And this type of cloning
>>is used in bzrlib/branch.py so that the Store objects have "base" set
>>appropriately.
>
>
> In bzrlib/branch.py, cloning at the top is checked for by
> 'new_transport.base == old_transport.base'. I was duplicating that
> interface.
Actually, that is used to check if you have reached the "root" of the
filesystem. In general if you cd /../ you will end up in / not in an
error. So the way you detect the top is to just check that the current
base is the same as the parent base.
If no offset is given, then you should return the same base, in the case
of an in-memory:// tree, I think you could have a base, but have it
always be root.
So that you would end up with in-memory:/ in-memory:/.bzr/
in-memory:/.bzr/branch-format, etc.
So that if you try and get "in-memory:/.." you would just get back
"in-memory:/", which would then succeed at the "new_t.base ==
old_t.base" to prevent infinite recursion.
>
>
>>clone() is also used in find_branch() since it returns a clone of the
>>parent directory when it is searching for ".bzr/".
>>Again, I realize that MemoryTransport is probably not used everywhere, but
>>I thought I would point out some small bugs.
>
>
> Right, this is the interface I was coding for, because that is how your
> current local transports appeared to indicate that they had failed to
> chdir.
>
No, they are only supposed to do this when they reach the root. (Because
of the cd to parent gives you parent at /).
Otherwise I don't have a way to indicate a failed chdir. The current
transports use this because that is what you get from os.chdir(), or
from just using cd on a normal filesystem.
However, I should also mention that MemoryTransport should probably use
a global/class dictionary, so that if I put a file into
".bzr/branch-format" and then I clone into ".bzr/", and then ask for
"branch-format" I get back the same file.
>
>>(sorry I don't have a patch/branch to give you, but I'm on a machine
>>without bzr installed, using webmail, in a coffee house).
>
>
> No probs.
>
> Rob
>
John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051018/dcf9cc26/attachment.pgp
More information about the bazaar
mailing list