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

[libc++][TZDB] Improves system time zone detection. #127339

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

mordante
Copy link
Member

On some (Linux) systems /etc/localtime is not a symlink to the time zone, but contains a copy of the binary time zone file. In these case there usually is a file named /etc/timezone which contains the text for the current time zone name.

Instead of throwing when /etc/localtime does not exist or is not a symlink use this fallback.

Fixes: #105634

On some (Linux) systems /etc/localtime is not a symlink to the time
zone, but contains a copy of the binary time zone file. In these case
there usually is a file named /etc/timezone which contains the text for
the current time zone name.

Instead of throwing when /etc/localtime does not exist or is not a
symlink use this fallback.

Fixes: llvm#105634
@mordante mordante requested a review from a team as a code owner February 15, 2025 17:10
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2025

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

On some (Linux) systems /etc/localtime is not a symlink to the time zone, but contains a copy of the binary time zone file. In these case there usually is a file named /etc/timezone which contains the text for the current time zone name.

Instead of throwing when /etc/localtime does not exist or is not a symlink use this fallback.

Fixes: #105634


Full diff: https://github.com/llvm/llvm-project/pull/127339.diff

1 Files Affected:

  • (modified) libcxx/src/experimental/tzdb.cpp (+50-18)
diff --git a/libcxx/src/experimental/tzdb.cpp b/libcxx/src/experimental/tzdb.cpp
index f38f495c2d0bb..987ae5bbd8e78 100644
--- a/libcxx/src/experimental/tzdb.cpp
+++ b/libcxx/src/experimental/tzdb.cpp
@@ -708,6 +708,39 @@ void __init_tzdb(tzdb& __tzdb, __tz::__rules_storage_type& __rules) {
   std::__throw_runtime_error("unknown time zone");
 }
 #else  // ifdef _WIN32
+
+[[nodiscard]] static string __current_zone_environment() {
+  if (const char* __tz = std::getenv("TZ"))
+    return __tz;
+
+  return {};
+}
+
+[[nodiscard]] static string __current_zone_etc_localtime() {
+  filesystem::path __path = "/etc/localtime";
+  if (!filesystem::exists(__path) || !filesystem::is_symlink(__path))
+    return {};
+
+  filesystem::path __tz = filesystem::read_symlink(__path);
+  // The path may be a relative path, in that case convert it to an absolute
+  // path based on the proper initial directory.
+  if (__tz.is_relative())
+    __tz = filesystem::canonical("/etc" / __tz);
+
+  return filesystem::relative(__tz, "/usr/share/zoneinfo/");
+}
+
+[[nodiscard]] static string __current_zone_etc_timezone() {
+  filesystem::path __path = "/etc/timezone";
+  if (!filesystem::exists(__path))
+    return {};
+
+  ifstream __f(__path);
+  string __name;
+  std::getline(__f, __name);
+  return __name;
+}
+
 [[nodiscard]] static const time_zone* __current_zone_posix(const tzdb& tzdb) {
   // On POSIX systems there are several ways to configure the time zone.
   // In order of priority they are:
@@ -726,30 +759,29 @@ void __init_tzdb(tzdb& __tzdb, __tz::__rules_storage_type& __rules) {
   //
   // - The time zone name is the target of the symlink /etc/localtime
   //   relative to /usr/share/zoneinfo/
+  //
+  // - The file /etc/timezone. This text file contains the name of the time
+  //   zone.
+  //
+  // On Linux systems it seems /etc/timezone is deprecated and being phased
+  // out. This file is used when /etc/localtime is not a symlink, but contains
+  // a copy of the binary time zone file. For more information and links see
+  // https://github.com/llvm/llvm-project/issues/105634
 
-  // The algorithm is like this:
-  // - If the environment variable TZ is set and points to a valid
-  //   record use this value.
-  // - Else use the name based on the `/etc/localtime` symlink.
+  string __name = chrono::__current_zone_environment();
 
-  if (const char* __tz = getenv("TZ"))
-    if (const time_zone* __result = tzdb.__locate_zone(__tz))
+  // Ignore invalid names in the environment.
+  if (!__name.empty())
+    if (const time_zone* __result = tzdb.__locate_zone(__name))
       return __result;
 
-  filesystem::path __path = "/etc/localtime";
-  if (!filesystem::exists(__path))
-    std::__throw_runtime_error("tzdb: the symlink '/etc/localtime' does not exist");
-
-  if (!filesystem::is_symlink(__path))
-    std::__throw_runtime_error("tzdb: the path '/etc/localtime' is not a symlink");
+  __name = chrono::__current_zone_etc_localtime();
+  if (__name.empty())
+    __name = chrono::__current_zone_etc_timezone();
 
-  filesystem::path __tz = filesystem::read_symlink(__path);
-  // The path may be a relative path, in that case convert it to an absolute
-  // path based on the proper initial directory.
-  if (__tz.is_relative())
-    __tz = filesystem::canonical("/etc" / __tz);
+  if (__name.empty())
+    std::__throw_runtime_error("tzdb: unable to determine the name of the current time zone");
 
-  string __name = filesystem::relative(__tz, "/usr/share/zoneinfo/");
   if (const time_zone* __result = tzdb.__locate_zone(__name))
     return __result;
 

@jrtc27
Copy link
Collaborator

jrtc27 commented Feb 15, 2025

This doesn't fully fix the bug. FreeBSD remains broken.

@mordante
Copy link
Member Author

This doesn't fully fix the bug. FreeBSD remains broken.

At the moment we don't support TZDB on FreeBSD, I've reached out privately to @emaste to get this working.
(There were some changes that needed to be made on the FreeBSD site, these are done now.)

@mordante mordante merged commit f796747 into llvm:main Feb 18, 2025
8 of 13 checks passed
@mordante mordante deleted the review/improves_time_zone_detection branch February 18, 2025 18:28
wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this pull request Feb 19, 2025
On some (Linux) systems /etc/localtime is not a symlink to the time
zone, but contains a copy of the binary time zone file. In these case
there usually is a file named /etc/timezone which contains the text for
the current time zone name.

Instead of throwing when /etc/localtime does not exist or is not a
symlink use this fallback.

Fixes: llvm#105634

---------

Co-authored-by: Louis Dionne <[email protected]>
mordante added a commit that referenced this pull request Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] tzdb throws if /etc/localtime is not a symlink
4 participants