code review 7547044: net: make sure to use /tmp in unix domain socket tests

net: make sure to use /tmp in unix domain socket tests

[mikio, 2013-03-12 00:13:41]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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

[bradfitz, 2013-03-12 00:14:33]

Really?
 On Mar 11, 2013 5:13 PM, <mikioh.mikioh@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net: make sure to use /tmp in unix domain socket tests
>
> Please review this at
https://codereview.appspot.**com/7547044/<https://codereview.appspot.com/7547...
>
> Affected files:
>   M src/pkg/net/protoconn_test.go
>
>
> Index: src/pkg/net/protoconn_test.go
> ==============================**==============================**=======
> --- a/src/pkg/net/protoconn_test.**go
> +++ b/src/pkg/net/protoconn_test.**go
> @@ -15,9 +15,11 @@
>         "time"
>  )
>
> -// testUnixAddr uses ioutil.TempFile to get a name that is unique.
> +// testUnixAddr uses ioutil.TempFile to get a name that is unique. It
> +// also uses /tmp directory in case it is prohibited to create UNIX
> +// sockets in TMPDIR.
>  func testUnixAddr() string {
> -       f, err := ioutil.TempFile("", "nettest")
> +       f, err := ioutil.TempFile("/tmp", "nettest")
>         if err != nil {
>                 panic(err)
>         }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[dave_cheney.net, 2013-03-12 00:17:09]

Yup, so saith rsc, https://codereview.appspot.com/7703044#msg2

On Tue, Mar 12, 2013 at 11:14 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Really?
>
> On Mar 11, 2013 5:13 PM, <mikioh.mikioh@gmail.com> wrote:
>>
>> Reviewers: golang-dev1,
>>
>> Message:
>> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> net: make sure to use /tmp in unix domain socket tests
>>
>> Please review this at https://codereview.appspot.com/7547044/
>>
>> Affected files:
>>   M src/pkg/net/protoconn_test.go
>>
>>
>> Index: src/pkg/net/protoconn_test.go
>> ===================================================================
>> --- a/src/pkg/net/protoconn_test.go
>> +++ b/src/pkg/net/protoconn_test.go
>> @@ -15,9 +15,11 @@
>>         "time"
>>  )
>>
>> -// testUnixAddr uses ioutil.TempFile to get a name that is unique.
>> +// testUnixAddr uses ioutil.TempFile to get a name that is unique. It
>> +// also uses /tmp directory in case it is prohibited to create UNIX
>> +// sockets in TMPDIR.
>>  func testUnixAddr() string {
>> -       f, err := ioutil.TempFile("", "nettest")
>> +       f, err := ioutil.TempFile("/tmp", "nettest")
>>         if err != nil {
>>                 panic(err)
>>         }
>>
>>
>> --
>>
>> ---You received this message because you are subscribed to the Google
>> Groups "golang-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-dev+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[mikio, 2013-03-12 00:18:20]

On Tue, Mar 12, 2013 at 8:14 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> Really?

Not sure. But both Russ and Albert said so in
https://code.google.com/p/go/source/detail?r=7c763891c5df1a0d1025ecc0b0025f98....

I guess it happend before before Go 1 release, n 2010, perhaps.

[rsc, 2013-03-12 01:03:48]

LGTM

I often set TMPDIR to a subdirectory of my NFS home directory (weird, I
know). It finds lots of problems.
It's also possible this wasn't one of them, but consistency is a plus
either way.

[mikio, 2013-03-12 23:26:30]

*** Submitted as https://code.google.com/p/go/source/detail?r=299a890cf699 ***

net: make sure to use /tmp in unix domain socket tests

R=golang-dev, bradfitz, dave, rsc
CC=golang-dev
https://codereview.appspot.com/7547044

[brainman, 2013-03-13 02:14:15]

NOT LGTM

This will always panic on windows.

[ioe, 2013-03-13 03:45:53]

On 2013/03/12 01:03:48, rsc wrote:
> I often set TMPDIR to a subdirectory of my NFS home directory (weird, I
> know). It finds lots of problems.
> It's also possible this wasn't one of them, but consistency is a plus
> either way.

Are you sure remote TMPDIR is a setup many programs should support?
I have seen /var/tmp on NFS for embedded systems, but never TMPDIR on NFS.
Although POSIX, SuS and Linux FHS2.3 don't forbid it.

They mention a space to store files. It is nowhere limited to REGULAR files
only.

So I guess you rely on unspecified behavior in your setup.

It also breaks the assumptions of per-user temporary directories. Although this
might be also implemented with user-private mounts/binds instead. But then it
could be NFS in your case, too. So overriding TMPDIR won't help.

[mikio, 2013-03-13 08:04:35]

Looks like reverting this CL makes sense.
Sorry about crashes on both Windows and Plan 9.

On Wed, Mar 13, 2013 at 11:45 AM,  <nightlyone@googlemail.com> wrote:
> On 2013/03/12 01:03:48, rsc wrote:
>>
>> I often set TMPDIR to a subdirectory of my NFS home directory (weird,
>
> I
>>
>> know). It finds lots of problems.
>> It's also possible this wasn't one of them, but consistency is a plus
>> either way.
>
>
> Are you sure remote TMPDIR is a setup many programs should support?
> I have seen /var/tmp on NFS for embedded systems, but never TMPDIR on
> NFS. Although POSIX, SuS and Linux FHS2.3 don't forbid it.
>
> They mention a space to store files. It is nowhere limited to REGULAR
> files only.
>
> So I guess you rely on unspecified behavior in your setup.
>
> It also breaks the assumptions of per-user temporary directories.
> Although this might be also implemented with user-private mounts/binds
> instead. But then it could be NFS in your case, too. So overriding
> TMPDIR won't help.
>
> https://codereview.appspot.com/7547044/

[brainman, 2013-03-13 08:10:20]

Brad has fix https://codereview.appspot.com/7778043/ for plan9 and windows.

Alex

[mikio, 2013-03-13 08:11:54]

Ah, thanks, still skimming on -dev not reached yet.

On Wed, Mar 13, 2013 at 4:10 PM,  <alex.brainman@gmail.com> wrote:
> Brad has fix https://codereview.appspot.com/7778043/ for plan9 and
> windows.
>
> Alex
>
> https://codereview.appspot.com/7547044/