code review 142200044: net: allow all unix /etc/* pathes to be assignable.

net: allow all unix /etc/* pathes to be assignable.

This fixes a major problem when compiling for Android or any other unix-based system that doesn't use the default /etc/{resolv.conf,hosts,protocols,services} pathes and it doesn't affect anything else at all.

Fixes issue 8020.
Fixes issue 8765.

[oneofone, 2014-09-17 22:36:37]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[0xjnml, 2014-09-18 08:40:35]

On 2014/09/17 22:36:37, oneofone wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go

What is the expected outcome when package A in its init() assigns value V to one
of those paths and package B in its init() assigns value W to the same variable?

I suggest to use foo_platform.go or baz_os.go instead for specialization of
those values.

-j

[oneofone, 2014-09-18 12:41:44]

On 2014/09/18 08:40:35, 0xjnml wrote:
> On 2014/09/17 22:36:37, oneofone wrote:
> > Hello mailto:golang-codereviews@googlegroups.com,
> > 
> > I'd like you to review this change to
> > https://code.google.com/p/go
> 
> What is the expected outcome when package A in its init() assigns value V to
one
> of those paths and package B in its init() assigns value W to the same
variable?
We could add a big warning for it, that it is the callers responsibility to
check and assign the value before anything calls net.*.

For example:
func init() {
    if net.ResolvConfPath == "/etc/resolv.conf" { ... }
}

sync/atomic-style, don't play with it unless you know what you're doing, and
that will fix the problem for the few people who actually needs it.

> I suggest to use foo_platform.go or baz_os.go instead for specialization of
> those values.
I'm not sure I fully understand what you mean by this.
It would still be impossible to use if a linux distro doesn't have /etc/*.
That happens on embedded distros and android.
> -j

I understand this solution isn't perfect, but the alternatives are either:

1. Use a different dns package.
2. Patch /net/ every time you update and have separate versions for different
targets.
3. Or completely redo how this works so it would be an assignable func that
returns []byte or an io.Reader, this would actually be the proper fix for
android since you'd need to call `getprop net.dns*` anyway.

I'm open for suggestions.

[iant, 2014-09-18 18:25:21]

It sounds like, in Go terms, this varies only based on GOOS, so it seems to me
we should different values in files using build tags based on GOOS.

To be it another way, is there a reason to expand the package API here?  Are
there systems where these files live in multiple locations only known at run
time?

[oneofone, 2014-09-18 19:19:32]

Besides android and my nameless cheap Chinese router, not that I'm aware of.

But the point is instead of having multiple files in the runtime for
different OS's, we would let the end user worry about it.

And it's 100℅ safe since it doesn't break any current code.
On Sep 18, 2014 9:25 PM, <iant@golang.org> wrote:

> It sounds like, in Go terms, this varies only based on GOOS, so it seems
> to me we should different values in files using build tags based on
> GOOS.
>
> To be it another way, is there a reason to expand the package API here?
> Are there systems where these files live in multiple locations only
> known at run time?
>
>
> https://codereview.appspot.com/142200044/
>

[iant, 2014-09-18 19:44:13]

On Thu, Sep 18, 2014 at 12:19 PM, Ahmed Waheed <oneofone@gmail.com> wrote:
> Besides android and my nameless cheap Chinese router, not that I'm aware of.
>
> But the point is instead of having multiple files in the runtime for
> different OS's, we would let the end user worry about it.
>
> And it's 100℅ safe since it doesn't break any current code.

I agree that it doesn't break any code, but it adds an API.  The Go 1
guarantee means that we will have to document and maintain that API
forever.  I don't want to do that if we don't have to.

Ian

[rsc, 2014-09-25 18:29:56]

NOT LGTM

These are Unix details leaking into the public API of the not-Unix-specific net
package.

And certainly not at this stage in the release cycle.

[gobot, 2014-12-19 05:13:22]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/142200044 is best) in the description in your
new CL.

Thanks very much.