[MERGE] (bug 193089) Change calls from socket.gethostname() to osutils.get_host_name()

[gnublade]

2009/7/6 Andrew Bennetts <andrew.bennetts at canonical.com>:
> 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.

The first change on the win32 platform is just for consistency, the
real bug fix is in the last call to socket.gethostname().

>> # 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?

I was attempting to follow jml's instructions here:

https://lists.ubuntu.com/archives/bazaar/2009q3/060216.html

and then I used `bzr send`

>> #
>> # 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: ..."

this isn't really a unicode issue but rather using the correct
encoding for the locale which osutils.get_host_name() does.

>> -    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 I said above I wasn't trying to fix the win32 case, but rather bug
193089 which is on ubuntu.  I've tried replicating the problem which I
can do without the fix, and the change appears to do the right thing.

> 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.

If the win32 case adds extra overhead I could just remove that for the
sake of this bug.

Cheers,

Andy