Transport patch (was Please explain LOCATION in the pull command)

[John A Meinel]

Martin Pool wrote:
> On 13/09/05, John A Meinel <john at arbash-meinel.com> wrote:
>
>
>>Currently only http:// and local references are supported. ftp support
>>in the current framework is possible.
>>I developed an backend change, which was supposed to make it easier to
>>install new protocols (and I implemented local filesystem, http, sftp
>>(crude) and rsync versions).
>>
>>Currently, a lot of the bzr code can only write to the local filesystem,
>>and makes assumptions (that os.normpath() works correctly) on files it
>>is going to write to. So my Transport work only really fixed the read
>>portion (but left open the ability to create proper write support).
>>
>>I think it has bit-rotted a little bit, I branched at about revno=900 or
>>so, and now bzr.dev is on 1197. I've been on vacation, but I'll
>>hopefully have some time to clean it up for Martin to look at it again.
>
>
> It has rotted a bit so not easy to merge in, but I'm looking at the
> diff.  (Attached, in case anyone else wants to comment - Robert?)
>
> It looks a bit large and intimidating but I see a fair fraction of the
> changes are just replacements of direct io with redirections through
> the transport, and that should be easy to update.

There were quite a few changes, but it should be as you say, the core of
the changes are fairly small, and the rest are just redirecting i/o.

I'll try and clean it up. When I do so, you might consider releasing it
as an "extra dev" branch, so that people can have some time to hack on
it. I think I wrote a bunch of test cases, but that probably needs more
work still.

>
> I think that the Branch should probably discard any cached information
> when it's unlocked.  While it's locked, we assume that nobody else can
> change it.

That seems like a decent time to do it. I just need to figure out how to
get back to the Transport from the lock object. Probably just need to
use a weakref.

>
> Indeed it might be nice to cache digested objects, rather than files,
> so as to avoid repeatedly parsing, but that doesn't have to be done
> now.

True, but that is a different layer of caching. I don't remember
Transport itself doing much caching, it was more other pieces doing it.
(RsyncTransport and SftpTransport both cache because they just copy the
files across). A child Transport sets a "should_cache" flag because the
old Branch code would create a CachedStore when appropriate. In the case
of Http it makes sense to cache, since HttpTransport doesn't do it, but
you don't want to cache local access, and Rsync/Sftp were already
caching things.

I agree that digested objects are better for caching, but that would be
at an entirely different layer than Transport/Storage.

>
> I see you commented out the code that hides the tracebacks.  I can
> understand why :-) but I think for regular users it's much better not
> to have them.  In my tree now if you set BZR_DEBUG=1 then you get
> trace messages and full backtraces on stderr, which is better for
> developers.

Good to hear. I'll use the environment flag for it then.

>
> We can probably get rid of ScratchBranch etc now that the test
> framework runs each test inside a temporary directory; that might mean
> moving some tests on branch from docstrings into regular tests but
> that's OK.
>
> A stylistic things, barely worth mentioning and some are arbitrary but
> it is good to keep things consistent.
>
>  - The first line of a docstring should be a self-contained sentence.

I'll work on that. I realize I tend to be a little too verbose, and
single sentence summaries are tricky (I always feel they are lying a
little bit).

>
>  - Our pattern is to keep tests in bzrlib.selftest, not in the code
> they describe.

I was writing a generic Transport test, which can be run for any
implementation, but isn't run on its own. I can certainly put that into
selftest instead.

>
> Reviewing this makes me think it would be nice to have a command we
> could call "diff-for-merge" (maybe something better, "outstanding"?),
> which shows a the diff since the last point on a branch that has been
> merged.

Just use my changeset plugin.
If you did:
bzr cset ../bzr.dev

I think it would find the base that would be picked by the merge code,
and then produce the appropriate changeset (which is basically just the
diff). You could also force the base with "bzr cset ../bzr.dev
../bzr.dev at something"

Though that might also have bit-rotted in the month I was gone.

>
> I'd be happy to put in an updated form of this.
>

I'll work on writing it. :)
Good to know you think the basic structure is okay.

Also, one thing I wanted to have implemented was a "get_partial" which
would let you pull only part of a remote file. (To support revfile, and
possible weaves depending on how they are designed). I'll try and put
that it before coming back to you (and yes, I'll make sure there are tests).

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/20050914/2a6d06a8/attachment.pgp