Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: Hostname() truncate hostname more than 15 characters under Windows #9982

Closed
kartlee opened this issue Feb 24, 2015 · 15 comments
Closed

os: Hostname() truncate hostname more than 15 characters under Windows #9982

kartlee opened this issue Feb 24, 2015 · 15 comments
Milestone

Comments

@kartlee
Copy link

@kartlee kartlee commented Feb 24, 2015

Hi Folks,

One of our virtual machine name is pdx-srv2012-wv01. And os.Hostname(..) return hostname as pdx-srv2013-wv0 in this case. The native hostname.exe of windows from sysinternals return the right hostname. It looks like the implementation in GO seem to use GetComputerName(..) instead of GetComputerNameEx(..) to return the NetBIOS name. Is there any reason for it? Can we change this to return the dns hostname as NetBIOS seem to be getting old with later version of windows?

The version of GO I am using is 1.3.

-Karthik

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Feb 24, 2015

@bradfitz bradfitz added this to the Go1.5Maybe milestone Feb 24, 2015
@bradfitz bradfitz added the os-windows label Feb 24, 2015
@mattn

This comment has been minimized.

Copy link
Member

@mattn mattn commented Feb 24, 2015

os.Hostname() / syscall.ComputerName() call GetComputerName()

https://msdn.microsoft.com/ja-jp/library/windows/desktop/ms724295%28v=vs.85%29.aspx

https://github.com/golang/go/blob/master/src/syscall/ztypes_windows.go#L144

And MAX_COMPUTERNAME_LENGTH is defined as 15 in mingw header.

We should use GetComputerNameEx instead.

https://msdn.microsoft.com/ja-jp/library/windows/desktop/ms724301(v=vs.85).aspx

@cookieo9

This comment has been minimized.

Copy link
Contributor

@cookieo9 cookieo9 commented Feb 24, 2015

I have a CL that adds the *Ex equivalent functions to internal/syscall/windows (to not change syscall) but when I modify os.Hostname to use them instead, the bootstrap build of os fails because it can't import internal/syscall/windows for some reason.

The regular build of os, and it's tests seem to run fine if the patch is applied to a previously compiled tree.

@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Feb 24, 2015

@mikioh mikioh changed the title os.Hostname() truncate hostname more than 15 characters under Windows os: Hostname() truncate hostname more than 15 characters under Windows Feb 24, 2015
@cookieo9

This comment has been minimized.

Copy link
Contributor

@cookieo9 cookieo9 commented Feb 24, 2015

@minux That fixed bootstrap, but:

  • the dependency check in go/build fails as os has a new dependency
  • the change hard-codes a windows specific package in the list of built pacakges by cmd/dist so might break other GOOS's
@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Feb 24, 2015

@mattn

This comment has been minimized.

Copy link
Member

@mattn mattn commented Feb 25, 2015

@cookieo9

This comment has been minimized.

Copy link
Contributor

@cookieo9 cookieo9 commented Feb 25, 2015

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Feb 25, 2015

@kartlee please check if https://go-review.googlesource.com/#/c/5852 does fixes your issue. Thank you.

@kartlee

This comment has been minimized.

Copy link
Author

@kartlee kartlee commented Feb 25, 2015

@alexbrainman This works great...Thanks for quickly addressing it.

-Karthik

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Feb 25, 2015

@cookieo9 fix it. Thank him.

Alex

@mikioh mikioh modified the milestones: Go1.5Maybe, Go1.5 Feb 25, 2015
@kartlee

This comment has been minimized.

Copy link
Author

@kartlee kartlee commented Feb 26, 2015

@cookieo9 Thanks for the fix.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Mar 2, 2015

@kartlee I also added new hostname test on windows https://go-review.googlesource.com/6070 Can you, please, update your source and try the test on pdx-srv2012-wv01 ? I am interested if test fail - see what your hostname command says. Thank you.

Alex

@kartlee

This comment has been minimized.

Copy link
Author

@kartlee kartlee commented Mar 3, 2015

@alexbrainman Sorry for not getting back earlier. I will run them today and update the ticket.

BTW, we are still using go 1.2 and doesn't want to upgrade for our current product release, so I just applied the code upstream into our custom package's Hostname() and added a similar nightly build test to inform when the result between os.Hostname(..) and custom package's Hostname(..) match for machine name greater than 15 characters. This will help us to move to your change with go 1.5.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Mar 3, 2015

Thank you for confirming.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.