Bazaar

Merge lp://staging/~vila/bzr/308472-info-version-smart-server into lp://staging/~bzr/bzr/trunk-old

  1. 308472-info-version-smart-server
  2. Merge into trunk-old
Proposed by Vincent Ladeuil
Status: Rejected
Rejected by: Vincent Ladeuil
Proposed branch: lp://staging/~vila/bzr/308472-info-version-smart-server
Merge into: lp://staging/~bzr/bzr/trunk-old
Diff against target: 18 lines
To merge this branch: bzr merge lp://staging/~vila/bzr/308472-info-version-smart-server
Reviewer Review Type Date Requested Status
Andrew Bennetts Needs Resubmitting
Robert Collins (community) Approve
Review via email: mp+10837@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

See bug #308472 and bug #420408.

I'm just forwarding the patch in the bug in that merge proposal so that we better track it.

Revision history for this message
Robert Collins (lifeless) wrote :

I think this is ok; it would be better to have tests.

What would be better [and definitely needs tests] is that doing 'bzr info bzr://SERVER/' (that is, pointing at somewhere that isn't a branch or repo at all, but has a functioning server) would still do something useful.

-Rob

review: Approve
Revision history for this message
Andrew Bennetts (spiv) wrote :

The idea seems ok, but:

 - it should guard against the server sending no 'Software version' header. (Unlikely, but possible.)
 - it should not assume repositories have a _client attribute. I expect that will break on local repositories. Instead it should probably use something like repository.bzrdir.root_transport.get_smart_medium() and construct a _SmartClient from that. (Actually, it shouldn't depend on there being a remote repository at all, e.g. consider branch references.)
 - I think the smart server version ought to be reported on its own line, not as part of the "format".

And as Robert says, it would be nice if it worked even without a remote bzrdir of any sort, and it would be good to add tests.

review: Needs Fixing
Revision history for this message
Andrew Bennetts (spiv) wrote :

> - it should not assume repositories have a _client attribute. I expect that

In fact, I see from my review in BundleBuggy it's worse than that, this is actually reporting the client's version, not the server's! See <https://lists.ubuntu.com/archives/bazaar/2009q3/060332.html>

Perhaps a good change would be to look for this header in the response to the 'Repository.gather_stats' RPC, although that doesn't fit neatly in the API, and won't help when there's no remote repository.

Perhaps the right time to fix this is when we add a no-VFS way to do info via smart server? (That way, even if the server is too old to support the new no-VFS verb for info we still have an obvious point to check for the 'Software version' header in the error response.)

review: Needs Resubmitting

Unmerged revisions

4506. By Stephen Emslie <stephen@janet-laptop>

Add smart server version number to info output.

4505. By Canonical.com Patch Queue Manager <email address hidden>

(robertc) Document how,
 why and issues with inventory deltas. (Robert Collins)

4504. By Canonical.com Patch Queue Manager <email address hidden>

(andrew) Fix some trivial bugs and unused/redundant imports reported
 by pyflakes.

4503. By Canonical.com Patch Queue Manager <email address hidden>

Fix selftest when using both --starting-with and --load-list

4502. By Canonical.com Patch Queue Manager <email address hidden>

(mbp) fix log+ transport decorator

4501. By Canonical.com Patch Queue Manager <email address hidden>

Add a --strict option to send

4500. By Canonical.com Patch Queue Manager <email address hidden>

(bialix) Add --use-existing-dir to branch

4499. By Canonical.com Patch Queue Manager <email address hidden>

(vila)(trivial) Fix test_selftest.py imports

4498. By Canonical.com Patch Queue Manager <email address hidden>

(igc) Improve paths are not versioned reporting (Benoît PIERRE)

4497. By Canonical.com Patch Queue Manager <email address hidden>

(vila) Push --strict checks tree/branch sync

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/info.py'
2--- bzrlib/info.py 2009-05-01 14:29:06 +0000
3+++ bzrlib/info.py 2009-08-31 04:35:14 +0000
4@@ -365,7 +365,13 @@
5 verbose = 2
6 layout = describe_layout(repository, branch, working)
7 format = describe_format(control, repository, branch, working)
8- outfile.write("%s (format: %s)\n" % (layout, format))
9+ from bzrlib.smart.client import _SmartClient
10+ if isinstance(repository._client, _SmartClient):
11+ server_version = repository._client._headers['Software version']
12+ server_info = " (smart server %s)" % server_version
13+ else:
14+ server_info = ""
15+ outfile.write("%s (format: %s)%s\n" % (layout, format, server_info))
16 _show_location_info(gather_location_info(repository, branch, working),
17 outfile)
18 if branch is not None: