-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
add timetzdata
build tag to binary releases
#33463
base: main
Are you sure you want to change the base?
add timetzdata
build tag to binary releases
#33463
Conversation
Thank you very much for the details. I just found a Go issue: According to the Go issue 38017, it seems that the problem is mainly related to Windows. Maybe we could add a |
Some more context from https://pkg.go.dev/time/tzdata:
We know some versions of Windows are affected, but it may affect others as well, like for example weird BSD variants etc. I think it's better to always include the fallback tzdata info. 450kB seems acceptable. |
Oh, and I think it would be great if we specify such default tags in Makefile instead of the actions, so that people who build from source can also benefit: Line 136 in 47bf836
|
Why pay the cost which doesn't really affect?
Why not just use |
Because you can't know for certain whether a given OS has tzdata available. Let's say I compile a barebone Linux distro like Arch or Gentoo and forget to install that package. Gitea would not start up. |
No, it WOULD start up. No timezone data affects nothing as long as the end user doesn't set it in their config. |
You mean #33235 (comment) does not happen in default config? Well, even if that's true, I think it's still good to include those 450kB just in case. This is only a 0.5% size increase assuming a 100mb binary. |
Your worry will never happen. Only Windows users need that timezone data.
|
updated per @wxiaoguang feedback. I chose settings module as location for import since we already have other conditional imports there, such as sqlite. |
…ogick/gitea into techknowlogick-patch-7
I will add some comments |
timetzdata
is already used in the docker images, this includes them for the binary release files too.Related to #33235 (I don't have a windows machine setup to test this though)