Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CXXFLAGS overrides default flags, CFLAGS doesn't #299

Closed
mglisse opened this issue Sep 21, 2024 · 4 comments · Fixed by #322
Closed

CXXFLAGS overrides default flags, CFLAGS doesn't #299

mglisse opened this issue Sep 21, 2024 · 4 comments · Fixed by #322

Comments

@mglisse
Copy link

mglisse commented Sep 21, 2024

cflags = _add_flags(cflags, 'C')
ldshared = _add_flags(ldshared, 'C')
cxxflags = os.environ.get('CXXFLAGS', cxxflags)

distutils now uses CXXFLAGS instead of CFLAGS for C++, that's good. However, it was implemented differently: while CFLAGS is appended to the default flags, CXXFLAGS replaces them. Is there a particular reason for this inconsistency? I have an application where I was setting both CFLAGS and CXXFLAGS to the same value, and was quite confused when the behavior changed with the new setuptools.

thesamesam added a commit to thesamesam/distutils that referenced this issue Dec 22, 2024
Since 2c93711, CXXFLAGS is used
as-is if set in the envionment rather than clobbered by whatever
CPython happened to be built with.

Do the same for CFLAGS: use it as-is if set in the environment, don't
prepend CPython's saved flags.

Fixes: pypa#299
thesamesam added a commit to thesamesam/distutils that referenced this issue Dec 22, 2024
Since 2c93711, CXXFLAGS is used
as-is if set in the envionment rather than clobbered by whatever
CPython happened to be built with.

Do the same for CFLAGS: use it as-is if set in the environment, don't
prepend CPython's saved flags.

Fixes: pypa#299
@jaraco jaraco closed this as completed in af7fcbb Dec 27, 2024
thesamesam added a commit to thesamesam/setuptools that referenced this issue Feb 21, 2025
Update the docs to reflect the distutils change for pypa/distutils#299
in af7fcbb.

Closes: pypa#4836
thesamesam added a commit to thesamesam/setuptools that referenced this issue Feb 21, 2025
Update the docs to reflect the distutils change for pypa/distutils#299
in af7fcbb.

Closes: pypa#4836
@cjwatson
Copy link

@thesamesam @jaraco I encountered a weird regression which I think is due to this change, and wondered where it ought to be fixed.

The regression is visible in https://bugs.debian.org/1098602, though it's quite confusing. From what I can tell, if Python is built with -DNDEBUG, and one tries to build persistent with CFLAGS set to something that doesn't also contain -DNDEBUG (in particular, Debian's packaging toolchain sets various hardening flags, but the problem can be reproduced by just setting CFLAGS to the empty string in the environment), then its extension fails to compile as follows:

x86_64-linux-gnu-gcc -fPIC -I/usr/include/python3.13 -c src/persistent/cPickleCache.c -o build/temp.linux-x86_64-cpython-313/src/persistent/cPickleCache.o
In file included from /usr/include/python3.13/Python.h:19,
                 from src/persistent/_compat.h:18,
                 from src/persistent/cPersistence.h:18,
                 from src/persistent/cPickleCache.c:96:
src/persistent/cPickleCache.c: In function ‘cc_oid_unreferenced’:
src/persistent/cPickleCache.c:626:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  626 |     assert(dead_pers_obj->ob_refcnt == 0);
      |                         ^~
src/persistent/cPickleCache.c:626:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  626 |     assert(dead_pers_obj->ob_refcnt == 0);
      |                         ^~
src/persistent/cPickleCache.c:636:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  636 |     assert(dead_pers_obj->ob_refcnt == 1);
      |                         ^~
src/persistent/cPickleCache.c:636:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  636 |     assert(dead_pers_obj->ob_refcnt == 1);
      |                         ^~
src/persistent/cPickleCache.c:665:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  665 |     assert(dead_pers_obj->ob_refcnt == 1);
      |                         ^~
src/persistent/cPickleCache.c:665:25: error: ‘cPersistentObject’ has no member named ‘ob_refcnt’
  665 |     assert(dead_pers_obj->ob_refcnt == 1);
      |                         ^~
error: command '/usr/bin/x86_64-linux-gnu-gcc' failed with exit code 1

Previously extensions were built with Python's own CFLAGS even if CFLAGS was set in the environment, so this wasn't a problem.

I don't know Python's C API well enough to determine whether this is a bug in persistent, a bug in this distutils change (via setuptools), or something else. But it seems like something you might want to know about.

@thesamesam
Copy link
Contributor

Thanks @cjwatson. Let me take a look.

@cjwatson
Copy link

On second thoughts, based on pypa/setuptools#4836, I think this is just a bug in persistent that's been exposed by this change. I've proposed zopefoundation/persistent#214 to fix it.

@thesamesam
Copy link
Contributor

Many thanks! That looks right to me and that the assertions were broken (and clearly haven't worked for at least some time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants