headzoo / surf

Stateful programmatic web browsing in Go.
MIT License
1.49k stars 160 forks source link

syscall.Utsname is OS dependent. #4

Closed shirou closed 9 years ago

shirou commented 10 years ago

agent/agent.go uses syscall.Utsname{} https://github.com/headzoo/surf/blob/master/agent/agent.go#L363

But this is OS dependent. not working Darwin (go 1.3.1).

Please use uname command like

    import  "os/exec"

    out, err := exec.Command("uname", "-s").Output()
    if err != nil {
        return "Linux"
    }
    return strings.ToLower(strings.TrimSpace(string(out)))

uname -s for osName and uname -r for osVersion.

However, uname -r shows the 'linux' version. But from agent_test.go, an expected string is a distribution version like 14.04 for Ubuntu 14.04.

If you want to get a distribution version, you may use https://github.com/shirou/gopsutil and HostInfo().

headzoo commented 10 years ago

Using gopsutil would probably be the best option, as uname is also OS dependent. gopsutil may be overkill for what we're trying to accomplish, but I suspect other uses for the library will be found later.

shirou commented 10 years ago

uname system call and command is specified by POSIX and most UNIX-like OS include. Some options are different but -s and -r are common. see http://en.wikipedia.org/wiki/Uname.

My point was, osVersion in the agent.go should return "distribution version(ex: 14.10)" not "os kernel version (ex: 3.8.0-25)". But I search UserAgent example one more time, it seems "os kernel version" is widely used. If so, uname -r is the best solution I think.

xiaods commented 9 years ago

@shirou how about give a patch?

shirou commented 9 years ago

@xiaods Thank you for the patch. As I said, uname is returns only the OS version, not distribution. But it's enough I think.

xiaods commented 9 years ago

@shirou All honor is yours.

mholt commented 9 years ago

I'm probably missing something here, but what's wrong with using runtime.GOOS? (Also getting build failures in Mac OS over here...)

xiaods commented 9 years ago

@mholt see my patch.

mholt commented 9 years ago

@xiaods I saw it, but I'm still not grasping its benefit over runtime.GOOS. Isn't uname still platform-dependent?

xiaods commented 9 years ago

@mholt yes, you are right, use runtime.GOOS should be elegant way, need a patch refresh it.

mholt commented 9 years ago

Should be pretty simple. If I get a chance this week I'll look at it (but no promises: final exams are around the corner).

headzoo commented 9 years ago

I wish I could be more help here, but I'm also tied up with work and family until after the holidays.

tatsushid commented 9 years ago

I wrote an another patch, without dependency on uname command

xiaods commented 9 years ago

i prefer this #7 patch.

shirou commented 9 years ago

My original concern is OS dependent. Thank you for the xiaods but I also prefer #7.