classification
Title: Separate compilation of time and datetime modules
Type: resource usage Stage: needs patch
Components: Windows Versions: Python 3.2
process
Status: closed Resolution: out of date
Dependencies: Superseder: Factorize code to convert int/float to time_t, timeval or timespec
View: 14180
Assigned To: Nosy List: BreamoreBoy, belopolsky, doko, georg.brandl, loewis, mark.dickinson, pitrou, r.david.murray, rhettinger, rnk, ruseel, steve.dower, tim.golden, tim.peters, tlesher, vstinner, zach.ware
Priority: high Keywords: patch

Created on 2010-06-16 15:46 by belopolsky, last changed 2014-06-22 07:57 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
timefunc-split.diff belopolsky, 2010-06-16 15:50
issue9012.diff belopolsky, 2010-06-16 16:57
time.diff doko, 2010-07-04 13:52 patch to build as a statically linked extension
issue9012a.diff belopolsky, 2010-07-04 14:15 Adding _time.c in Setup.dist
add_time_to_vc8_build.diff tlesher, 2010-07-21 20:49 Corrected relative path in _time.c for VS8 build
Messages (24)
msg107929 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-16 15:46
Here is the history of the issue per Martin v. Löwis on python-dev:

"""

This was added with

------------------------------------------------------------------------
r36221 | bcannon | 2004-06-24 03:38:47 +0200 (Do, 24. Jun 2004) | 3 Zeilen

Add compilation of timemodule.c with datetimemodule.c to get
__PyTime_DoubleToTimet().

------------------------------------------------------------------------

So it's clearly intentional. I doubt its desirable, though. If only
__PyTime_DoubleToTimet needs to be duplicated, I'd rather put that
function into a separate C file that gets included twice, instead of including the full timemodule.c into datetimemodule.c.
"""
-- "Sharing functions between C extension modules in stdlib", http://mail.python.org/pipermail/python-dev/2010-June/100587.html

I hope this is non-controversial, but I would like someone to comment on the choice of file name.  I chose _timefunc.c to make it more distinguishable from module file names.  Ideally, however I would like to see this in either _time.h/_time.c pair (both in Module dir like _math.{c,h}) or in pytime.h/pytime.c (like pymath.{c,h}).

Marking this as "resource usage" because the patch will result in smaller size of datetime module.
msg107936 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-16 16:47
Based on IRC discussion, here is a modified patch that places C code in _time.c and creates a stub for _time.h so that future shared definitions can go there.
msg107937 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-16 16:57
Added new Module/_time.h to the patch.
msg107961 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-16 22:40
Committed in r82034.
msg107962 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-06-16 22:49
Reopen: r82034 broke Windows build

http://www.python.org/dev/buildbot/all/builders/x86%20Windows7%203.x/builds/802/steps/compile/logs/stdio

-------
Build started: Project: pythoncore, Configuration: Debug|Win32
Linking...
   Creating library D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\PCbuild\python32_d.lib and object D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\PCbuild\python32_d.exp
datetimemodule.obj : error LNK2019: unresolved external symbol __PyTime_DoubleToTimet referenced in function _date_local_from_time_t
timemodule.obj : error LNK2001: unresolved external symbol __PyTime_DoubleToTimet
D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\PCbuild\\python32_d.dll : fatal error LNK1120: 1 unresolved externals
Build log was saved at "file://D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\PCbuild\Win32-temp-Debug\pythoncore\BuildLog.htm"
-------
msg107963 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2010-06-16 23:14
> Reopen: r82034 broke Windows build

Fixed by r82035.
msg109215 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-07-04 13:33
this breaks the build if the time and datetime extensions are built statically into the python interpreter.

The build fails with a link error; adding the _time extension to Modules/Setup.dist

@@ -160,6 +160,7 @@
 #cmath cmathmodule.c _math.c # -lm # complex math library functions
 #math mathmodule.c _math.c # -lm # math library functions, e.g. sin()
 #_struct _struct.c     # binary structure packing/unpacking
+#_time _time.c # # segregated code shared between time and datetime modules
 #time timemodule.c # -lm # time operations and variables
 #operator operator.c   # operator.add() and similar goodies
 #_weakref _weakref.c   # basic weak reference support

you get another link error:

