Marc-Andre Lemburg <m...@egenix.com> added the comment: Jeffrey Yasskin wrote: > > New submission from Jeffrey Yasskin <jyass...@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()). ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue9189> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
