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

Use TZDIR for ZONEINFO_DIR if the variable is set. #120

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Use TZDIR for ZONEINFO_DIR if the variable is set. #120

merged 1 commit into from
Jul 31, 2023

Conversation

iyzsong
Copy link
Contributor

@iyzsong iyzsong commented Jul 22, 2023

Hello, this is needed on Guix where there is no /usr and TZDIR is changed when the tzdata package got updated.
Also glibc use it, so it seems reasonable thing to me. eg: https://www.man7.org/linux/man-pages/man3/tzset.3.html

Thanks.

Copy link
Member

@zhuyaliang zhuyaliang left a comment

Choose a reason for hiding this comment

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

TZDIR: If this variable is set its value takes precedence over the system configured timezone database directory path.

@raveit65
Copy link
Member

How is that testable?
Should i add a new location with a different timezone in clock-applet?

@iyzsong
Copy link
Contributor Author

iyzsong commented Jul 30, 2023

For test, I think you can set TZDIR point to a tzdata-2022a version, which will lack Europe/Kiev.

@raveit65
Copy link
Member

How did you test that it works?

@iyzsong
Copy link
Contributor Author

iyzsong commented Jul 31, 2023

x
It can be seen in the Clock (top-right panel)'s Locactions settings, the Timezone will be empty when TZDIR set to an invalid value, and works fine when TZDIR point to the right directory (in my case: /gnu/store/19z4zyx0nykgcmj75izhsbi6z0ascq4i-tzdata-2022a/share/zoneinfo).

Thanks.

@raveit65
Copy link
Member

The question was more how/where to set old obsolete TZ data, and not to set up a location in our gui.
Any way, i will try to do that for my self when i found time.

@raveit65
Copy link
Member

Deleting/re-adding /usr/share/africa and selecting Johannesburg works fine, so i assume this change is working.

Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

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

LGTM

@raveit65 raveit65 merged commit df02377 into mate-desktop:master Jul 31, 2023
1 check passed
@@ -71,7 +71,10 @@ parse_tzdata (const char *tzname, time_t start, time_t end,
char initial_isdst, second_isdst;
int i;

filename = g_build_filename (ZONEINFO_DIR, tzname, NULL);
tzdir = g_getenv ("TZDIR");
Copy link
Member

@raveit65 raveit65 Oct 19, 2023

Choose a reason for hiding this comment

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

Sorry , i didn't saw that during review.
We got a build warning here.
https://app.travis-ci.com/github/mate-desktop/libmateweather/jobs/606993727#L1649

mateweather-timezone.c:74:11: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]

   74 |     tzdir = g_getenv ("TZDIR");

      |           ^

@iyzsong
Any chance to fix the warning?

@iyzsong
Copy link
Contributor Author

iyzsong commented Oct 20, 2023 via email

@raveit65
Copy link
Member

Hello, declare tzdir as 'const char *tzdir' instead of 'char *tzdir'
should fix the the warning.

@iyzsong
Can you fix your work please and do a pull request?

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

Successfully merging this pull request may close these issues.

None yet

3 participants