libpython3.2.a(config.o):(.data+0x58): undefined reference to `PyInit__time'
collect2: ld returned 1 exit status

because _time isn't a proper extension.

raising the severity as this is a regression
msg109216 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-07-04 13:52
please find attached a patch to build as a statically linked extension, registering the empty _time module.
msg109219 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-04 14:15
_time.c is not supposed to be compiled into an extension.  It is similar to _math.c - if you add a line

_math _math.c

to Setup, you get a similar compile error. What needs to be done, however, is to add _time.c to time and datetime lines.  See issue9012a.diff.
msg109221 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-07-04 14:47
doesn't work:

ranlib libpython3.2.a
Modules/_time.o: In function `_PyTime_DoubleToTimet':
/scratch/packages/python/3.2/python3.2-3.2~~20100704/build-shared/../Modules/_time.c:11: multiple definition of `_PyTime_DoubleToTimet'
Modules/_time.o:/scratch/packages/python/3.2/python3.2-3.2~~20100704/build-shared/../Modules/_time.c:11: first defined here
collect2: ld returned 1 exit status
ln: accessing `libpython3.2.so.1.0': No such file or directory
make[1]: *** [libpython3.2.so] Error 1
msg109222 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-07-04 15:01
Matthias, does building both the math and cmath modules statically into the interpreter fail in a similar manner?
msg109224 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-07-04 15:17
yes, same issue with math/cmath.
msg109225 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-07-04 15:21
Thanks.  Looks like we need a general solution to the problem of shared (non-module) dependencies in modules.

So all that's needed is a way to stop _time.o showing up twice in MODOBJS in the Makefile, right?

(For some reason, ranlib on OS X doesn't seem to have any problem with multiply-defined symbols;  obviously Linux is different.)
msg110572 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-17 16:10
I am merging in the nosy list from issue9079 after we had a lengthy discussion there and on IRC about the best way to share code between stdlib extension modules.

For the issue9079, we decided to bring the shared code into python core, but this cannot be a general solution.  I still don't see any solution other than exposing C API via a capsule mechanism, but Antoine objected saying that capsule mechanism is for 3rd party modules and should not be used in stdlib.
msg111078 - (view) Author: Tim Lesher (tlesher) * Date: 2010-07-21 15:52
Added patch that replicates the change in r82035 for Visual Studio 2005 (VC8) builds.
msg111103 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-21 18:47
What about PC/VS7.1/pythoncore.vcproj?  Is there some automation in place to keep the project files in sync?  ISTM, all the info to generate these should be in setup.py.

The patch looks good, but I am hesitant to commit changes that I cannot test.
msg111109 - (view) Author: Tim Lesher (tlesher) * Date: 2010-07-21 19:58
No, there's no automated way to keep "legacy" Windows toolchains in sync; short of adopting something like Scons or CMAKE (which I'm *not* suggesting) I don't think I've seen a trustworthy way of doing so.

The PCBuild's "readme.txt" states:

"You can find build directories for older versions of Visual Studio and Visual C++ in the PC directory. The legacy build directories are no longer actively maintained and may not work out of the box."

To date, I believe that the attitude toward these older build files has been "unsupported; use at own risk; patches welcome".
msg111110 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-21 20:06
On Wed, Jul 21, 2010 at 3:58 PM, Tim Lesher <report@bugs.python.org> wrote:
..
> To date, I believe that the attitude toward these older build files has been
> "unsupported; use at own risk; patches welcome".
>

OK, I guess there is little risk in committing this patch.  I'll do it
later this week unless someone who actually knows anything about VC8
beats me to it. :-)
msg123892 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-13 19:02
Bump.  This bug has priority high and it sounds like the patch is ready for commit.
msg123895 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-12-13 19:29
> it sounds like the patch is ready for commit.

there are two pending patches here:

time.diff - a hack that makes _time.c look like a module while it is not.

add_time_to_vc8_build.diff - a patch for VC 8.0 project file.

I don't like time.diff because it fixes the symptom rather than the core problem.  I believe the code that is shared between C modules should be segregated into a true module which would expose its C API via capsule mechanism.  Others suggested that this is an overkill and that capsule mechanism is not suitable for the stdlib.  On the latter argument, I would like to point that unicodedata already uses capsule mechanism to make itself available to Python's builtin str type.

