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

time.mktime doesn't update time.tzname #66987

Closed
4kir4 mannequin opened this issue Nov 5, 2014 · 17 comments
Closed

time.mktime doesn't update time.tzname #66987

4kir4 mannequin opened this issue Nov 5, 2014 · 17 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@4kir4
Copy link
Mannequin

4kir4 mannequin commented Nov 5, 2014

BPO 22798
Nosy @malemburg, @abalkin, @pitrou, @4kir4
Files
  • test-timezone-info-is-updated.diff: show that time functions fail to update the timezone info
  • test_mktime_changes_tzname.c: improve error handling (doesn't change the result)
  • sync-time-timezone-attr-with-c.diff
  • issue22798.diff
  • test-mktime.c
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/abalkin'
    closed_at = <Date 2016-09-13.17:19:55.439>
    created_at = <Date 2014-11-05.12:12:28.107>
    labels = ['3.7', 'type-bug', 'library']
    title = "time.mktime doesn't update time.tzname"
    updated_at = <Date 2016-09-13.17:19:55.437>
    user = 'https://github.com/4kir4'

    bugs.python.org fields:

    activity = <Date 2016-09-13.17:19:55.437>
    actor = 'belopolsky'
    assignee = 'belopolsky'
    closed = True
    closed_date = <Date 2016-09-13.17:19:55.439>
    closer = 'belopolsky'
    components = ['Library (Lib)']
    creation = <Date 2014-11-05.12:12:28.107>
    creator = 'akira'
    dependencies = []
    files = ['37133', '37134', '37273', '40598', '44641']
    hgrepos = []
    issue_num = 22798
    keywords = ['patch']
    message_count = 17.0
    messages = ['230674', '230675', '231648', '249296', '249395', '249402', '249413', '249417', '249479', '249480', '251712', '251714', '251829', '251840', '251889', '276295', '276307']
    nosy_count = 4.0
    nosy_names = ['lemburg', 'belopolsky', 'pitrou', 'akira']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22798'
    versions = ['Python 3.7']

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Nov 5, 2014

    time.tzname is initialized from C tzname variable or tm_zone around Jan, Jul of the current year.

    If time.mktime() is called with a time tuple from the past/future then after the call time.tzname might be out-of-sync with the corresponding C tzname and tm_zone values. Because C mktime may change tzname, tm_zone values on some systems and time.mktime calls C mktime.

    I've attached test_mktime_changes_tzname.c file that demonstrates that mktime() may change tzname.

    @4kir4 4kir4 mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 5, 2014
    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Nov 5, 2014

    I've attached test-timezone-info-is-updated.diff file -- a patch for Lib/test/test_time.py that demonstrates that time functions fail to update the timezone info.

    The test uses Europe/Moscow timezone but most timezones around the world had/will have different tzname/utc offset (unrelated to DST changes) in the past/future.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Nov 25, 2014

    One of the ways to fix this issue is to synchronize time.tzname
    attribute with the corresponding C tzname variable.

    I've uploaded sync-time-timezone-attr-with-c.diff patch that
    synchronizes tzname, timezone, altzone, daylight attributes.
    The patch also includes tests. No docs changes are necessary.
    The code follows C, POSIX standards and the time module documentation.
    Any differences are unintetional.

    The patch also fixes "wrong time.timezone" issue
    http://bugs.python.org/issue22799

    The patch doesn't use tm_gmtoff, tm_zone C values from jan, jul to set
    timezone, tzname time module attributes therefore it may break
    software that relies on that behaviour. I would be interested to hear
    about such instances.

    I've removed the cygwin branch in the code (I haven't found cygwin
    buildbot). It could be added back as is if necessary.

    @abalkin
    Copy link
    Member

    abalkin commented Aug 28, 2015

    C mktime itself should not change timezone globals, but it may indirectly if it calls tzset() and TZ changed between calls.

    I don't understand what this issue is about. Unlike C mktime, time.mktime is not documented to call time.tzset(). If you want to change time.tzname - you need to call time.tzset() after changing TZ.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Aug 31, 2015

    C mktime itself should not change timezone globals, but it may indirectly if it calls tzset() and TZ changed between calls.

    You should have run the attached test_mktime_changes_tzname.c which demonstrates that (at least on some systems) C mktime *does* change tzname global even if TZ envvar is constant in the test.

    Nowhere in my bug report I had mentioned different TZ values. I did mentioned *past* *future* dates -- the same timezone may have different utc offset, timezone abbreviations at different times. These timezone globals can change even if TZ is constant.

    @abalkin
    Copy link
    Member

    abalkin commented Aug 31, 2015

    You should have run the attached test_mktime_changes_tzname.c ...

    I did run it and tried several times to understand what it does. It did not help.

    If you claim that calling mktime() changes the value of the global tzname variable, you should be able to demonstrate it in five lines of code:

    extern char **tzname;
    tzset();
    printf("%s %s\n", tzname[0], tzname[1]);
    mktime(&tt);
    printf("%s %s\n", tzname[0], tzname[1]);

    (plus a few lines to initialize tt)

    If the code above prints two different lines - file a bug report with your C library vendor.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Aug 31, 2015

    The C code produces correct values according to the tz database.

    If TZ=Europe/Moscow then
    tzname={"MSK", "MSD"} at 2010-07-01 and
    tzname={"MSK", "MSK"} at 2015-07-01. Notice the difference!

    The code calls C mktime() with corresponding time tuples
    and checks that *tzname* is equal to the expected values. That's all.

    If C code is incomprehensible; here's its Python analog:

      >>> import os
      >>> os.environ['TZ'] = 'Europe/Moscow'
      >>> import time
      >>> time.tzset()
      >>> time.mktime((2010,7,1,0,0,0,-1,-1,-1))
      1277928000.0
      >>> time.tzname #XXX expected ('MSK', 'MSD')
      ('MSK', 'MSK')
      >>> time.mktime((2015,7,1,0,0,0,-1,-1,-1))
      1435698000.0
      >>> time.tzname
      ('MSK', 'MSK')

    C tzname changes on my machine after the corresponding C mktime() calls
    but Python time.tzname does not change after the time.mktime() calls.

    @abalkin
    Copy link
    Member

    abalkin commented Aug 31, 2015

    C tzname changes on my machine after the corresponding C mktime() calls

    Did you run the code that I posted? If not - please do and report the results.

    (I think there is a valid issue behind all this, but you are not helping to reveal it. I suspect we can do better by just relying on system tzset() and not have the try June and January work-around. That's why I added Antoine to the "nosy" as the developer who last touched this code.)

    @malemburg
    Copy link
    Member

    mktime() does change several global C runtime variables on Linux. See the man page on http://linux.die.net/man/3/mktime

    However, the Python documentation does not mention anything about having time.tzname being tied to the C lib global: https://docs.python.org/3.5/library/time.html#time.tzname

    So I don't think this qualifies as bug.

    Note that those global C variable are not thread-safe, so you can't really trust their values relating to anything you've just set using mktime() anyway.

    @malemburg
    Copy link
    Member

    BTW: The most portable way to access the timezone name of a given local time is to use strftime() with %Z as format character.

    @abalkin
    Copy link
    Member

    abalkin commented Sep 27, 2015

    Akira,

    Would bpo-22798.diff patch address your issue?

    @abalkin abalkin removed the invalid label Sep 27, 2015
    @abalkin abalkin self-assigned this Sep 27, 2015
    @abalkin
    Copy link
    Member

    abalkin commented Sep 27, 2015

    Is there any platform where mktime resets global tzname but does not provide tm_zone? If not, OP's issue is largely theoretical, but I still like MAL's suggestion of using strftime("%Z") as a fall-back.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Sep 29, 2015

    Would bpo-22798.diff patch address your issue?

    No. The issue is that C mktime() may update C tzname on some platforms
    but time.mktime() does not update time.tzname on these platforms while
    the time module docs suggest that it might be expected e.g.:

    Most of the functions defined in this module call platform C library
    functions with the same name. It may sometimes be helpful to consult
    the platform documentation, because the semantics of these functions
    varies among platforms.

    ---

    Unrelated: time.strftime('%Z') and
    time.strftime('%Z', time.localtime(time.time())) can differ on some
    platforms and python versions
    http://stackoverflow.com/questions/32353015/python-time-strftime-z-is-always-zero-instead-of-timezone-offset

    @malemburg
    Copy link
    Member

    On 29.09.2015 11:31, Akira Li wrote:

    Akira Li added the comment:

    > Would bpo-22798.diff patch address your issue?

    No. The issue is that C mktime() may update C tzname on some platforms
    but time.mktime() does not update time.tzname on these platforms while
    the time module docs suggest that it might be expected e.g.:

    Most of the functions defined in this module call platform C library
    functions with the same name. It may sometimes be helpful to consult
    the platform documentation, because the semantics of these functions
    varies among platforms.

    tzname is set when the module is being loaded and not updated
    afterwards (unless you call tzset()). I can't really see why you
    would expect a module global in Python to follow the semantics
    of a C global, unless this is explicitly documented.

    Note: The fact that tzset() does update the module globals is
    not documented.

    ---

    Unrelated: time.strftime('%Z') and
    time.strftime('%Z', time.localtime(time.time())) can differ on some
    platforms and python versions
    http://stackoverflow.com/questions/32353015/python-time-strftime-z-is-always-zero-instead-of-timezone-offset

    The StackOverflow discussion targets %z (lower case z),
    not %Z (with capital Z).

    I think this is just a documentation bug.

    Overall, I think that relying on those module globals is
    not a good idea. They are not threadsafe and their values
    can only be trusted right after module import. It's much
    safer to access the resp. values by using struct_time values
    or strftime().

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Sep 29, 2015

    Marc-Andre Lemburg <report@bugs.python.org> writes:
    ...

    tzname is set when the module is being loaded and not updated
    afterwards (unless you call tzset()). I can't really see why you
    would expect a module global in Python to follow the semantics
    of a C global, unless this is explicitly documented.

    Python docs recommend reading platform docs.
    Platform docs say that mktime() may change tzname.
    Python docs do not contradict.

    Why wouldn't I assume that mktime() may change tzname?

    Note: The fact that tzset() does update the module globals is
    not documented.

    It is documented e.g., I see: "These will be propagated into
    time.tzname" in tzset() docs.

    Though tzset() has nothing to do with the issue (TZ envvar hasn't been
    changed).

    > Unrelated: time.strftime('%Z') and
    > time.strftime('%Z', time.localtime(time.time())) can differ on some
    > platforms and python versions
    > http://stackoverflow.com/questions/32353015/python-time-strftime-z-is-always-zero-instead-of-timezone-offset

    The StackOverflow discussion targets %z (lower case z),
    not %Z (with capital Z).

    Look at the answers. The examples clearly shows both %Z and %z.

    I think this is just a documentation bug.

    Overall, I think that relying on those module globals is
    not a good idea. They are not threadsafe and their values
    can only be trusted right after module import. It's much
    safer to access the resp. values by using struct_time values
    or strftime().

    Agree. Moreover, you can't trust them even "right after module import"
    sometimes.

    Though, some predictability in the behavior such as "time.tzname is
    whatever C tzname is -- consult the platform docs" would be nice.

    @abalkin
    Copy link
    Member

    abalkin commented Sep 13, 2016

    MAL> The fact that tzset() does update the module globals is
    not documented.

    See bpo-28130.

    @abalkin abalkin added the 3.7 (EOL) end of life label Sep 13, 2016
    @abalkin
    Copy link
    Member

    abalkin commented Sep 13, 2016

    I ran the attached test-mktime.c program on Linux and MacOS X with the following results (TZ=America/New_York):

    $ cat test-mktime.c
    #include <time.h>
    #include <stdio.h>
    static void
    print_globals(struct tm *tm)
    {
      printf("%04d-%02d: %s/%s (%s) %d %ld (%ld)\n",
           	 1900 + tm->tm_year, 1 + tm->tm_mon,
           	 tzname[0], tzname[1], tm->tm_zone?tm->tm_zone:"NULL",
           	 daylight, timezone, -tm->tm_gmtoff);
    }
    
    int main() {
      struct tm tm = {0, 0, 0, 1, 0, -100};
      print_globals(&tm);
      tzset();
      print_globals(&tm);
      mktime(&tm);
      print_globals(&tm);
      tm.tm_year = 43;
      mktime(&tm);
      print_globals(&tm);
      tm.tm_year = 45;
      tm.tm_mon = 8;
      mktime(&tm);
      print_globals(&tm);
    }

    Linux:

    $ gcc test-mktime.c && ./a.out
    1800-01: GMT/GMT (NULL) 0 0 (0)
    1800-01: EST/EDT (NULL) 1 18000 (0)
    1800-01: LMT/EDT (LMT) 1 18000 (17762)
    1943-01: EST/EWT (EWT) 1 18000 (14400)
    1945-09: EST/EPT (EPT) 1 18000 (14400)

    MacOS X:

    $ clang test-mktime.c && ./a.out
    1800-01: WILDABBR/WILDABBR (NULL) 0 0 (0)
    1800-01: EST/EDT (NULL) 1 18000 (0)
    1800-01: EST/EDT (NULL) 1 18000 (0)
    1943-01: EST/EWT (EWT) 1 18000 (14400)
    1945-09: EST/EPT (EPT) 1 18000 (14400)

    Indeed, mktime changes tzname as a side effect, but the result is system dependent (see LMT/EDT vs. EST/EDT in the third line).

    Furthermore, the values of daylight and timezone variables do not get updated, resulting in an inconsistent state. For this reason, I don't think Python should expose the new values of tzname. People who really need access to those can use ctypes.

    @abalkin abalkin closed this as completed Sep 13, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants