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

python PortGroup: require C11 with py313 packages due to atomics requirement, adjust a few subports #26830

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

barracuda156
Copy link
Contributor

@barracuda156 barracuda156 commented Nov 30, 2024

Description

This does not change anything for Pythons themselves; this simply extends requirement for C11 from Python 3.13 to py313 packages, otherwise those fail to build, since Python 3.13 checks for C11 atomics and does not find it for old Xcode gcc.

@reneeotten I do not know how wide is c99 requirement for python312, it may be that many ports pass -std=c99 anyway from upstream code, or it is a recent change, or I did not build affected ports earlier. Do you think this rather should also be done via PG?
Since pythons 311+ themselves cannot be built with pre-C11 compilers anyway, it may also be okay just to require it for py312+ ports, though it is not a requirement with py312- ones, strictly speaking.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.6
Xcode 3.2

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@lpsinger for port py-acor.
@IPGlider for port py-aiohttp, py-yarl.
@reneeotten for port py-bigfloat.
@tobypeterson for port py-propcache.

@barracuda156
Copy link
Contributor Author

With atomics, looks like this is a general requirement. At least I kept getting the same error with a sequence of py313 ports now (whichever had compiled code), like:

--->  Configuring py313-brotli
--->  Building py313-brotli
Executing:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-brotli/py313-brotli/work/brotli-1.1.0" && /opt/local/Library/Frameworks/Python.framework/Versions/3.13/bin/python3.13 -m build --no-isolation --wheel --outdir /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-brotli/py313-brotli/work 
* Getting build dependencies for wheel...
/opt/local/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/site-packages/setuptools/_distutils/dist.py:261: UserWarning: Unknown distribution option: 'test_suite'
  warnings.warn(msg)
running egg_info
creating python/Brotli.egg-info
writing python/Brotli.egg-info/PKG-INFO
writing dependency_links to python/Brotli.egg-info/dependency_links.txt
writing top-level names to python/Brotli.egg-info/top_level.txt
writing manifest file 'python/Brotli.egg-info/SOURCES.txt'
ERROR setuptools_scm._file_finders.git listing git files failed - pretending there aren't any
reading manifest file 'python/Brotli.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'python/_brotli.cc'
adding license file 'LICENSE'
writing manifest file 'python/Brotli.egg-info/SOURCES.txt'
* Building wheel...
/opt/local/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/site-packages/setuptools/_distutils/dist.py:261: UserWarning: Unknown distribution option: 'test_suite'
  warnings.warn(msg)
running bdist_wheel
running build
running build_py
creating bin/lib.macosx-10.6-ppc-cpython-313
copying python/brotli.py -> bin/lib.macosx-10.6-ppc-cpython-313
running build_ext
<string>:70: DeprecationWarning: dep_util is Deprecated. Use functions from setuptools instead.
building '_brotli' extension
creating bin/temp.macosx-10.6-ppc-cpython-313/python
creating bin/temp.macosx-10.6-ppc-cpython-313/c/common
creating bin/temp.macosx-10.6-ppc-cpython-313/c/dec
creating bin/temp.macosx-10.6-ppc-cpython-313/c/enc
/usr/bin/gcc-4.2 -fno-strict-overflow -Wsign-compare -fno-common -dynamic -DNDEBUG -g -O3 -Wall -pipe -Os -DOS_MACOSX=1 -Ic/include -I/opt/local/Library/Frameworks/Python.framework/Versions/3.13/include/python3.13 -c python/_brotli.c -o bin/temp.macosx-10.6-ppc-cpython-313/python/_brotli.o -arch ppc -isysroot/
In file included from /opt/local/Library/Frameworks/Python.framework/Versions/3.13/include/python3.13/pyatomic.h:9,
                 from /opt/local/Library/Frameworks/Python.framework/Versions/3.13/include/python3.13/Python.h:70,
                 from python/_brotli.c:2:
/opt/local/Library/Frameworks/Python.framework/Versions/3.13/include/python3.13/cpython/pyatomic.h:543:4: error: #error "no available pyatomic implementation for this platform/compiler"
error: command '/usr/bin/gcc-4.2' failed with exit code 1

ERROR Backend subprocess exited when trying to invoke build_wheel
Command failed:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-brotli/py313-brotli/work/brotli-1.1.0" && /opt/local/Library/Frameworks/Python.framework/Versions/3.13/bin/python3.13 -m build --no-isolation --wheel --outdir /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-brotli/py313-brotli/work 
Exit code: 1
Error: Failed to build py313-brotli: command execution failed

etc.

@kencu
Copy link
Contributor

kencu commented Nov 30, 2024

as python is needed to build clang, there is a potential for bootstrapping problems doing this. You'll have to check to make sure bootstrapping still can be done.

@barracuda156
Copy link
Contributor Author

as python is needed to build clang, there is a potential for bootstrapping problems doing this. You'll have to check to make sure bootstrapping still can be done.

@kencu This is not done for Python itself, Ken. It is in port group, and only for py313 ports. Does any Clang use py313 subports for bootstrap?

@barracuda156 barracuda156 changed the title python PG: require C11 with python313 due to atomics, adjust a few subports python PortGroup: require C11 with python313 due to atomics, adjust a few subports Nov 30, 2024
@kencu
Copy link
Contributor

kencu commented Nov 30, 2024

Some clangs are building with python310, some with python311. They do use various python supports I notice.

Usually -- if you require some functionality in the root (perl, python, R, etc) then you have to build the root with the enabled features (like atomics) and then the different modules the same way. It works like that with c+11 support, for example.

Just a heads-up though -- when you require newer compiler features for ports that are used building the toolchain, bootstrapping loops can happen and need to be looked for.