As I mentioned in msg109219, this situation is not unique to time/datetime.  The math and cmath modules similarly use linker tricks to share code in _math.c.

Unassigning because I don't have an informed opinion on add_time_to_vc8_build.diff and cannot move the _time.c issue forward until similar problem is solved for _math.c which I used as the basis for _time.c design.
msg219948 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-07 16:08
msg111078 refers to r82035 for Visual Studio 2005 (VC8) builds.  Given that http://code.activestate.com/lists/python-dev/131023/ refers to VC14 for 3.5 can we close this as out of date?
msg220064 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-06-08 22:47
No.  The problem has nothing to do with the VS version.
msg221199 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-21 20:41
@Steve/Zach any interest in this one?
msg221229 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-06-22 07:57
This issue has been superseded by #14180. __PyTime_DoubleToTimet no longer exists; its successor now lives in pytime.c.
History
Date User Action Args
2014-06-22 07:57:17loewissetstatus: open -> closed
superseder: Factorize code to convert int/float to time_t, timeval or timespec
resolution: out of date
messages: + msg221229
2014-06-21 20:41:45BreamoreBoysetnosy: + zach.ware, steve.dower
messages: + msg221199
2014-06-08 22:47:39r.david.murraysetmessages: + msg220064
2014-06-07 16:08:04BreamoreBoysetnosy: + BreamoreBoy
messages: + msg219948
2010-12-13 19:29:39belopolskysetassignee: belopolsky ->
messages: + msg123895
2010-12-13 19:02:26r.david.murraysetnosy: + r.david.murray
messages: + msg123892
2010-10-05 12:23:34ruseelsetnosy: + ruseel
2010-07-22 07:47:11brett.cannonsetnosy: - brett.cannon
2010-07-21 20:49:01tleshersetfiles: + add_time_to_vc8_build.diff
2010-07-21 20:47:43tleshersetfiles: - add_time_to_vc8_build.diff
2010-07-21 20:06:33belopolskysetmessages: + msg111110
2010-07-21 19:58:23tleshersetmessages: + msg111109
2010-07-21 18:47:15belopolskysetnosy: tim.peters, loewis, brett.cannon, georg.brandl, rhettinger, doko, mark.dickinson, belopolsky, pitrou, vstinner, tim.golden, tlesher, rnk
messages: + msg111103
components: + Windows
2010-07-21 15:52:52tleshersetfiles: + add_time_to_vc8_build.diff
nosy: + tlesher
messages: + msg111078

2010-07-17 16:11:16belopolskysetstage: resolved -> needs patch
2010-07-17 16:10:49belopolskysetnosy: + tim.peters, georg.brandl, rhettinger, pitrou, rnk
messages: + msg110572
2010-07-14 14:56:03belopolskysetnosy: + tim.golden
2010-07-04 15:21:04mark.dickinsonsetmessages: + msg109225
2010-07-04 15:17:16dokosetmessages: + msg109224
2010-07-04 15:01:38mark.dickinsonsetmessages: + msg109222
2010-07-04 14:47:55dokosetmessages: + msg109221
2010-07-04 14:15:08belopolskysetfiles: + issue9012a.diff

messages: + msg109219
2010-07-04 13:52:09dokosetfiles: + time.diff

messages: + msg109216
2010-07-04 13:33:25dokosetresolution: fixed -> (no value)
2010-07-04 13:33:09dokosetstatus: closed -> open
priority: normal -> high

nosy: + doko
messages: + msg109215
2010-06-16 23:14:21vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg107963
2010-06-16 22:49:31vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg107962

resolution: accepted -> (no value)
2010-06-16 22:40:01belopolskysetstatus: open -> closed

messages: + msg107961
stage: commit review -> resolved
2010-06-16 16:58:03belopolskysetfiles: - issue9012.diff
2010-06-16 16:57:56belopolskysetfiles: + issue9012.diff

messages: + msg107937
2010-06-16 16:47:27belopolskysetresolution: accepted
2010-06-16 16:47:12belopolskysetstage: patch review -> commit review
2010-06-16 16:47:02belopolskysetfiles: + issue9012.diff

messages: + msg107936
2010-06-16 15:50:50belopolskysetfiles: + timefunc-split.diff
keywords: + patch
2010-06-16 15:46:52belopolskycreate