python / cpython

The Python programming language
https://www.python.org/
Other
60.78k stars 29.34k forks source link

HAVE_CLOCK_GETTIME not repected in pytime.c #84355

Closed 782118db-faa4-4147-b3f9-be883fe8b38c closed 2 years ago

782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago
BPO 40174
Nosy @vstinner, @benjaminp, @pmp-p, @Jerome-PS, @iritkatriel
PRs
  • python/cpython#19374
  • python/cpython#19385
  • 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: ```python assignee = None closed_at = created_at = labels = ['interpreter-core', 'build', '3.8'] title = 'HAVE_CLOCK_GETTIME not repected in pytime.c' updated_at = user = 'https://github.com/Jerome-PS' ``` bugs.python.org fields: ```python activity = actor = 'iritkatriel' assignee = 'none' closed = True closed_date = closer = 'iritkatriel' components = ['Interpreter Core'] creation = creator = 'jerome.hamm' dependencies = [] files = [] hgrepos = [] issue_num = 40174 keywords = ['patch'] message_count = 10.0 messages = ['365703', '365712', '365751', '365758', '365775', '365794', '365800', '365822', '412100', '412166'] nosy_count = 6.0 nosy_names = ['vstinner', 'benjamin.peterson', 'pmpp', 'python-dev', 'jerome.hamm', 'iritkatriel'] pr_nums = ['19374', '19385'] priority = 'normal' resolution = None stage = 'resolved' status = 'closed' superseder = None type = 'compile error' url = 'https://bugs.python.org/issue40174' versions = ['Python 3.8'] ```

    782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago

    Hi,

    In the file Python/pytime.c (line 886 and others), functions and constants that I infer should be declared by HAVE_CLOCK_GETTIME are called without #ifdef.

    Best regards, Jérôme.

    benjaminp commented 4 years ago

    Does this cause a problem? At this point, it might be preferable to just assume clock_gettime exists on POSIX systems.

    782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago

    Hold on, this was the case with Python 3.8.2, but I just checked Github and the code is different! I'll have to check again with HEAD, and sorry if it was fixed!

    "Drop your panties sir William, I cannot wait till lunchtime".

    782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago

    Hi, OK, I was looking at the wrong line numbers, the problem is still there and as follows. You might call me a perfectionist, but if HAVE_CLOCK_GETTIME is not defined, the function pytime_fromtimespec is taken out by the preprocessor, but still called by the function pymonotonic, so python does not compile. I'm thinking of two ways to go, either stop configure if HAVE_CLOCK_GETTIME is not defined, or rewrite the function differently (I don't know how badly a truly monotonic clock is needed). It does feel strange to me to partially only honor HAVE_CLOCK_GETTIME.

    "With government backing, I could make it very silly"

    vstinner commented 4 years ago

    Python has a long history (30 years). time.time() had multiple implementations. When I added time.monotonic() in Python 3.3, it was optional. I changed that in Python 3.5: time.monotonic() is now always available. https://docs.python.org/dev/library/time.html#time.monotonic

    time.monotonic() has multiple implementations:

    So far (since Python 3.5), no one complained about build error on any platform. It looks safe in practice to expect clock_gettime() to be available.

    I suggest to close this issue, and only change the code is Python fails to build on a specific platform.

    By the way, glibc 2.31 release notes: "We plan to remove the obsolete function ftime, and the header \<sys/timeb.h>, in a future version of glibc."

    782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago

    Hi,

    I think it is nice to inform the user that clock_gettime is mandatory (just like pthreads, see PR). Can be nice for someone trying to port it to a new platform.

    "It's no particularly silly, is it?"

    vstinner commented 4 years ago

    Usually, what I did to use #error in the C code. Something like:

    #ifdef MS_WINDOWS
    ...
    #elif defined(...)
    ...
    #else
    #  error "your platform doesn't support monotonic clock"
    #endif

    I dislike relying on configure for that, it's too far from the actual implementation (like pymonotonic()).

    782118db-faa4-4147-b3f9-be883fe8b38c commented 4 years ago

    Yes, I get your meaning, and I totally agree with it, I was wondering what was the best way to do it, stay close to the cause, which would allow to more easily go along with changes, or put it in the configure file and warn the user as soon as possible... It was kind of a flip a coin decision in the end.

    For my information, is there a kind of committee or someone taking these kinds of decisions or at least expressing rules as to the spirit in which they should be made?

    On the same track, how can I find why someone wrote " / Various compilers have only certain posix functions */ / XXX Gosh I wish these were all moved into pyconfig.h */ " in posixmodule.c? Is it just because no-one had the opportunity to do it and it is OK for me to do it?

    "Fetchez la vache"

    iritkatriel commented 2 years ago

    For my information, is there a kind of committee or someone taking these kinds of decisions or at least expressing rules as to the spirit in which they should be made?

    You can bring this up on the python-ideas mailing list if you want to pull in additional core developers into the discussion.

    iritkatriel commented 2 years ago

    I'll close this in a week if there is no response.

    (It seems to have been abandoned by the OP after a minor code review disagreement if I understand their last comment.)