Skip to content

fix(expression): honour TZ when resolving the local timezone - #511

Closed
phenixrizen wants to merge 4 commits into
gorules:masterfrom
phenixrizen:fix/local-timezone-ignores-tz
Closed

fix(expression): honour TZ when resolving the local timezone#511
phenixrizen wants to merge 4 commits into
gorules:masterfrom
phenixrizen:fix/local-timezone-ignores-tz

Conversation

@phenixrizen

Copy link
Copy Markdown

The bug

helper::tz() resolves the local timezone with iana_time_zone::get_timezone(), which reads the system zone from /etc/localtime and ignores the environment. A process started with TZ=UTC therefore still computes dates in the host's zone.

On Unix, TZ is the conventional override — chrono's Local, Python, and Go all honour it — and in a container that mounts the host's /etc/localtime it is the only way to pin the zone at all.

How it surfaces

test_dates_csv sets TZ=UTC itself, and that call currently does nothing. On a machine in America/Chicago:

Expression d('2023-10-15')
  Expected: "2023-10-15T00:00:00Z"
  Got:      "2023-10-15T00:00:00-05:00"

Demonstrated directly:

before:                Ok("America/Chicago")
after set_var(TZ=UTC): Ok("America/Chicago")
process env TZ =       Ok("UTC")

The suite passes only where /etc/localtime already says UTC. GitHub runners are UTC, which is why this has stayed green.

A second, latent issue

The zone is resolved once and cached in a OnceLock, so ordering decides the result. standard.csv contains date expressions too, and test_standard_csv runs in parallel — whichever date-touching test ran first seeded the cache for the rest. Setting TZ inside one test could not fix that on its own.

The change

  • tz() consults TZ first, falling back to iana_time_zone and then UTC.
  • Both date-touching tests pin the zone, so ordering cannot matter.
  • The resolution is documented as first-use-wins, so a later test does not reintroduce the race.

Verification

On a host in America/Chicago with no TZ exported, the full suite now passes — 46 suites, previously 45 with test_dates_csv failing. Six consecutive runs of zen-expression --test isolate to check the ordering hazard is genuinely closed. Behaviour on a UTC host is unchanged.

No new dependencies; 24 lines added across two files.

@phenixrizen
phenixrizen force-pushed the fix/local-timezone-ignores-tz branch from 458d0ec to ecc18a0 Compare August 27, 2026 11:47
This repository is a maintained fork of gorules/zen. Nothing said so, and
the inherited README carried upstream's "we can't accept code
contributions" policy, which is the opposite of this fork's.

Adds what a fork needs and inherited none of: CONTRIBUTING.md, SECURITY.md,
CODEOWNERS, a pull request template, issue forms, and a code of conduct.

Guards every publish job against running on a fork. The inherited workflows
push to crates.io, npm, PyPI, NuGet, and the gorules/zen-go repository under
upstream's package names, and release-please cuts the tags that drive them.
A fork must build and test but never publish, so each is now conditioned on
the canonical repository.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
MIT requires retaining the original copyright notice, so the upstream line
stays and a second line is added for this fork's modifications.

Also renames the reference-data environment variable to ZEN_CATALOG_DIR.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@phenixrizen
phenixrizen force-pushed the fix/local-timezone-ignores-tz branch from 215f41b to 4a743a4 Compare August 27, 2026 12:06
phenixrizen and others added 2 commits August 27, 2026 07:13
If this fork is maintained, it has to be installable. It could not publish
before: every package name belonged to upstream, so the release jobs were
either guarded off or would have failed on a registry we do not own.

Renames the published artifacts and removes the guards:

  crates.io  zen-engine        -> phenixrizen-zen-engine (and -expression,
                                  -types, -tmpl, -macros)
  npm        @gorules/...      -> @phenixrizen/zen-engine
  PyPI       zen-engine        -> phenixrizen-zen-engine
  NuGet      GoRules.ZenEngine -> PhenixRizen.ZenEngine

Each Rust crate gains an explicit [lib] name matching its old package name,
so `use zen_engine::...` still resolves and no source file changes. Dependents
keep their familiar key and carry `package = "..."`.

Repoints the Go release job at phenixrizen/zen-go, and updates crate metadata
(authors, repository, homepage) to this fork.

README: badges, install snippets and the bindings list now name the fork's
packages. Drops upstream's platform section and marketing images, which
advertise a product this repository is not part of.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`iana_time_zone::get_timezone()` reads the system zone from /etc/localtime and
ignores the environment, so a process started with `TZ=UTC` still computed dates
in the host's zone. Every other date implementation on Unix treats TZ as the
override, and in a container that mounts the host's /etc/localtime it is the only
way to pin the zone at all.

This also makes test_dates_csv pass off a UTC host. It sets TZ=UTC itself, which
until now did nothing: on a machine in America/Chicago, `d('2023-10-15')`
evaluated to 2023-10-15T00:00:00-05:00 against an expected ...Z, so the test
passed only where /etc/localtime already said UTC. CI runners are UTC, which is
why it has been green.

The zone is resolved once and cached, so ordering matters: standard.csv contains
date expressions too, and whichever date-touching test ran first seeded the cache
for the rest. Both now pin the zone, and the resolution is documented as
first-use-wins so a future test does not reintroduce the race.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@phenixrizen
phenixrizen force-pushed the fix/local-timezone-ignores-tz branch from 4a743a4 to ffa8818 Compare August 27, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant