| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
!buildbot iOS |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 9c0b0d2 🤖 The command will test the builders whose names match following regular expression: iOS The builders matched are:
|
Sorry, something went wrong.
|
@ned-deily Are you able to explain the macOS build failures here? AFAICT, TARGET_OS_IPHONE and TARGET_OS_OSX should both be defined by importing AvailabilityMacros.h (which imports TargetConditionals.h). The current main branch has the exact same preprocessor directive on L2967, and it compiles fine - but adding a copy of the exact same preprocessor directive at the top of the file causes both directives to fail to compile. I can't reproduce these compilation errors locally, and I can't see anything obvious in the CI configuration that is different from my setup. As best as I can make out, the error would be caused if the compilation is being done by a "bare" gcc, rather than clang... but CI is finding the macOS SDK, which should (I think?) be making gcc an alias of clang. Am I missing something obvious? |
Sorry, something went wrong.
There was a problem hiding this comment.
It is normal for CI to fail because TARGET_OS_IPHONE is not defined.
Sorry, something went wrong.
If look at the SDK mirror, this never actually happens. |
Sorry, something went wrong.
There was a problem hiding this comment.
Updated review.
Sorry, something went wrong.
|
@aeiouaeiouaeiouaeiouaeiouaeiou
On older SDKs, sure. But the current mainline has a reference to TARGET_OS_IPHONE, and is currently passing CI.
It definitely does on more recent SDKs (MacOSX SDK 15.2 has this import chain). So - the changes you've suggested definitely make sense for guaranteeing support on older SDKs - but they don't explain why main is passing CI, but this PR doesn't. |
Sorry, something went wrong.
|
!buildbot iOS |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit f81a68e 🤖 The command will test the builders whose names match following regular expression: iOS The builders matched are:
|
Sorry, something went wrong.
|
And I've worked it out. os/log.h imports the target conditionals, so when it's included unconditionally, the later references work. However, when the import of os/log.h is made conditional, and you've got an SDK that doesn't include the target conditional import, it fails. There was also a stray usage of an OS_LOG symbol when the logging methods were registered, so I've made the compilation of the entire system logger tooling conditional on support existing in the first place. |
Sorry, something went wrong.
|
!buildbot iOS |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 0a689b3 🤖 The command will test the builders whose names match following regular expression: iOS The builders matched are:
|
Sorry, something went wrong.
|
Looks perfect, now I will test these changes locally on my system and VM. |
Sorry, something went wrong.
|
Sorry for a delay, I was away from my PowerPC until now. Will run the build now. |
Sorry, something went wrong.
|
@freakboy3742 Thank you! With your patches it builds now. P. S. There are two other errors:
/opt/local/bin/gcc-mp-14 -fno-strict-overflow -Wsign-compare -fno-common -dynamic -DNDEBUG -g -O3 -Wall -pipe -Os -arch ppc -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I./Include/internal/mimalloc -I. -I./Include -I/opt/local/include -DPy_BUILD_CORE_BUILTIN -c ./Modules/_threadmodule.c -o Modules/_threadmodule.o
In file included from ./Include/Python.h:64,
from ./Modules/_threadmodule.c:4:
./Modules/_threadmodule.c: In function '_thread__get_name_impl':
./Include/pymacro.h:105:5: error: passing argument 3 of 'pthread_getname_np' makes pointer from integer without a cast [-Wint-conversion]
105 | (sizeof(array) / sizeof((array)[0]))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
./Modules/_threadmodule.c:2381:47: note: in expansion of macro 'Py_ARRAY_LENGTH'
2381 | int rc = pthread_getname_np(thread, name, Py_ARRAY_LENGTH(name));
| ^~~~~~~~~~~~~~~
In file included from ./Include/cpython/pythread.h:17,
from ./Include/pythread.h:124,
from ./Include/Python.h:120:
/usr/include/pthread.h:360:52: note: expected 'size_t *' {aka 'long unsigned int *'} but argument is of type 'long unsigned int'
360 | int pthread_getname_np(pthread_t,char*,size_t*);
| ^~~~~~~
/opt/local/bin/gcc-mp-14 -fno-strict-overflow -Wsign-compare -fno-common -dynamic -DNDEBUG -g -O3 -Wall -pipe -Os -arch ppc -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I./Include/internal/mimalloc -I. -I./Include -I/opt/local/include -DPy_BUILD_CORE_BUILTIN -c ./Modules/posixmodule.c -o Modules/posixmodule.o
./Modules/posixmodule.c: In function '_pystatvfs_fromstructstatfs':
./Modules/posixmodule.c:13231:5: error: static assertion failed: "assuming large file"
13231 | _Static_assert(sizeof(st.f_blocks) == sizeof(long long), "assuming large file");
| ^~~~~~~~~~~~~~
The second is fixable via: --- Modules/posixmodule.c
+++ Modules/posixmodule.c 2024-10-31 03:05:47.000000000 +0800
@@ -13212,6 +13212,11 @@
* os.statvfs is implemented in terms of statfs(2).
*/
+#if defined(__i386__) || defined(__ppc__)
+#define statfs statfs64
+#define fstatfs fstatfs64
+#endif
+
static PyObject*
_pystatvfs_fromstructstatfs(PyObject *module, struct statfs st) {
PyObject *StatVFSResultType = get_posix_state(module)->StatVFSResultType;
For the first I just passed a flag to downgrade it back to a warning (pre-gcc14 behavior): -Wno-error=int-conversion. |
Sorry, something went wrong.
Thanks for that testing - I'll merge once I've got a review from someone on the core team (that should be mostly procedural, but might take a couple of days given the time of year).
This doesn't appear to be related to the os_log change; could I please ask you to report this as a separate issue.
When you say "known" - known to who? Is there an existing CPython ticket for this problem?
That seems like a reasonable patch based on my reading of the man page for statfs; If you want to submit that change (plus a small docstring explaining why/where the definition is needed) as a PR and tag me, I'll gladly review and merge it. |
Sorry, something went wrong.
|
@freakboy3742 this PR looks good as is, so when will it be merged? |
Sorry, something went wrong.
@freakboy3742 Thank you and sorry for a delay, a lot of stuff in a pipeline. I will deal with this in a day or two. |
Sorry, something went wrong.
I've been waiting for a review from another member of core; a lot of people have low availability over the new year break. @ned-deily Any chance you'll have time over the next few days to take a look at this? |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM. And I verified that current main builds fail on macOS 10.11 without this PR and no longer fail with it.
Sorry, something went wrong.
|
Thanks @freakboy3742 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
…thonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
|
GH-128575 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
…H-128165) (#128575) gh-128146: Exclude os/log.h import on older macOS versions. (GH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…thon#128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log.
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…s. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…ns. (pythonGH-128165) (python#128575) pythongh-128146: Exclude os/log.h import on older macOS versions. (pythonGH-128165) Reworks the handling of Apple system log handling to account for older macOS versions that don't provide os-log. (cherry picked from commit e837a1f) Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
| Back | FazBrowse Home | New Git URL |
#127592 recently added support for redirecting stdout/err to the Apple System logs, using the os_log API. This API was added in macOS 10.12; and while the usage of the API was gated with a preprocessor directive, the header declaration was not. This adds preprocessor gating on the header definition.
/cc @barracuda156