This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author lemburg
Recipients Arfrever, brett.cannon, jyasskin, lemburg, loewis, mark.dickinson, pitrou
Date 2010-07-07.22:21:19
SpamBayes Score 0.0014100767
Marked as misclassified No
Message-id <4C34FDDD.90604@egenix.com>
In-reply-to <1278517873.84.0.191654592449.issue9189@psf.upfronthosting.co.za>
Content
Jeffrey Yasskin wrote:
> 
> New submission from Jeffrey Yasskin <jyasskin@gmail.com>:
> 
> Patch at http://codereview.appspot.com/1749042.
> 
> The idea here is to let the user set CFLAGS on either configure or make (or both), and have later settings appear later in the $CC command line. I left OPT, BASECFLAGS, and EXTRA_CFLAGS around in case people have written scripts using them, but I think they're superfluous as user-visible knobs after this patch.

Passing in user values to CFLAGS on configure should already work
without the patch. I'm not sure why you'd want to modify CFLAGS
on Makefile.

> I prevented AC_PROG_CC from setting a default $CFLAGS value because the values it would set are already put into $BASECFLAGS when appropriate, and @CFLAGS@ needs to appear after @BASECFLAGS@ to allow the user to override Python's defaults at configure time. We could also accomplish this by removing BASECFLAGS and OPT entirely and instead prepending their contents to $CFLAGS in configure. That's a bigger patch, but if any of you feel strongly about it I can do that instead.

We've already had a long discussion about the use of AC_PROG_CC on another
ticket. We are you trying to remove the default setting again ?

BASECFLAGS is not supported by autoconf, it's a Python invention. Do you
really want to put all the logic that autoconf uses to setup CFLAGS currently
and in the future into our own configure code ? Are you willing to maintain
that code ?

We've also discussed switching the order of $(OPT) and $(CFLAGS): that doesn't
work, since $(OPT) is meant to override the $(CFLAGS) settings.

> I made the same changes for CPPFLAGS and LDFLAGS but no other user-settable variables. I don't have strong opinions about the exact set we support this for, as long as it includes CFLAGS, but these three seemed like a sensible set.

Some more comments:

I don't really like the approach you've taken with the variable naming.
It's better to stick to Makefile all caps standards and instead prepend
Python specific variables with a PY or PY_ prefix. The already existing
PY_CFLAGS should then be renamed to PY_RUNTIME_CFLAGS or just PYTHON_CFLAGS.

Please also keep the default values for CFLAGS et al around, ie.

CFLAGS = @CFLAGS@
PY_CFLAGS = $(BASECFLAGS) $(CFLAGS) $(OPT) $(EXTRA_CFLAGS)

Note that using ":=" will cause trouble with non-GNU makes. Why do
you need this ?

In summary: While we're breaking Makefile parsing anyway, we might as
well go for the big patch and remove the whole BASECFLAGS, OPT and
EXTRA_CFLAGS cruft. These cause all sorts of problem when building
extensions for Python and not even distutils always gets them
right (distutils needs to mimic the CFLAGS setup when setting up
the compiler, so it has to replicate what's going on in the Makefile
in distutils.utils.ccompiler.customize_compiler()).
History
Date User Action Args
2010-07-07 22:21:22lemburgsetrecipients: + lemburg, loewis, brett.cannon, mark.dickinson, pitrou, jyasskin, Arfrever
2010-07-07 22:21:20lemburglinkissue9189 messages
2010-07-07 22:21:19lemburgcreate