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

Assume UTC if "tzdata" are not available #153

Open
voxik opened this issue Sep 29, 2023 · 4 comments
Open

Assume UTC if "tzdata" are not available #153

voxik opened this issue Sep 29, 2023 · 4 comments

Comments

@voxik
Copy link

voxik commented Sep 29, 2023

Recently, tzdata were removed from default Fedora installation (and this will eventually impact also RHEL). Now running ActiveSupport test suite (but also other test suites) we are facing erros such as:

/usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_sources/zoneinfo_data_source.rb:252:in `initialize': None of the paths included in TZInfo::DataSources::ZoneinfoDataSource.search_path are valid zoneinfo directories. (TZInfo::DataSources::ZoneinfoDirectoryNotFound)
    from /usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_source.rb:157:in `new'
    from /usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_source.rb:157:in `create_default_data_source'
    from /usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_source.rb:55:in `block in get'
    from /usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_source.rb:54:in `synchronize'
    from /usr/share/gems/gems/tzinfo-2.0.6/lib/tzinfo/data_source.rb:54:in `get'
    from /usr/share/gems/gems/activesupport-7.0.8/lib/active_support/railtie.rb:88:in `block in <class:Railtie>'
    from /usr/share/gems/gems/railties-7.0.8/lib/rails/initializable.rb:32:in `instance_exec'
    from /usr/share/gems/gems/railties-7.0.8/lib/rails/initializable.rb:32:in `run'
    from /usr/share/gems/gems/railties-7.0.8/lib/rails/initializable.rb:61:in `block in run_initializers'
    from /usr/share/ruby/tsort.rb:231:in `block in tsort_each'
    from /usr/share/ruby/tsort.rb:353:in `block (2 levels) in each_strongly_connected_component'
    from /usr/share/ruby/tsort.rb:434:in `each_strongly_connected_component_from'
    from /usr/share/ruby/tsort.rb:352:in `block in each_strongly_connected_component'
    from /usr/share/ruby/tsort.rb:350:in `each'
    from /usr/share/ruby/tsort.rb:350:in `call'
    from /usr/share/ruby/tsort.rb:350:in `each_strongly_connected_component'
    from /usr/share/ruby/tsort.rb:229:in `tsort_each'
    from /usr/share/ruby/tsort.rb:208:in `tsort_each'
    from /usr/share/gems/gems/railties-7.0.8/lib/rails/initializable.rb:60:in `run_initializers'
    from /usr/share/gems/gems/railties-7.0.8/lib/rails/application.rb:372:in `initialize!' 

It seems this cannot be workaround on ActiveSupport side, but would there be and option to fallback to UTC, when no tzdata are found? It seems that this is the consensus, reiterated in this Fedora thread:

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/C37PSJNLJDOUZEBBWGTUSW6UZVT6BXKI/

@philr
Copy link
Member

philr commented Sep 30, 2023

What do you mean by 'assume UTC' / 'fallback to UTC'? TZInfo has no concept of a current time zone. The Fedora advice seems targeted at applications that need to know the system time zone. For that, assuming UTC would make sense.

I see four possible options:

  1. If no zoneinfo files are found and tzinfo-data is not installed, then fallback to a TZInfo::DataSource implementation that only contains a 'UTC' time zone.
  2. Have a UTC-only data source like option 1, but require an explicit opt-in to use it, e.g. TZInfo::DataSource.set(:utc_only).
  3. Make the 'UTC' time zone a special case and always allow TZInfo::Timezone.get('UTC') to succeed even if there's no available data source.
  4. Provide a means to obtain a special case UTC time zone via a new TZInfo::Timezone.utc method.

I think option 1 would cause confusion. Currently, an exception is thrown when there's no available data source. The behaviour would change to TZInfo::InvalidTimezoneIdentifier and TZInfo::InvalidCountryCode exceptions being raised when trying to get time zones and countries with TZInfo::Timezone.get and TZInfo::Country.get. An invalid identifier/code seems less obviously related to a missing data source.

Option 2 perhaps doesn't add much. If you're going to have to explicitly 'opt-in' to UTC-only, then you could also make a zoneinfo directory available with just a UTC zone (or install a complete set of time zone data).

Regarding option 3, TZInfo is intended to expose the data from the (IANA Time Zone Database) data source without making any changes. Forcing 'UTC' to be available doesn't fit with that philosophy. Some thought would also be needed on what methods like TZInfo::Timezone.all_identifiers should do.

Option 4 sidesteps the issues with option 3. However, it would require downstream users, such as ActiveSupport, to handle UTC as a special case. They could presumably do that anyway without using TZInfo.

I'm not really convinced a change is needed. Fundamentally, TZInfo is a library for handling time zone conversions. It doesn't seem unreasonable that a source of time zone data should be required to use it.

@voxik
Copy link
Author

voxik commented Oct 2, 2023

From the options you proposed, the option 1 or 3 are something along the lines I imagine. I believe that in the context of rails/rails#49375, it would solve 99% of use cases, because the default value is 'UTC'

@philr
Copy link
Member

philr commented Oct 17, 2023

For reference, the IANA Time Zone Database time zone ActiveSupport currently looks for when asked for 'UTC' is actually 'Etc/UTC' (via ActiveSupport::TimeZone::MAPPING). This is actually what would need to be available.

'Etc/UTC' is the canonical UTC time zone in the current version 2023c of the IANA Time Zone Database. It also contains the following aliases: 'Etc/UCT', 'Etc/Universal', 'Etc/Zulu', 'UCT', 'Universal', 'UTC' and 'Zulu'.

From the options you proposed, the option 1 or 3 are something along the lines I imagine. I believe that in the context of rails/rails#49375, it would solve 99% of use cases, because the default value is 'UTC'

As I outlined, I'm not keen on of either of those options. Option 1 would lead to confusion (and break the missing time zone data check on startup of Rails). Option 3 deviates from the design philosophy - the data is supposed to all be provided externally.

It would just about be possible to modify ActiveSupport::TimeZone.[] to handle 'UTC' as a special case. That could return an ActiveSupport::TimeZone instance that bypassed tzinfo completely. However, that would be incomplete because ActiveSupport::TimeZone is a leaky abstraction - providing access to the TZInfo::Timezone, etc.

If option 4 was implemented, then the ActiveSupport::TimeZone.[] handling of 'UTC' could be changed to call a (hypothetical) TZInfo::Timezone.utc method to obtain an actual TZInfo::Timezone instance.

@voxik
Copy link
Author

voxik commented Oct 17, 2023

@rafaelfranca would some of the proposals be acceptable for Rails? If not, then I think we can close this ticket

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

No branches or pull requests

2 participants