[MERGE] (bug 193089) Change calls from socket.gethostname() to osutils.get_host_name()
Andrew Bennetts
andrew.bennetts at canonical.com
Mon Jul 6 02:53:38 BST 2009
Andy Kilner wrote:
> (bug 193089) Change calls from socket.gethostname() to osutils.get_host_name()
>
> Actually sending the bundle this time.
bb:tweak
I'm not sure this code will actually cause any practical improvement. I do
like that it moves closer to relying on osutils for platform-specific logic,
though.
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: gnublade at googlemail.com-20090704163725-oigqsl7uf3ji8aj0
> # target_branch: lp:~gnublade/bzr/bzr.dev
> # testament_sha1: 0e48f112d6a378c12622f2c6541aa870d6fd07dd
> # timestamp: 2009-07-04 17:49:52 +0100
> # source_branch: lp:bzr
> # base_revision_id: pqm at pqm.ubuntu.com-20090703154118-f5ncmxfk75wgzh6l
You appear to have the target_branch and source_branch in this merge
directive reversed, so I expect Bundle Buggy will ignore it. How did you
generate this merge directive?
> #
> # Begin patch
> === modified file 'bzrlib/config.py'
> --- bzrlib/config.py 2009-06-11 06:49:21 +0000
> +++ bzrlib/config.py 2009-07-04 16:37:25 +0000
> @@ -833,7 +833,7 @@
> 'bzr whoami "Your Name <name at domain.com>"')
> host = win32utils.get_host_name_unicode()
> if host is None:
> - host = socket.gethostname()
> + host = osutils.get_host_name()
This code already tries win32utils.get_host_name_unicode first... still,
osutils.get_host_name() is probably better, because it can return unicode
rather than str, which is consistent with get_host_name_unicode().
I'm not sure it returns unicode consistently though... perhaps
osutils.get_host_name should use get_host_name_unicode not
win32utils.get_host_name? Then this code could simply do:
host = osutils.get_host_name()
With no conditionals, on all platforms.
Oh, I see, osutils.get_host_name is subtly different to this logic: it
doesn't fallback to socket.gethostname() if
win32utils.get_host_name_unicode() returns None. Probably
osutils.get_host_name should be fixed to do what this code does, i.e. "host
= win32...; if host is None: ..."
> - return realname, (username + '@' + socket.gethostname())
> + return realname, (username + '@' + osutils.get_host_name())
This code isn't used at all on win32, but it seems reasonable to make this
consistent with the win32 case.
As mentioned above I think it might be good to have this function just
always call osutils.get_host_name(), and nothing else, to determine the
host, regardless of platform. Then the platform-specific logic in this
function would be restricted to determining the realname and username. That
might be a bit too much scope creep for your simple change, so I'm happy
with your current change if you don't do this.
-Andrew.
More information about the bazaar
mailing list