Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: TZ=UTC does not cause Location field to be nil #19202

Closed
pzinovkin opened this issue Feb 20, 2017 · 19 comments

Comments

Projects
None yet
8 participants
@pzinovkin
Copy link

commented Feb 20, 2017

What did you do?

package main

import (
	"fmt"
	"os"
	"time"
)

func main() {
	fmt.Println("TZ:", os.Getenv("TZ"))
	fmt.Printf("%#v\n", time.Now())
	fmt.Printf("%#v\n", time.Now().UTC())
}
bash-3.2$ TZ="UTC" go run tt.go
TZ: UTC
time.Time{sec:63623205117, nsec:63974400, loc:(*time.Location)(0x110e4e0)}
time.Time{sec:63623205117, nsec:64010002, loc:(*time.Location)(nil)}

What did you expect to see?

Both strings to contain nil Location.

System details

go version go1.8 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pzinovkin"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/fv/0mdc02116kn485ml31tvs26w0000gn/T/go-build085816660=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.8 X:framepointer
uname -v: Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.12.3
BuildVersion:	16D32
lldb --version: lldb-360.1.70
@peterGo

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2017

src/time/zoneinfo.go:

// NOTE(rsc): Eventually we will need to accept the POSIX TZ environment
// syntax too, but I don't feel like implementing it today.
@pzinovkin

This comment has been minimized.

Copy link
Author

commented Feb 20, 2017

Isn't it already supported?
https://github.com/golang/go/blob/master/src/time/zoneinfo_unix.go#L56

Anyway it should fallback to UTC. And UTC must contain nil loc like in second string.

@ianlancetaylor ianlancetaylor changed the title time.Now() does not respect TZ env variable time: TZ=UTC does not cause Location field to be nil Feb 20, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2017

Your program clearly shows that the time package does respect the TZ environment variable.

I think the question is: should the time package require that TZ=UTC cause a local Time value to have a nil Location field? The docs do say that "All UTC times are represented with loc==nil, never loc==&utcLoc." The question is whether that applies when the local time zone is UTC.

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Feb 20, 2017

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Feb 20, 2017

Your program clearly shows that the time package does respect the TZ environment variable.

I'm not sure about that.

bash-3.2$ go version
go version go1.7.5 darwin/amd64
bash-3.2$ cat tt.go
package main

import (
	"fmt"
	"os"
	"time"
)

func main() {
	fmt.Println("TZ:", os.Getenv("TZ"))
	fmt.Printf("%#v\n", time.Now().Location())
}
bash-3.2$ TZ="MSK" go run tt.go
TZ: MSK
&time.Location{name:"", zone:[]time.zone(nil), tx:[]time.zoneTrans(nil), cacheStart:0, cacheEnd:0, cacheZone:(*time.zone)(nil)}
bash-3.2$ TZ="UTC" go run tt.go
TZ: UTC
&time.Location{name:"", zone:[]time.zone(nil), tx:[]time.zoneTrans(nil), cacheStart:0, cacheEnd:0, cacheZone:(*time.zone)(nil)}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2017

@pzinovkin That is not the right test. Try actually fetching some field from the location first. For example, try adding

fmt.Println(time.Now().Location())

(which will call the String method) before you print out the contents of the location.

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Feb 20, 2017

You're right. Thanks.

bash-3.2$ cat tt.go
package main

import (
	"fmt"
	"os"
	"time"
)

func main() {
	fmt.Println("TZ:", os.Getenv("TZ"))
	fmt.Println(time.Now())
	fmt.Println(time.Now().Location())
}
bash-3.2$ TZ="ROK" go run tt.go
TZ: ROK
2017-02-21 04:16:47.677760884 +0900 KST
ROK
bash-3.2$ TZ="" go run tt.go
TZ:
2017-02-20 19:16:51.373521089 +0000 UTC
UTC
bash-3.2$ TZ="Asia/Irkutsk" go run tt.go
TZ: Asia/Irkutsk
2017-02-21 03:17:54.094113655 +0800 +08
Asia/Irkutsk

@bradfitz bradfitz closed this Jun 7, 2017

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Jun 8, 2017

@bradfitz is this issue already resolved?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 8, 2017

Sorry, I quickly interpreted the "You're right" as the issue no longer existing.

On second read, it looks like we're still trying to determine whether Location should always be nil, like @ianlancetaylor says in #19202 (comment):

The question is whether that applies when the local time zone is UTC.

@bradfitz bradfitz reopened this Jun 8, 2017

@admtnnr

This comment has been minimized.

Copy link

commented Jun 8, 2017

This looks like #19486?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

Yes, I think Russ addresses this issue in #19486 when he said "Go always maintains a distinction between local and UTC times, even when TZ=UTC in the environment."

I'm going to close this again. If you think this is wrong, please follow up with some code that shows a problem that is not related to internal details of the time package.

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Jun 8, 2017

Well, I guess we need to start setting time to UTC() explicitly then. Because setting TZ=UTC won't work once we need to compare time.Now() and time.Now().UTC().
I guess this distinction should be clearly stated in docs to avoid confusion.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

What do you mean by "compare?" Comparing using time.Equal ignores the location anyhow. You should normally not use == to compare two time.Time values.

(Edited to add a missing "not".)

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

@ianlancetaylor did you mean "you should normally not use =="?

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Jun 8, 2017

What about reflect.DeepEqual when time in some struct?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

Argh, yes, I mean you should not use ==. I edited the comment.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

@pzinovkin Yes, reflect.DeepEqual does not work correctly for times stored in structs, as is true of many other types as well. See #20519, #20444, and probably others.

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Jun 9, 2017

Yes, I think Russ addresses this issue in #19486 when he said "Go always maintains a distinction between local and UTC times, even when TZ=UTC in the environment."

Addressed or stated the issue? Please see https://go-review.googlesource.com/c/31144
From my perspective reflect.DeepEqual with time worked fine prior go1.8. I guess it got broken after CL above.
Question is should Location be nil when env TZ=UTC is set?
@rsc can you clarify?

@pzinovkin

This comment has been minimized.

Copy link
Author

commented Jun 9, 2017

Also consider this example:

$ cat tt.go
package main

import (
	"fmt"
	"os"
	"reflect"
	"time"
)

func main() {
	loc, _ := time.LoadLocation(os.Getenv("TZ"))

	fmt.Println("TZ:", os.Getenv("TZ"), "Location:", loc)

	t := time.Now()

	tu := time.Date(t.Year(), t.Month(), t.Day(),
		t.Hour(), t.Minute(), t.Second(), t.Nanosecond(), loc)

	fmt.Printf("%v\n%v\n -> %t", t, tu, reflect.DeepEqual(t, tu))
}

$ TZ="UTC" go run tt.go
TZ: UTC Location: UTC
2017-06-09 11:05:35.076609043 +0000 UTC
2017-06-09 11:05:35.076609043 +0000 UTC
 -> false

$ TZ="Asia/Irkutsk" go run tt.go
TZ: Asia/Irkutsk Location: Asia/Irkutsk
2017-06-09 19:05:31.133827158 +0800 +08
2017-06-09 19:05:31.133827158 +0800 +08
 -> true
@rsc

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2017

Question is should Location be nil when env TZ=UTC is set?

No. time.Now().Location() == time.Local and time.Local != time.UTC, even when TZ=UTC.

@golang golang locked and limited conversation to collaborators Jun 9, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.