Skip to content

Commit

Permalink
Using conda-forge's tzdata on all platforms
Browse files Browse the repository at this point in the history
  • Loading branch information
SylvainCorlay committed May 31, 2024
1 parent 7a30c43 commit bd4cd30
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .azure-pipelines/azure-pipelines-osx.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 0 additions & 9 deletions .ci_support/linux_64_.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
c_stdlib:
- sysroot
c_stdlib_version:
- '2.12'
cdt_name:
- cos6
channel_sources:
Expand All @@ -14,10 +10,5 @@ cxx_compiler_version:
- '12'
docker_image:
- quay.io/condaforge/linux-anvil-cos7-x86_64
libcurl:
- '8'
target_platform:
- linux-64
zip_keys:
- - c_stdlib_version
- cdt_name
9 changes: 0 additions & 9 deletions .ci_support/linux_aarch64_.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
BUILD:
- aarch64-conda_cos7-linux-gnu
c_stdlib:
- sysroot
c_stdlib_version:
- '2.17'
cdt_arch:
- aarch64
cdt_name:
Expand All @@ -18,10 +14,5 @@ cxx_compiler_version:
- '12'
docker_image:
- quay.io/condaforge/linux-anvil-aarch64
libcurl:
- '8'
target_platform:
- linux-aarch64
zip_keys:
- - c_stdlib_version
- cdt_name
9 changes: 0 additions & 9 deletions .ci_support/linux_ppc64le_.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
c_stdlib:
- sysroot
c_stdlib_version:
- '2.17'
cdt_name:
- cos7
channel_sources:
Expand All @@ -14,10 +10,5 @@ cxx_compiler_version:
- '12'
docker_image:
- quay.io/condaforge/linux-anvil-ppc64le
libcurl:
- '8'
target_platform:
- linux-ppc64le
zip_keys:
- - c_stdlib_version
- cdt_name
6 changes: 0 additions & 6 deletions .ci_support/osx_64_.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ MACOSX_DEPLOYMENT_TARGET:
- '11.0'
MACOSX_SDK_VERSION:
- '11.0'
c_stdlib:
- macosx_deployment_target
c_stdlib_version:
- '11.0'
channel_sources:
- conda-forge
channel_targets:
Expand All @@ -14,8 +10,6 @@ cxx_compiler:
- clangxx
cxx_compiler_version:
- '16'
libcurl:
- '8'
macos_machine:
- x86_64-apple-darwin13.4.0
target_platform:
Expand Down
6 changes: 1 addition & 5 deletions .ci_support/osx_arm64_.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
MACOSX_DEPLOYMENT_TARGET:
- '11.0'
c_stdlib:
- macosx_deployment_target
c_stdlib_version:
MACOSX_SDK_VERSION:
- '11.0'
channel_sources:
- conda-forge
Expand All @@ -12,8 +10,6 @@ cxx_compiler:
- clangxx
cxx_compiler_version:
- '16'
libcurl:
- '8'
macos_machine:
- arm64-apple-darwin20.0.0
target_platform:
Expand Down
4 changes: 0 additions & 4 deletions .ci_support/win_64_.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
c_stdlib:
- vs
channel_sources:
- conda-forge
channel_targets:
- conda-forge main
cxx_compiler:
- vs2019
libcurl:
- '8'
target_platform:
- win-64
vc:
Expand Down
4 changes: 2 additions & 2 deletions .gitattributes

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .travis.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,24 @@ source:
fn: {{ name }}-{{ version }}.tar.gz
url: https://github.com/HowardHinnant/date/archive/v{{ version }}.tar.gz
sha256: {{ sha256 }}
patches:
- use-conda-tzdb-on-all-platforms.patch

build:
number: 3
number: 4
skip: true # [win and vc<14]

requirements:
build:
- {{ compiler('cxx') }}
- {{ stdlib('c') }}
- cmake
- make # [unix]
- pkg-config
- gnuconfig
host:
- dirent # [win]
run:
- tzdata

test:
commands:
Expand Down
169 changes: 169 additions & 0 deletions recipe/use-conda-tzdb-on-all-platforms.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
From 6effd1dbd74475d26a38e3c711434f55423c86ae Mon Sep 17 00:00:00 2001
From: Sylvain Corlay <[email protected]>
Date: Fri, 31 May 2024 14:19:04 +0200
Subject: [PATCH] use conda tzdb on all platforms

---
CMakeLists.txt | 2 +-
include/date/tz.h | 6 -----
src/tz.cpp | 65 +++++++++++++----------------------------------
3 files changed, 19 insertions(+), 54 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6885a43..aa470fe 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -132,7 +132,7 @@ if( BUILD_TZ_LIB )
target_compile_definitions( date-tz PRIVATE AUTO_DOWNLOAD=1 HAS_REMOTE_API=1 )
endif()

