msg228690 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-06 14:20 |
Under certain circumstances, Modules/posixmodule.c will fail to compile due to a number of utime-related functions using a variable named "utime" when a function named "utime" already exists in the compiler's C header files. Specifically, if the following are undefined:
HAVE_UTIMENSAT
HAVE_UTIMES
and the following are defined:
HAVE_UTIMENSAT
HAVE_LUTIMES
HAVE_UTIME_H
the compiler will fail because the UTIME_TO_UTIMBUF module attempts to access utime->now when utime is acutually a function included from utime.h.
I've attached a patch that renames the uname functions' parameter "utime" to "ut" to avoid the conflict.
This bug was encountered using Open Watcom 2.0 (owcc) under GNU/Linux 32-bit.
|
msg228729 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-10-06 19:50 |
I don't understand. If utime is a *function*, then the local variable should simply take precedence. Do you possibly mean that utime is a *macro*?
What compilation error do you get?
|
msg228731 - (view) |
Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * |
Date: 2014-10-06 19:57 |
Maybe it is unnecessary strictness in Open Watcom compiler.
If renaming was necessary, then I suggest utime_obj instead of ut.
|
msg228732 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-06 20:01 |
Under the conditions I described in Modules/posixmodules.c on line 4815, the utime() function is called. With the current code, the following correct compiler error is emitted:
./Modules/posixmodule.c(4815): Error! E1012: Expression is not a function
The above occurs because utime is treated as a local variable and an attempt is made at calling it as a function.
The use of a possible (and fairly standard) function name as a local variable is unfortunate, and I'm guessing these conditions haven't been tested in a while.
|
msg228733 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-06 20:19 |
So in my original report, I actually got things backwards. The failure is because utime is a local variable, but a call to utime() is then attempted.
Regardless, the code is still completely broken in Modules/posixmodule.c under the conditions I described.
|
msg228754 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2014-10-07 06:59 |
Patch LGTM.
|
msg228862 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-10-09 11:55 |
New changeset 58601c36a357 by Victor Stinner in branch '3.4':
Issue #22568: Fix compilation of posixmodule.c with Open Watcom: rename "utime"
https://hg.python.org/cpython/rev/58601c36a357
New changeset 588657f910ac by Victor Stinner in branch 'default':
(Merge 3.4) Issue #22568: Fix compilation of posixmodule.c with Open Watcom:
https://hg.python.org/cpython/rev/588657f910ac
|
msg228863 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-10-09 11:56 |
> Patch LGTM.
The patch is safe. Even if I don't care of the Open Watcom compiler, the patch is simple enough to be accepted. (It doesn't add crazy #ifdef WATCOM).
I commited your change, thanks Jeffrey for your contribution.
|
msg228879 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2014-10-09 16:35 |
UTIME_TO_UTIMBUF actually still looks dodgy and UTIME_TO_TIME_T is completely broken.
* There should be only one "utimbuf" structure, because it already contains both times (see man 2 utime).
* UTIME_TO_TIME_T uses a type called "struct timet" which I don't think exists, it should be "time_t *". The "&" operator is wrong.
|
msg228880 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-09 16:37 |
My patch merely fixes broken code that wasn't being used by Python's "supported" compilers under most common configurations. It's really independent of compiler. I realize nobody here cares about Open Watcom as a supported compiler; the Python community has made that *abundantly* clear.
|
msg228882 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2014-10-09 16:52 |
Jeffrey: I did not mean to devalue your patch, I just wanted to bring even more issues to Victor's attention.
|
msg228884 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-09 16:56 |
Georg: Sorry, that wasn't directed at you. Your comment snuck in before mine. It was more general frustration.
|
msg228889 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-10-09 18:17 |
Georg, you want to take a swing at it, be my guest. The current mess is my doing, and I claim this is an *improvement* over what came before.
|
msg228891 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2014-10-09 18:46 |
I'm sure you've improved the code. Without looking at the hg history, you've likely just inherited the broken C code from the previous version and made it prettier :)
Since this breakage has not been reported so far, the last two #elses appear to be a combination of #defines that is very rare.
|
msg228897 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-10-09 19:47 |
> I realize nobody here cares about Open Watcom as a supported compiler; the Python community has made that *abundantly* clear.
I only heard once about Open Watcom, maybe 1 or 2 years ago. I see on http://www.openwatcom.org/ that the latest release is 4 years old.
Open Watcom is not packaged in my Linux Fedora 20. I downloaded the source code of the release 1.9 and I tried to compile it with build.sh. I had to modify build.sh to use bash instead of sh. Than the compilation of c/mglob.c failed with "Unknown CPU architecture", it looks like x86_64 CPU is not supported :-(
On which platform are you working? Dummy question, why not using a different compiler like GCC or Clang?
It was discussed recently to define rules to support a platform (ex: OpenBSD). For example, a requirement to have a buildbot and someone able to fix issues. I'm trying to help to support AIX even if I don't consider the platform important enough and I don't like proprietary software :) There is a buildbot and a developer proposing regulary patches.
If you want an official and full support of OpenWatcom, it's better to discuss it on the python-dev mailing list.
|
msg228898 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-10-09 19:49 |
> UTIME_TO_UTIMBUF actually still looks dodgy and UTIME_TO_TIME_T is completely broken.
Agreed. It looks like dead code which was never used nor tested.
@Jeffrey: Is your patch enough to compile posixmodule.c with OpenWatcom?
If it's enough, I will close the issue.
|
msg228907 - (view) |
Author: Jeffrey Armstrong (Jeffrey.Armstrong) * |
Date: 2014-10-09 20:26 |
The last "official" Open Watcom release (1.9) is indeed quite dated. The codebase was forked for a variety of reasons where development continues:
https://github.com/open-watcom/open-watcom-v2
The current development release does indeed build on x86_64 (the build system is quite complicated). I'm running Open Watcom nightly builds on Debian Wheezy x86 myself. I've mentioned supporting OW in Python, most notably in a PyCon lightning talk last year, but there really seems to be no interest even if there are some benefits. OW allows building the full interpreter and standard library on Windows, for example, without any MS tooling.
I was using it in this particular case as a benchmark for compile times for a presentation in a few weeks. OW is able to build the interpreter in ~25sec whereas GCC is taking 1min 10sec on my desktop. I don't have any plans to pursue Python support for OW any further, although I'm open to the idea.
This patch is enough to compile *this portion* of Modules/posixmodule.c with OW, but, no, it is not sufficient to compile the whole file. There are two more places where some preprocessor definitions cause issues. One is OW-specific, and I don't plan on filing a bug against it. The other is #22579. I only filed this bug because the code was broken.
|
msg228942 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-10-10 02:03 |
I remember manually setting/unsetting #defines to force alternate compilation paths. But I think there were one or two that I didn't have library support for (testing on Linux and Windows).
|
msg229120 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-10-12 06:45 |
New changeset 922526816b25 by Georg Brandl in branch '3.4':
Closes #22568: fix UTIME_TO_* macros in posixmodule for rare cases.
https://hg.python.org/cpython/rev/922526816b25
New changeset 0b2c7ea86d96 by Georg Brandl in branch 'default':
#22568: merge with 3.4
https://hg.python.org/cpython/rev/0b2c7ea86d96
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:08 | admin | set | github: 66758 |
2014-10-12 06:45:35 | python-dev | set | status: open -> closed resolution: fixed messages:
+ msg229120
stage: resolved |
2014-10-10 02:03:06 | larry | set | messages:
+ msg228942 |
2014-10-09 20:26:41 | Jeffrey.Armstrong | set | messages:
+ msg228907 |
2014-10-09 19:49:14 | vstinner | set | messages:
+ msg228898 |
2014-10-09 19:47:16 | vstinner | set | messages:
+ msg228897 |
2014-10-09 18:46:52 | georg.brandl | set | messages:
+ msg228891 |
2014-10-09 18:17:44 | larry | set | messages:
+ msg228889 |
2014-10-09 16:56:23 | Jeffrey.Armstrong | set | messages:
+ msg228884 |
2014-10-09 16:52:19 | georg.brandl | set | messages:
+ msg228882 |
2014-10-09 16:37:36 | Jeffrey.Armstrong | set | messages:
+ msg228880 |
2014-10-09 16:35:01 | georg.brandl | set | messages:
+ msg228879 |
2014-10-09 11:56:41 | vstinner | set | messages:
+ msg228863 |
2014-10-09 11:55:38 | python-dev | set | nosy:
+ python-dev messages:
+ msg228862
|
2014-10-07 10:40:20 | vstinner | set | nosy:
+ vstinner
|
2014-10-07 06:59:19 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg228754
|
2014-10-06 20:19:13 | Jeffrey.Armstrong | set | messages:
+ msg228733 |
2014-10-06 20:01:08 | Jeffrey.Armstrong | set | messages:
+ msg228732 |
2014-10-06 19:57:58 | Arfrever | set | nosy:
+ Arfrever messages:
+ msg228731
|
2014-10-06 19:50:26 | larry | set | messages:
+ msg228729 |
2014-10-06 14:28:17 | pitrou | set | nosy:
+ larry
|
2014-10-06 14:20:05 | Jeffrey.Armstrong | create | |