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

Date comparison fails with unqualified date values #12236

Closed
jmooring opened this issue Mar 13, 2024 · 9 comments · Fixed by #12239
Closed

Date comparison fails with unqualified date values #12236

jmooring opened this issue Mar 13, 2024 · 9 comments · Fixed by #12239

Comments

@jmooring
Copy link
Member

This worked as expected in v0.122.0, fails in v0.123.x.

site config

timeZone = "Europe/London"

content/p1.md

---
title: p1
date: 2024-03-13T06:00:00
---

layouts/_default/single.html

{{ .Date | warnf "date = %#v" }}
{{ .Lastmod | warnf "lastmod = %#v" }}
{{ eq .Date .Lastmod | warnf "%#v" }}

expected (v0.122.0)

WARN  date = time.Date(2024, time.March, 13, 6, 0, 0, 0, time.Location("Europe/London"))
WARN  lastmod = time.Date(2024, time.March, 13, 6, 0, 0, 0, time.Location("Europe/London"))
WARN  true

actual (v0.123.x)

WARN  date = time.Date(2024, time.March, 13, 6, 0, 0, 0, time.Location("Europe/London"))
WARN  lastmod = time.Date(2024, time.March, 13, 6, 0, 0, 0, time.UTC)
WARN  false

Both are correct (they point to the same moment in time), but the two values are not equal.

Reference: https://discourse.gohugo.io/t/if-eq-date-lastmod-0-123-8/48798

@bep
Copy link
Member

bep commented Mar 13, 2024

Your Hugo versions, are they built with the same Go version?

@bep bep removed the NeedsTriage label Mar 13, 2024
@bep bep added this to the v0.123.9 milestone Mar 13, 2024
@jmooring
Copy link
Member Author

hugo v0.123.8-5fed9c591b694f314e5939548e11cc3dcb79a79c+extended linux/amd64 BuildDate=2024-03-07T13:14:42Z VendorInfo=gohugoio
GOOS="linux"
GOARCH="amd64"
GOVERSION="go1.22.0"
github.com/sass/libsass="3.6.5"
github.com/webmproject/libwebp="v1.3.2"
github.com/sass/dart-sass/protocol="2.5.0"
github.com/sass/dart-sass/compiler="1.71.0"
github.com/sass/dart-sass/implementation="1.71.0"

hugo v0.122.0-b9a03bd59d5f71a529acb3e33f995e0ef332b3aa+extended linux/amd64 BuildDate=2024-01-26T15:54:24Z VendorInfo=gohugoio
GOOS="linux"
GOARCH="amd64"
GOVERSION="go1.21.6"
github.com/sass/libsass="3.6.5"
github.com/webmproject/libwebp="v1.3.2"
github.com/sass/dart-sass/protocol="2.5.0"
github.com/sass/dart-sass/compiler="1.71.0"
github.com/sass/dart-sass/implementation="1.71.0"

@bep
Copy link
Member

bep commented Mar 13, 2024

OK, I think I remember seeing something about time equality in go1.22.0 release notes, but it wasn't a "red flag" for me, but I may have been mistaken.

@jmooring
Copy link
Member Author

I know I've used this in templates: https://pkg.go.dev/time#Time.Equal

Equal reports whether t and u represent the same time instant. Two times can be equal even if they are in different locations. For example, 6:00 +0200 and 4:00 UTC are Equal. See the documentation on the Time type for the pitfalls of using == with Time values; most code should use Equal instead.

Not sure what we're using internally.

@jmooring
Copy link
Member Author

The template workaround is to do this:

{{ if .Date.Equal .Lastmod  }}

instead of this:

{{ if eq .Date .Lastmod  }}

@bep
Copy link
Member

bep commented Mar 13, 2024

Not sure what we're using internally.

We use reflect.DeepEqual; I have checked now, and that func falls back to == for dates.

This means that we can easily fix the above failing case, but it would still fail on:

{{ $d1 := dict "date" .Lastmod }}
{{ $d2 := dict "date" .Date }}
{{ eq $d1 $d2 | warnf "%#v" }}

I suggest that we:

  1. Fix the common case by checking if both sides are time.Time and use Equal.
  2. Dig up the Go 1.22 issue that made this fail and make an argument for updating reflect.DeepEqual in Go's repo.

@jmooring
Copy link
Member Author

jmooring commented Mar 13, 2024

But they're not equal...

It seems to me that the comparison provided the expected result (from a user's standpoint) because of how we set the fallback values in v0.122.0. It seems like that is what changed.

@bep
Copy link
Member

bep commented Mar 13, 2024

Yea, right ... OK, now I see it.

Copy link

github-actions bot commented Apr 4, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants