v icon indicating copy to clipboard operation
v copied to clipboard

epoch generation is based on timezone

Open prashanth-hegde opened this issue 2 years ago • 7 comments

Describe the bug

The epoch generation is considering timezone into account. This is inconsistent with the standard epoch implementation. In linux, you can generate an epoch by date +%s command, and that generates an epoch that's independent of timezone. Same with python's print(time.time()) method.

import time
println(time.now().unix_time())

Expected Behavior

time.now().unix_time() should generate the same value as Linux's date +%s command

Current Behavior

On a system that's on a different timezone than UTC, generate the epoch timestamp in V, and generate the unix timestamp in Linux. When you subtract one value from the other, you get a value that's the timezone difference in seconds.

Example: Run this script on a machine that's in a different timezone than GMT

import time
import os

fn main() {
	linux_epoch := os.execute("date +%s").output.trim(r'\s').i64()
	v_epoch := time.now().unix_time()
	diff := linux_epoch - v_epoch
	println("linux_poch = ${linux_epoch} | v_epoch = ${v_epoch} | diff = ${diff}")
}

And you will get a value that is equal to the timezone difference in seconds

Reproduction Steps

Run this script on a machine that's in a different timezone than GMT

import time
import os

fn main() {
	linux_epoch := os.execute("date +%s").output.trim(r'\s').i64()
	v_epoch := time.now().unix_time()
	diff := linux_epoch - v_epoch
	println("linux_poch = ${linux_epoch} | v_epoch = ${v_epoch} | diff = ${diff}")
}

And you will get a value that is equal to the timezone difference in seconds

Possible Solution

No response

Additional Information/Context

No response

V version

V 0.3.3 713c95f

Environment details (OS name and version, etc.)

MacOs Ventura M1

prashanth-hegde avatar Mar 26 '23 13:03 prashanth-hegde

If this were to change, the old behavior should be possible. Probably in a separate function.

l1mey112 avatar Mar 26 '23 13:03 l1mey112

I don't see a reason to keep the current way of generating epochs based on timezones. By definition, epoch is always for UTC

image

prashanth-hegde avatar Mar 26 '23 15:03 prashanth-hegde

Yes, @prashanth-hegde is correct.

The unix_time should convert the time to UTC first and then subtract 1970/1/1 from it to get the milliseconds. It will break the code, which mistakenly expected that unix_time should return milliseconds since 1970/1/1 in the local time zone. Some bugfixes have to break the old code, so that all code will work correctly. And there's a good chance, that there's code outside, which expects the proper behaviour of unix_time and which is broken now because of the bug.

prantlf avatar Mar 26 '23 23:03 prantlf

The current routine could be renamed to local_epoch, and a corrected epoch could be calculated.

JalonSolov avatar Mar 27 '23 00:03 JalonSolov

@JalonSolov - what would be a use case for local_epoch ? And how would we ever possibly convert a local_epoch to the correct time, because local_epoch would not include a timezone information (as it is i64), which is the root of the bug since we cannot convert this local epoch to the correct time.

prashanth-hegde avatar Mar 27 '23 12:03 prashanth-hegde

:man_shrugging: just a thought in case it was needed.

JalonSolov avatar Mar 27 '23 12:03 JalonSolov

I agree that local_epoch shouldn't be added, because it won't be usable. You'd have to store the original time zone together with it. The UTC timestamp was introduced exactly against this - so that you don't have to carry the TZ with it.

Instead of introducing a function without a clear use case, even suggesting a wrong usage and inventing a new term "local epoch", it's better to fix the unix_time and announce the change of its behaviour.

prantlf avatar Mar 27 '23 17:03 prantlf