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

build: define TZDIR for tzcode build #7186

Merged
merged 1 commit into from Jun 2, 2022

Conversation

grafin
Copy link
Member

@grafin grafin commented May 25, 2022

NixOS (and probably some other distributives) places zoneinfo directory not to /usr/share, but rather in directory of a form /nix/store/izida45xkdjkejekje8282hdhdh6q3-tzdata-2022a/share/zoneinfo/. In this case NixOS sets TZDIR environment variable accordingly. Currently, in Tarantool we hardcode /usr/share/zoneinfo as a location to use, disregarding TZDIR env variable.

This commit adds compile-time definition for TZDIR if such env variable defined, which allows to fix zoneinfo lookup in case of NixOS.

@grafin grafin requested a review from tsafin May 25, 2022 11:30
@grafin grafin requested a review from ligurio May 25, 2022 11:32
nixos (and probably some other distributives) place zoneinfo directory
not in /usr/share (in /etc for example). TZDIR is set accordingly.
Currently zoneinfo is looked for in /usr/share, disregarding TZDIR env
variable.

This commit adds compile definition for TZDIR if such env variable is
defined. This fixes zoneinfo lookup for nixos.

NO_CHANGELOG=build
NO_DOC=build
NO_TEST=build
Copy link
Contributor

@tsafin tsafin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@grafin grafin added the full-ci Enables all tests for a pull request label May 25, 2022
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 84.819% when pulling c2ec986 on grafin:grafin/tzcode-tzdir into 4eaff4e on tarantool:master.

@tsafin tsafin mentioned this pull request May 26, 2022
16 tasks
Copy link
Member

@ligurio ligurio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (see a small comment inline)

@@ -13,6 +13,10 @@ set(TZDATA_FULL_TARGET ${TZCODE_SOURCE_DIR}/${TZDATA_TARGET})
set(TZCODE_SRC ${PROJECT_SOURCE_DIR}/src/lib/tzcode)
set(TZLUA_SRC ${PROJECT_SOURCE_DIR}/src/lua)

if(DEFINED ENV{TZDIR})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can evaluate ENV{TZDIR} only once and then use variable with value. I'll not insist, it's up to you.

@kyukhin kyukhin merged commit b9c9a7b into tarantool:master Jun 2, 2022
@kyukhin
Copy link
Contributor

kyukhin commented Jun 2, 2022

Backported to 2.10 as well.

CuriousGeorgiy added a commit to CuriousGeorgiy/tarantool that referenced this pull request Jan 17, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes tarantool#8136

NO_CHANGELOG=<tarantoolgh-7186 was not released yet>
NO_DOC=bugfix
locker pushed a commit that referenced this pull request Jan 18, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes #8136

NO_CHANGELOG=<gh-7186 was not released yet>
NO_DOC=bugfix
Lord-KA pushed a commit to Lord-KA/tarantool that referenced this pull request Feb 27, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes tarantool#8136

NO_CHANGELOG=<tarantoolgh-7186 was not released yet>
NO_DOC=bugfix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants