code review 64230043: net, os: update packages to be compatible with An...

net, os: update packages to be compatible with Android (FOR DISCUSSION)§ IMPORTANT: This is just a quick and dirty implementation for discussion and it is based on minux's previous work https://codereview.appspot.com/6454055 Changes: - dnsconfig needs to fetch name servers from Android's System properties. /etc/resolv.conf does not exist in Android. - temp directory is not in /tmp but /data/local/tmp Differences from minux's solution: - No need for Android NDK - No need to allow cross-compilation with cgo since Issue 4714 - Fix dnsconfig with quick and dirty solution using /system/bin/getprop I did use this solution to cross compile binaries for Android from darwin

[ernesto, 2014-02-15 01:11:30]

Hello golang-codereviews@googlegroups.com (cc: bradfitz@golang.org, minux.ma@gmail.com), I'd like you to review this change to https://code.google.com/p/go

[dave_cheney.net, 2014-02-15 01:21:47]

A bunch of small comments. I don't have a position on android in the go std lib, but I think you can make the change less invasive. https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... File src/pkg/net/dnsconfig_android.go (right): https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:1: // Copyright 2009 The Go Authors. All rights reserved. 2014 https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:25: conf := new(dnsConfig) conf = &dnsConfig { servers: make([]string, 0, 3) .... } https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:27: conf.search = make([]string, 0) dont' need https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:34: for i := 1; i <= 4; i++ { i < 5 please make this a constant with a comment https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:35: out, err := exec.Command("/system/bin/getprop", fmt.Sprintf("net.dns%v", i)).Output() %d if you know it's an integer https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:36: m := len(out) delete https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:37: if err != nil { if err != nil || m < 2 { continue } https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:43: ip = string(out[:m-1]) this is a noop, should it be ip = string(out) https://codereview.appspot.com/64230043/diff/40001/src/pkg/net/dnsconfig_andr... src/pkg/net/dnsconfig_android.go:45: a := conf.servers couldn't all this be written as conf.servers = append(conf.servers, ip) https://codereview.appspot.com/64230043/diff/40001/src/pkg/os/file_android.go File src/pkg/os/file_android.go (right): https://codereview.appspot.com/64230043/diff/40001/src/pkg/os/file_android.go... src/pkg/os/file_android.go:1: // Copyright 2012 The Go Authors. All rights reserved. 2014 https://codereview.appspot.com/64230043/diff/40001/src/pkg/os/file_android.go... src/pkg/os/file_android.go:10: func TempDir() string { if dir := Getenv("TMPDIR"); dir != "" { return dir } return "/data/local/tmp" https://codereview.appspot.com/64230043/diff/40001/src/pkg/os/file_no_android.go File src/pkg/os/file_no_android.go (right): https://codereview.appspot.com/64230043/diff/40001/src/pkg/os/file_no_android... src/pkg/os/file_no_android.go:9: // TempDir returns the default directory to use for temporary files. This file is the same as the previous one, wouldn't it be simpler to reuse the same logic and have a constant. or even if dir := ... [ // got dir, return } // default case if runtime.GOOS == "android" { return "/data/..." } return "/tmp"

[minux1, 2014-02-15 01:50:57]

I don't know whether the property storage format is stable or not (i guess it's not), but I'd think it'd be nice for Go to directly query that from system rather than relying on getprop(1). If the format is not stable, perhaps our best way to go is to use libc, just like the Solaris port. Let's wait for Brad's opinion. 

[gobot, 2014-02-23 23:56:41]

R=golang-dev (assigned by dave@cheney.net)

[rsc, 2014-03-05 19:57:26]

R=close not for go 1.3