[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