@barracuda156
Copy link
Contributor Author

@kencu Could you maybe take a look at the commits first? There is no change to pythons whatsoever. There is no global change for any python packages prior to py313.

Python313 port already sets C11:

compiler.c_standard 2011

So again, could you please illustrate which potential problem can requiring C11 create? And how someone would end up building a py313-* port without C11 compiler and without python313 itself, which requires C11 compiler? :)

@kencu
Copy link
Contributor

kencu commented Nov 30, 2024

My comment begins and ends with:

When you change how python works, you have to look at bootstrapping and make sure you haven't broken it, as python is part of the clang bootstrapping process.

The rest I leave to you, to make sure you're happy you haven't broken anything.

@barracuda156 barracuda156 changed the title python PortGroup: require C11 with python313 due to atomics, adjust a few subports python PortGroup: require C11 with py313 packages due to atomics requirement, adjust a few subports Nov 30, 2024
@tobypeterson
Copy link
Contributor

I wonder about the C99 changes - that is a Python header, should that be handled in the python PortGroup instead?

@barracuda156
Copy link
Contributor Author

I wonder about the C99 changes - that is a Python header, should that be handled in the python PortGroup instead?

That would probably make life easier, since while not all python312 ports need that, seems quite a number of them do.

@barracuda156
Copy link
Contributor Author

@reneeotten Could you say what is the correct way to pass -std=c99 via the PG? Given a number of failing ports otherwise, it makes sense indeed to move it to PG and avoid gazillion per-port hacks.
Just now, for example:

building 'statsmodels.tsa.stl._stl' extension
creating build/temp.macosx-10.6-ppc-cpython-312/statsmodels/tsa/stl
/usr/bin/gcc-4.2 -fno-strict-overflow -Wsign-compare -fno-common -dynamic -DNDEBUG -g -O3 -Wall -pipe -Os -arch ppc -isysroot/ -DCYTHON_TRACE_NOGIL=0 -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -I/opt/local/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/numpy/core/include -I/opt/local/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/numpy/core/include -I/opt/local/Library/Frameworks/Python.framework/Versions/3.12/include/python3.12 -c statsmodels/tsa/stl/_stl.c -o build/temp.macosx-10.6-ppc-cpython-312/statsmodels/tsa/stl/_stl.o
In file included from statsmodels/tsa/stl/_stl.c:32006:
/opt/local/Library/Frameworks/Python.framework/Versions/3.12/include/python3.12/internal/pycore_frame.h: In function ‘_PyFrame_Initialize’:
/opt/local/Library/Frameworks/Python.framework/Versions/3.12/include/python3.12/internal/pycore_frame.h:134: error: ‘for’ loop initial declaration used outside C99 mode
error: command '/usr/bin/gcc-4.2' failed with exit code 1

ERROR Backend subprocess exited when trying to invoke build_wheel
Command failed:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-statsmodels/py312-statsmodels/work/statsmodels-0.14.1" && /opt/local/Library/Frameworks/Python.framework/Versions/3.12/bin/python3.12 -m build --no-isolation --wheel --outdir /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_python_py-statsmodels/py312-statsmodels/work 
Exit code: 1

Just setting compiler.c_standard 1999 does not have a needed effect, since old gcc needs an explicit flag, and MacPorts does not do that.

@tobypeterson
Copy link
Contributor

I don't think I love the idea of explicitly passing -std=c99 either, because newer Xcode/OS combinations will default to a newer C standard than C99.

@tobypeterson
Copy link
Contributor

tobypeterson commented Dec 6, 2024

This almost seems like something that needs a bit of support in base. Some kind of settings like compiler.add_c_standard_argument and compiler.add_cxx_standard_argument, maybe some kind of way to specify a minimum standard version, and knowledge about default standard versions for each compiler so that we don't downgrade, etc?

Theoretically, it isn't necessary to code in all the knowledge about which each compiler defaults to, it could be tested with cc -E -dM -xc /dev/null | grep __STDC_VERSION__ or equivalent.

So maybe it should look more like compiler.minimum_c_standard 1999, and that would check what the compiler defaults to, then add the -std=c99 argument only if needed. There is the added complication of ports that may need GNU extensions (e.g. -std=gnu99).

This logic doesn't have to live in base, it's definitely something that could be tried out in the python portgroup at first, but I suspect that more and more ports will need changes like this to keep building on ancient OS/Xcode versions.

@tobypeterson
Copy link
Contributor

tobypeterson commented Dec 6, 2024

Regarding implementation, I think this could be done in the python group with the following logic, generally speaking:

  • Add another setting that would be handled similarly to, for example, python.add_archflags.
  • Make it default to an appropriate value for each python version.
  • See if the current compiler's default __STDC_VERSION__ is too old (this should probably live in a separate function), then lappend the appropriate -std=... argument to pycflags if needed.

@barracuda156
Copy link
Contributor Author

@tobypeterson In fact we can do it in a simple way, setting this flag only for *gcc-4.*. Since everything on buildbots works fine, that means no default clang needs this fix. Which leaves only Xcode gccs. There is arguably to need to bother about cases where a user forces some exotic compiler against MacPorts' defaults. Such solution should also be uncontroversial, as it only impacts < 10.7.

@barracuda156
Copy link
Contributor Author

@reneeotten @tobypeterson Could someone review this please? We need the fix.

# python3.12/internal/pycore_frame.h:134: error:
# ‘for’ loop initial declaration used outside C99 mode
if {[string match *gcc-4.* ${configure.compiler}]} {
python.add_cflags yes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does anything, since add_cflags is checked earlier in this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
Development

Successfully merging this pull request may close these issues.

6 participants