Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3967)

Issue 1749042: Improve Python's handling of CFLAGS and friends (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 9 months ago by Jeffrey Yasskin
Modified:
13 years, 9 months ago
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Description

http://bugs.python.org/issue9189 `../src/configure --with-pydebug && make` originally produced gcc -c -fno-strict-aliasing -g -O2 -g -O0 -Wall -Wstrict-prototypes -I. -IInclude -I../src/Include -DPy_BUILD_CORE -o Python/pythonrun.o ../src/Python/pythonrun.c After this patch, that becomes gcc -c -fno-strict-aliasing -g -O0 -Wall -Wstrict-prototypes -I. -IInclude -I../src/Include -DPy_BUILD_CORE -o Python/pythonrun.o ../src/Python/pythonrun.c For `../src/configure --with-pydebug CFLAGS=-g3 && make CFLAGS=-Werror`, we get gcc -c -fno-strict-aliasing -g -O0 -Wall -Wstrict-prototypes -g3 -I. -IInclude -I../src/Include -DPy_BUILD_CORE -o Python/pythonrun.o ../src/Python/pythonrun.c Without pydebug, we get: gcc -c -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I../src/Include -DPy_BUILD_CORE -o Python/pythonrun.o ../src/Python/pythonrun.c

Patch Set 1 #

Patch Set 2 : + NEWS entry #

Total comments: 1

Patch Set 3 : Fix Brett's comment #

Patch Set 4 : Fix some of Marc-Andre's comments #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -38 lines) Patch
M Lib/sysconfig.py View 1 2 3 1 chunk +5 lines, -0 lines 2 comments Download
M Makefile.pre.in View 1 2 3 11 chunks +25 lines, -19 lines 0 comments Download
M Misc/NEWS View 2 1 chunk +6 lines, -0 lines 0 comments Download
M Modules/makesetup View 1 chunk +1 line, -1 line 0 comments Download
M configure View 4 chunks +10 lines, -11 lines 0 comments Download
M configure.in View 1 chunk +6 lines, -7 lines 0 comments Download

Messages

Total messages: 3
brett.cannon
LGTM short of one letter change in the NEWS entry. http://codereview.appspot.com/1749042/diff/2001/3003 File Misc/NEWS (right): http://codereview.appspot.com/1749042/diff/2001/3003#newcode1513 ...
13 years, 9 months ago (2010-07-07 04:07:22 UTC) #1
Antoine Pitrou
http://codereview.appspot.com/1749042/diff/9001/10004 File Lib/sysconfig.py (right): http://codereview.appspot.com/1749042/diff/9001/10004#newcode265 Lib/sysconfig.py:265: done[var] = done['PY_' + var] Why do you need ...
13 years, 9 months ago (2010-07-09 00:16:12 UTC) #2
Jeffrey Yasskin
13 years, 9 months ago (2010-07-09 00:55:48 UTC) #3
http://codereview.appspot.com/1749042/diff/9001/10004
File Lib/sysconfig.py (right):

http://codereview.appspot.com/1749042/diff/9001/10004#newcode265
Lib/sysconfig.py:265: done[var] = done['PY_' + var]
On 2010/07/09 00:16:13, Antoine Pitrou wrote:
> Why do you need to do that?
> 
> Right now, you have:
> 
> >>> sysconfig.get_config_var('CFLAGS')
> '-g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes'
> >>> sysconfig.get_config_var('PY_CFLAGS')
> '-g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude
> -I./Include  -DPy_BUILD_CORE'

The above is the behavior in SVN. Well, I get:
>>> sysconfig.get_config_var('CFLAGS')
'-fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes'
>>> sysconfig.get_config_var('PY_CFLAGS')
'-fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes
-I. -IInclude -I../src/Include  -DPy_BUILD_CORE'
>>> sysconfig.get_config_var('PY_CORE_CFLAGS')
>>> 

After the patch, I get:
>>> sysconfig.get_config_var('CFLAGS')
'-fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes'
>>> sysconfig.get_config_var('PY_CFLAGS')
'-fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes'
>>> sysconfig.get_config_var('PY_CORE_CFLAGS')
'-fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I.
-IInclude -I../src/Include  -DPy_BUILD_CORE'

Note that after this patch, CFLAGS isn't assigned in the Makefile, so it would
have an empty value, were it not for the above lines.
Parts of distutils assume that 'CFLAGS' will have the above value. So I kept
backward-compatibility by assigning the new values back to the old names.
Distutils doesn't seem to look for the old value of 'PY_CFLAGS' (which I renamed
to PY_CORE_CFLAGS at Marc-Andre's request). That's logical, since distutils
doesn't want to define Py_BUILD_CORE.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b