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
Closed

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

pzinovkin opened this issue Feb 20, 2017 · 19 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@pzinovkin
Copy link

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
Copy link
Contributor

peterGo 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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

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 as completed Jun 7, 2017
@pzinovkin
Copy link
Author

@bradfitz is this issue already resolved?

@bradfitz
Copy link
Contributor

bradfitz 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
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 8, 2017
@admtnnr
Copy link

admtnnr commented Jun 8, 2017

This looks like #19486?

@ianlancetaylor
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

ianlancetaylor 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
Copy link
Contributor

cespare commented Jun 8, 2017

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

@pzinovkin
Copy link
Author

What about reflect.DeepEqual when time in some struct?

@ianlancetaylor
Copy link
Contributor

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

@ianlancetaylor
Copy link
Contributor

@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
Copy link
Author

pzinovkin 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
Copy link
Author

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
Copy link
Contributor

rsc 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.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

8 participants