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
=== modified file 'bzrlib/info.py'
--- bzrlib/info.py 2009-05-01 14:29:06 +0000
+++ bzrlib/info.py 2009-08-31 04:35:14 +0000
@@ -365,7 +365,13 @@
365 verbose = 2365 verbose = 2
366 layout = describe_layout(repository, branch, working)366 layout = describe_layout(repository, branch, working)
367 format = describe_format(control, repository, branch, working)367 format = describe_format(control, repository, branch, working)
368 outfile.write("%s (format: %s)\n" % (layout, format))368 from bzrlib.smart.client import _SmartClient
369 if isinstance(repository._client, _SmartClient):
370 server_version = repository._client._headers['Software version']
371 server_info = " (smart server %s)" % server_version
372 else:
373 server_info = ""
374 outfile.write("%s (format: %s)%s\n" % (layout, format, server_info))
369 _show_location_info(gather_location_info(repository, branch, working),375 _show_location_info(gather_location_info(repository, branch, working),
370 outfile)376 outfile)
371 if branch is not None:377 if branch is not None: