Fix Datetime.{h,cpp} to include cmake.h for HAVE_TIMEGM define#105
Merged
Conversation
There is a preprocessor define to use the system version of timegm() rather than a [slow] fallback function implemented in libshared. The function Datetime::timegm() conditionally manifests if said define is unset, but the code won't actually consult the value (provided by cmake), unless we include the cmake header. (This function is only used by Timewarrior, so Taskwarrior need not peg to a new libshared just for this fix.)
lauft
approved these changes
Aug 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous move of
Datetime::timegm()into libshared (from Taskwarrior) in PR #99 caused a significant performance regression in Timewarrior because a slow fallback method was used due to-DHAVE_TIMEGMnot being picked up from cmake.That fallback calls
tzset()for everytimegm()call, which is done twice for every interval in the Timewarrior data, and involves file I/O with/etc/localtime. These calls can number in the tens of thousands even for simple operations, depending on time db size.This patch includes
cmake.hso theHAVE_TIMEGMdefine can be picked up and we'll defer to the libc implementation oftimegm(), which is much faster (it does not dotzset()).Windows may still be affected by the slowness, it may not have
timegm(), but it will continue to work there, and should use the faster libc on BSDs and GNU systems.Problem described more in GothenburgBitFactory/timewarrior#703 and in #104.
Taskwarrior does not seem to use
timegm(), so no need to bump its libshared.Fixes #104.