- if ( USE_SYSTEM_TZ_DB AND NOT WIN32 AND NOT MANUAL_TZ_DB )
+ if ( USE_SYSTEM_TZ_DB AND NOT MANUAL_TZ_DB )
target_compile_definitions( date-tz PRIVATE INSTALL=. PUBLIC USE_OS_TZDB=1 )
else()
target_compile_definitions( date-tz PUBLIC USE_OS_TZDB=0 )
diff --git a/include/date/tz.h b/include/date/tz.h
index 0f9f2c5..3514559 100644
--- a/include/date/tz.h
+++ b/include/date/tz.h
@@ -82,12 +82,6 @@ static_assert(HAS_REMOTE_API == 0 ? AUTO_DOWNLOAD == 0 : true,
# define USE_SHELL_API 1
#endif

-#if USE_OS_TZDB
-# ifdef _WIN32
-# error "USE_OS_TZDB can not be used on Windows"
-# endif
-#endif
-
#ifndef HAS_DEDUCTION_GUIDES
# if __cplusplus >= 201703
# define HAS_DEDUCTION_GUIDES 1
diff --git a/src/tz.cpp b/src/tz.cpp
index 82e4312..638f9bd 100644
--- a/src/tz.cpp
+++ b/src/tz.cpp
@@ -457,51 +457,12 @@ CONSTCD14 const sys_seconds min_seconds = sys_days(min_year/min_day);

#endif // USE_OS_TZDB

-#ifndef _WIN32
-
static
std::string
discover_tz_dir()
{
- struct stat sb;

This comment has been minimized.

Copy link
@rwdougla

rwdougla Aug 13, 2024

The purpose of this code was to explicitly use the system's timezone info. This PR has destroyed that feature of this library, which we apparently depended on. To make matters worse, the lack of CONDA_PREFIX in the environment causes a runtime exception in std::string being created from a null value. So even as is, it's a regression since the library, prior, had well-defined behavior in the event the timezone data couldnt be found on the system.

If I could simply set this environment variable and have it only affect this patch, this wouldn't be a big deal. But I can't guarantee that setting it, where it wasnt before, wont have behavior changes for other libraries.

Please, either:

  1. Revert this in its entirety,
  2. Simply make the environment variable preferred and fallback if not set, or
  3. Use a different, unique environment variable like CONDA_TZ_DIR that can reliably be set without impacting other libraries.
- using namespace std;
-# ifndef __APPLE__
- CONSTDATA auto tz_dir_default = "/usr/share/zoneinfo";
- CONSTDATA auto tz_dir_buildroot = "/usr/share/zoneinfo/uclibc";
-
- // Check special path which is valid for buildroot with uclibc builds
- if(stat(tz_dir_buildroot, &sb) == 0 && S_ISDIR(sb.st_mode))
- return tz_dir_buildroot;
- else if(stat(tz_dir_default, &sb) == 0 && S_ISDIR(sb.st_mode))
- return tz_dir_default;
- else
- throw runtime_error("discover_tz_dir failed to find zoneinfo\n");
-# else // __APPLE__
-# if TARGET_OS_IPHONE
-# if TARGET_OS_SIMULATOR
- return "/usr/share/zoneinfo";
-# else
- return "/var/db/timezone/zoneinfo";
-# endif
-# else
- CONSTDATA auto timezone = "/etc/localtime";
- if (!(lstat(timezone, &sb) == 0 && S_ISLNK(sb.st_mode) && sb.st_size > 0))
- throw runtime_error("discover_tz_dir failed\n");
- string result;
- char rp[PATH_MAX+1] = {};
- if (readlink(timezone, rp, sizeof(rp)-1) > 0)
- result = string(rp);
- else
- throw system_error(errno, system_category(), "readlink() failed");
- auto i = result.find("zoneinfo");
- if (i == string::npos)
- throw runtime_error("discover_tz_dir failed to find zoneinfo\n");
- i = result.find('/', i);
- if (i == string::npos)
- throw runtime_error("discover_tz_dir failed to find '/'\n");
- return result.substr(0, i);
-# endif
-# endif // __APPLE__
+ std::string CONDA_PREFIX = std::getenv("CONDA_PREFIX");
+ return CONDA_PREFIX + folder_delimiter + "share" + folder_delimiter + "zoneinfo";
}

static
@@ -512,8 +473,6 @@ get_tz_dir()
return tz_dir;
}

-#endif
-
// +-------------------+
// | End Configuration |
// +-------------------+
@@ -607,7 +566,6 @@ parse_month(std::istream& in)
return static_cast<unsigned>(++m);
}

-#if !USE_OS_TZDB

#ifdef _WIN32

@@ -811,6 +769,7 @@ load_timezone_mappings_from_xml_file(const std::string& input_path)

#endif // _WIN32

+#if !USE_OS_TZDB
// Parsing helpers

static
@@ -1840,9 +1799,15 @@ time_zone::time_zone(const std::string& s, detail::undocumented)

enum class endian
{
+#ifdef _WIN32
+ little = 0,
+ big = 1,
+ native = little
+#else
native = __BYTE_ORDER__,
little = __ORDER_LITTLE_ENDIAN__,
big = __ORDER_BIG_ENDIAN__
+#endif
};

static
@@ -2149,8 +2114,8 @@ time_zone::init_impl()
{
using namespace std;
using namespace std::chrono;
- auto name = get_tz_dir() + ('/' + name_);
- std::ifstream inf(name);
+ auto name = get_tz_dir() + (folder_delimiter + name_);
+ std::ifstream inf(name, std::ios::in | std::ios::binary);
if (!inf.is_open())
throw std::runtime_error{"Unable to open " + name};
inf.exceptions(std::ios::failbit | std::ios::badbit);
@@ -3825,7 +3790,13 @@ locate_zone(std::string_view tz_name)
locate_zone(const std::string& tz_name)
#endif
{
- return get_tzdb().locate_zone(tz_name);
+ #if _WIN32 && USE_OS_TZDB
+ std::string local_tz_name(tz_name);
+ std::replace(local_tz_name.begin(), local_tz_name.end(), '/', '\\');
+ return get_tzdb().locate_zone(local_tz_name);
+ #else
+ return get_tzdb().locate_zone(tz_name);
+ #endif
}

#if USE_OS_TZDB
--
2.40.1

0 comments on commit bd4cd30

Please sign in to comment.