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: tzinfo for Thailand not present #26029

Closed
zhexuany opened this issue Jun 24, 2018 · 11 comments
Closed

time: tzinfo for Thailand not present #26029

zhexuany opened this issue Jun 24, 2018 · 11 comments

Comments

@zhexuany
Copy link

zhexuany commented Jun 24, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/zhexuany/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/zhexuany/repo/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10.3/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10.3/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/t9/s5qvdzpx3h5g58xzpm5gn8l00000gn/T/go-build676470505=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

In Thailand, I ran the following code:

package main

import (
        "fmt"
	"time"
	)

func main() {
	t := time.Now()
	fmt.Println(t.String())
}

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

2018-06-24 12:17:01.930111356 +0700 ICT m=+0.000337067

What did you see instead?

2018-06-24 12:17:01.930111356 +0700 +07 m=+0.000337067

@zhexuany
Copy link
Author

zhexuany commented Jun 24, 2018

Thailand is currently using timezone called ICT. I, personally, feel like it is better to return time(in string, of course) with a timezone name ICT rather than +07.

@zhexuany zhexuany changed the title time.Now gives me unexpected result in Thailand. time.Now() gives me unexpected result in Thailand. Jun 24, 2018
@agnivade
Copy link
Contributor

We just updated the vendored tzdata a couple of days ago. Is it possible for you to try with the master branch ?

@agnivade agnivade added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 24, 2018
@agnivade agnivade changed the title time.Now() gives me unexpected result in Thailand. time: tzinfo for Thailand not present Jun 24, 2018
@agnivade
Copy link
Contributor

NVM, just checked src/time/zoneinfo_abbrs_windows.go. It's not there.

@bradfitz - Should we update that file after bb222cd ?

@agnivade agnivade removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 24, 2018
@ALTree
Copy link
Member

ALTree commented Jun 24, 2018

This is working as intended.

The name of the timezone is UTC+07:00. The invented three-letter abbreviations are being removed from the tzdata database in favour of the offset names (+07, in this case). We didn't decide this, it's the policy that has been adopted upstream. It started in the 2017a release.

Additionally, as of tzdata-2017a there has been a policy of removal of zone abbreviations where these abbreviations have no official standing and were invented for convenience.

The tzdata 2017a changelog says:

Switch to numeric time zone abbreviations for South America, as
part of the ongoing project of removing invented abbreviations.
[...] Similarly, switch from invented to numeric time zone
abbreviations for
[...] Indochina [...]

http://mm.icann.org/pipermail/tz-announce/2017-February/000045.html

@zhexuany
Copy link
Author

zhexuany commented Jun 24, 2018

Ah, I did not notice this change.

Under this case, maybe we may need change the behavior time.Parse("20060102-15:04:05 -0700 MST", "20180624 12:17:01 +0700 +07")?

Below is a snippet to reproduce

package main

import (
        "fmt"
	"time"
	)

func main() {
	if _, err := time.Parse("20060102-15:04:05 -0700 MST", "20180624-12:17:01 +0700 +07"); err != nil {
		fmt.Println(err)
	}
}

It returns a error complaining parsing time "20180624-12:17:01 +0700 +07" as "20060102-15:04:05 -0700 MST": cannot parse "+07" as "MST"

@agnivade
Copy link
Contributor

Ah thanks for the clarification @ALTree.

Also, please correct me if I am wrong - don't we need to re-run genzabbrs.go after updating the zoneinfo.zip file ?

@ALTree
Copy link
Member

ALTree commented Jun 24, 2018

@zhexuany This is #24071 and it's already fixed on tip (see CL 98157). The code above doesn't print an error in go1.11 (to be released soon).

@ALTree
Copy link
Member

ALTree commented Jun 24, 2018

@agnivade Possibly? It looks like it's only needed for windows and I don't use it so I'm not sure... @alexbrainman committed all that contraption. Alex, can you help us? When should genzabbrs.go be run?

I just tried it and git diff shows several changes, so it appears that the zoneinfo_abbrs_windows.go may indeed be outdated.

@zhexuany
Copy link
Author

gotcha. Closing this issue since such bug was already fixed. A lot thanks to @ALTree

@alexbrainman
Copy link
Member

@alexbrainman committed all that contraption

Guilty as charged.

When should genzabbrs.go be run?

You can run it any time. It uses current version of http://unicode.org/cldr/data/common/supplemental/windowsZones.xml and your $GOROOT/lib/time/zoneinfo.zip file.

Alex

@ALTree
Copy link
Member

ALTree commented Jun 24, 2018

@alexbrainman thanks! I sent a change: https://go-review.googlesource.com/c/go/+/120559

@golang golang locked and limited conversation to collaborators Jun 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants