Reading the specs, I also think at some point soon it may make sense to split the specs up into subdirs so as to keep them focused on segments of the Socket namespace, but that's a future decision.

I believe it would be better for the name to be Socket.hostname, following naming conventions of not using "get" for getters.

@lbguilherme That might make sense, how would that relate to a scenario in which someone is setting a hostname?
Socket.sethostname is very clear as to what to expect the outcome should be.

Unless we think that Socket.hostname returns current hostname and Socket.hostname("foobar.com") is an acceptable approach?

The latter seems more confusing to me.

Perhaps Socket is the wrong home for it after all. Shall we introduce a System module or so for this kind of stuff? /cc @crystal-lang/crystallers Anybody wants to see what existent stuff could be moved there? I can imagine there's some on Process already.

Another name I can think of is Socket.current_hostname. sethostname(2) would be mapped to Socket.current_hostname=(new_name), if we ever decide to do it.

In any case, this functionality is blocking and maybe it should be non-blocking (use libevent, or some other mechanism), so this will have to wait untli @waj can approve this (he's on vacations for a few weeks now)

@asterite It is a blocking system call, but it is not network dependent. It simply reads system configuration. It should usually be the same as the nodename property of the uname(2) call from my understanding.

@jhass Oh, my mistake, I confused this with something like getaddrinfo.

I agree that Socket.hostname is probably not the best place to put this. I always check how other languages provide this. Go has it in the os package. C# with Dns.GetHostName. In Java it's java.net.InetAddress.getLocalHost().getHostName() (of course).

Go also has Getpid, Getuid and others in os package, and we (and Ruby) has those in Process.

So maybe Process.hostname and Process.hostname=? That was actually my initial though when I read the man page for gethostname:

The gethostname() function returns the standard host name for the current processor

Well, Process is certainly different than Processor, and hostname doesn't change between processes, but since it's a class method I think it's not that confusing. We could add a System or OS class/module, but Process is already there. Thoughts?

POSIX words it as

The gethostname() function shall return the standard host name for the current machine

I don't like it on Process, it should keep at stuff specific to a process, not to a machine. I would prefer Machine or System, os always felt like an odd hodgepodge in Python, so it might attract too different stuff.

System sounds good, but I'd like to find more things to put in it. If we'll end up with just System.hostname then I still prefer to use Process or even Socket for this (python too uses socket). Or maybe we can just use System and later use it if we ever need more system-related functionality.

I see this as my first foray into LibC here, so wanted to get the
discussion going.
I foresee a large variety of system-related functions that we should be
able to call, irrespective of their underlying POSIX namespaces.
I'm in favor of a System. class, and can port it over there.
I originally chose Socket to be similar to Ruby.

On Thu, May 26, 2016, 09:54 Ary Borenszweig notifications@github.com
wrote:

System sounds good, but I'd like to find more things to put in it. If
we'll end up with just System.hostname then I still prefer to use Process
or even Socket for this (python too uses socket
https://docs.python.org/3/library/socket.html#socket.gethostname). Or
maybe we can just use System and later use it if we even need more
system-related functionality.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#2649 (comment)

OK, let's use System 😸

I was planning on releasing 0.17.4 today, which contains a few goodies and many fixes. I'd like to include this too because it's backwards compatible and generally useful.

We have to create a system.cr file and require it in prelude.cr. system.cr must require "c/sys/socket".

We have these options:

1. You update the code and make a PR targetting the release/0.17 branch (though I guess it can target master and then I can merge it in release/0.17, @jhass ?)
2. I merge this PR into release/0.17 and then move the method to the System module
3. I commit directly into release/0.17, thanking you (I'll of course mention you in the Changelog, as usual)

@asterite I've updated the PR, let's hope CI doesn't bite me. :)

• I wasn't certain if there was an "order" to prelude.cr - so I appended to the end. Does order matter in this file? If so, we should document it inline. Otherwise, alpha-sort makes me happiest.
• The specs passed without the require "c/sys/socket" - os that getting loaded in some other manner?

I'm happy to send a PR to any branch you like.

@miketheman Thanks!

Order used to matter, but now it matters less and less. It only matters if macros are used (macros need to be defined before they are used), but a first compiler pass gathers all classes and methods, so in this case it's OK to put it in any order.

It seems libevent requires c/netdb, which in turns requires c/sys/socket, so that's why it works (libevent is always required). In any case, it's better to have a specific require in case we remove libevent or we change its dependencies.

@asterite awesome - I think I'll add a note to prelude.cr in another PR about ordering.

For this one, all tests pass, the world didn't explode, is there any further steps?

No, thank you!

Woohoo, thanks!