-
Notifications
You must be signed in to change notification settings - Fork 61
[RFC] :LEGACY typezones with deprecation warnings.
#504
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,14 +39,27 @@ US/Pacific (UTC-8/UTC-7) | |
| TimeZone(::AbstractString, ::Class) | ||
|
|
||
| function TimeZone(str::AbstractString, mask::Class=Class(:DEFAULT)) | ||
| tz, class = get(_TZ_CACHE, str) do | ||
| tz, class, link = get(_TZ_CACHE, str) do | ||
| if occursin(FIXED_TIME_ZONE_REGEX, str) | ||
| FixedTimeZone(str), Class(:FIXED) | ||
| FixedTimeZone(str), Class(:FIXED), InlineString31("") | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, a |
||
| else | ||
| throw(ArgumentError("Unknown time zone \"$str\"")) | ||
| end | ||
| end | ||
|
|
||
| # Auto-redirect LEGACY timezones to their modern equivalents | ||
| # Only when user hasn't explicitly opted in to LEGACY class | ||
| if !isempty(link) && class == Class(:LEGACY) && mask & Class(:LEGACY) == Class(:NONE) | ||
| # Note: Using depwarn here allows users to control behavior via --depwarn flag. | ||
| # With --depwarn=error, this becomes an error (strict mode). | ||
| # This matches the behavior requested in issue #469 https://github.com/JuliaTime/TimeZones.jl/issues/469#issuecomment-2341741754. | ||
| Base.depwarn( | ||
| "The time zone \"$str\" is deprecated, using \"$link\" instead.", | ||
| :TimeZone | ||
| ) | ||
| return TimeZone(String(link), mask) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE: If the |
||
| end | ||
|
|
||
| if mask & class == Class(:NONE) | ||
| throw(ArgumentError( | ||
| "The time zone \"$str\" is of class `$(repr(class))` which is " * | ||
|
|
@@ -83,7 +96,10 @@ function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT)) | |
| return true | ||
| end | ||
|
|
||
| # Checks against pre-compiled time zones | ||
| class = get(() -> (UTC_ZERO, Class(:NONE)), _TZ_CACHE, str)[2] | ||
| # Checks against pre-compiled time zones (3-tuple now: tz, class, link) | ||
| _, class, link = get(() -> (UTC_ZERO, Class(:NONE), InlineString31("")), _TZ_CACHE, str) | ||
|
|
||
| # Allow linked legacy timezones to auto-redirect | ||
| !isempty(link) && class == Class(:LEGACY) && mask & Class(:LEGACY) == Class(:NONE) && return true | ||
| return mask & class != Class(:NONE) | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| # Use a separate cache for FixedTimeZone (which is `isbits`) so the container is concretely | ||
| # typed and we avoid allocating a FixedTimeZone every time we get one from the cache. | ||
| # Note: link uses InlineString31 (not Union{InlineString31,Nothing}) to keep tuples isbits. | ||
| # An empty string "" is used as a sentinel value for "no link target". | ||
| struct TimeZoneCache | ||
| ftz::Dict{String,Tuple{FixedTimeZone,Class}} | ||
| vtz::Dict{String,Tuple{VariableTimeZone,Class}} | ||
| ftz::Dict{String,Tuple{FixedTimeZone,Class,InlineString31}} | ||
| vtz::Dict{String,Tuple{VariableTimeZone,Class,InlineString31}} | ||
| lock::ReentrantLock | ||
| initialized::Threads.Atomic{Bool} | ||
| end | ||
|
|
@@ -28,12 +30,16 @@ function reload!(cache::TimeZoneCache, compiled_dir::AbstractString=_COMPILED_DI | |
| empty!(cache.vtz) | ||
|
|
||
| walk_tz_dir(compiled_dir) do name, path | ||
| tz, class = open(TZJFile.read, path, "r")(name) | ||
| tz, class, link = open(TZJFile.read, path, "r")(name) | ||
|
|
||
| # Convert link to InlineString31 to keep tuples isbits | ||
| # Use empty string as sentinel for "no link target" | ||
| entry = (tz, class, link === nothing ? InlineString31("") : InlineString31(link)) | ||
|
|
||
| if tz isa FixedTimeZone | ||
| cache.ftz[name] = (tz, class) | ||
| cache.ftz[name] = entry | ||
| elseif tz isa VariableTimeZone | ||
| cache.vtz[name] = (tz, class) | ||
| cache.vtz[name] = entry | ||
| else | ||
| error("Unhandled TimeZone class encountered: $(typeof(tz))") | ||
| end | ||
|
|
@@ -63,10 +69,17 @@ function Base.get(body::Function, cache::TimeZoneCache, name::AbstractString) | |
| end | ||
|
|
||
| # Build specific tzdata version if specified by `JULIA_TZ_VERSION` | ||
| function _build() | ||
| desired_version = TZData.tzdata_version() | ||
| if desired_version != TZJData.TZDATA_VERSION | ||
| _COMPILED_DIR[] = TZData.build(desired_version, _scratch_dir()) | ||
| # Also rebuilds if the TZJFile format version doesn't match the expected version | ||
| function _build(tzjf_version::Integer=TZJFile.tzjfile_version()) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another main hack is for switching compiled file formats. |
||
| expected_dir = TZData.compiled_dir() | ||
|
|
||
| # Rebuild if the expected directory doesn't exist | ||
| # TODO: I believe this currently avoids using TZJData.jl and forces a rebuild, but makes testing V1 vs V2 behaviour easier | ||
| if !isdir(expected_dir) | ||
| _COMPILED_DIR[] = TZData.build(TZData.tzdata_version(), _scratch_dir(); tzjf_version) | ||
| elseif _COMPILED_DIR[] != expected_dir | ||
| # Expected directory exists but we're pointing to wrong location (e.g., artifact) | ||
| _COMPILED_DIR[] = expected_dir | ||
| end | ||
|
|
||
| return nothing | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,36 @@ using Dates: Dates, DateTime, Second, datetime2unix, unix2datetime | |
| using ...TimeZones: FixedTimeZone, VariableTimeZone, Class, Transition | ||
| using ...TimeZones.TZFile: combine_designations, get_designation, timestamp_min | ||
|
|
||
| const DEFAULT_VERSION = 1 | ||
| const DEFAULT_VERSION = 2 | ||
|
|
||
| """ | ||
| tzjfile_version() -> Int | ||
|
|
||
| Returns the TZJFile format version to use, controlled by the `JULIA_TZJ_VERSION` environment | ||
| variable. If not set, defaults to `DEFAULT_VERSION` (currently $DEFAULT_VERSION). | ||
|
|
||
| This allows users to opt-in or opt-out of new file format versions for testing or | ||
| compatibility purposes. | ||
|
|
||
| # Examples | ||
| ```julia | ||
| # Use default version (currently 2) | ||
| julia> TZJFile.tzjfile_version() | ||
| 2 | ||
|
|
||
| # Use version 1 via environment variable | ||
| julia> ENV["JULIA_TZJ_VERSION"] = "1" | ||
| julia> TZJFile.tzjfile_version() | ||
| 1 | ||
| ``` | ||
| """ | ||
| function tzjfile_version() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, a handy feature for switching compiled file versions, but it could be removed. |
||
| version_str = get(ENV, "JULIA_TZJ_VERSION", string(DEFAULT_VERSION)) | ||
| version = tryparse(Int, version_str) | ||
| version === nothing && error("Invalid JULIA_TZJ_VERSION: \"$version_str\". Must be an integer (1 or 2).") | ||
| version ∉ (1, 2) && error("Unsupported JULIA_TZJ_VERSION: $version. Must be 1 or 2.") | ||
| return version | ||
| end | ||
|
|
||
| include("utils.jl") | ||
| include("read.jl") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the version feature flag and path hacks for testing both v1 and v2 behaviour.