Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

`./setup.py build` on macOS floods screen with system header warnings

Open Artoria2e5 opened this issue 1 year ago • 10 comments

On my machine, running ./setup.py build floods the screen with 206 irrelevant warnings from the system's stdlib.h, like this:

Library/Developer/CommandLineTools/SDKs/MacOSX14.2.sdk/usr/include/stdlib.h:352:38: warning: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Wnullability-completeness]
int      sradixsort(const unsigned char **__base, int __nel, const unsigned char *__table,
                                         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX14.2.sdk/usr/include/stdlib.h:352:38: note: insert '_Nullable' if the pointer may be null
int      sradixsort(const unsigned char **__base, int __nel, const unsigned char *__table,
                                         ^
                                           _Nullable 
/Library/Developer/CommandLineTools/SDKs/MacOSX14.2.sdk/usr/include/stdlib.h:352:38: note: insert '_Nonnull' if the pointer should never be null
int      sradixsort(const unsigned char **__base, int __nel, const unsigned char *__table,
                                         ^

So it looks like clang is not properly recognizing some includes as the system's own; https://stackoverflow.com/questions/48096872/shouldnt-clang-suppress-warnings-from-a-header-file-in-usr-local-include describes how.

This is how I think it happened:

  1. This is probably important for reproduction. I have both Xcode "Command Line Tools" and Xcode proper installed.
  2. setup.py side-steps the proper "-isystem" invocation and directly does _add_directory(include_dirs, os.path.join(sdk_path, "usr", "include")).
  3. Because -isystem is not used, clang does not know it's a system include file.
  4. Because I have multiple SDKs, the clang I use comes from /Applications/Xcode.app/Contents/Developer (according to xcode-select -p), which is a different one compared to the guy in /Library/Developer/CommandLineTools/usr/bin/. As a result, the -internal-isystem knowledge also fails.

This is how I think it should be fixed:

  • Inject a CFLAG with the value of [f"--system-header-prefix={sdk_path}"].

Now setuptools.CCompiler does not allow you to inject extra arguments, but setuptools.Extension does via the extra_compile_args member. Your modified build_ext should be able to do a dirty hack here: go through all the ext_modules to be built and give them some extra_compile_args (you're already doing it for fuzzing). You can even preserve the CFLAG suggestions from pkg-config this way, if you so wish.


Right, confirmed my cute little theory. I ran sudo xcode-select -s /Library/Developer/CommandLineTools so the clang is from the same place from my SDK. And it's quiet now.

It still needs a proper fix.

Artoria2e5 avatar Jan 29 '24 14:01 Artoria2e5

Invoking setup.py directly is deprecated:

https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html

Please could you try via pip, like pip install -e . or pip install .?

https://pillow.readthedocs.io/en/stable/installation.html

hugovk avatar Jan 29 '24 14:01 hugovk

Pip does not print the compiler's stderr (contains warnings and errors) unless there is a problem with the compilation. Because the HEAD builds (you have a CI for that), you never see the issue.

So all is fine I try to do something to some C extension code and break the build. At which point I am showered with the same irrelevant warnings, mixed with my actual error; worse, there are no colors. That's bad developer experience and what led me to do ./setup.py build in the first place: it rolls in real time and lets me see stderr colors to make sense of the flood of output.

Artoria2e5 avatar Jan 29 '24 14:01 Artoria2e5

Pip does not print the compiler's stderr (contains warnings and errors) unless there is a problem with the compilation. Because the HEAD builds (you have a CI for that), you never see the issue.

It does if you include -v:

pip install -v .

nulano avatar Jan 29 '24 15:01 nulano

Continuing the suggestion of using pip install in the future, https://github.com/python-pillow/Pillow/pull/7760 has now been merged, so that in the next version of Pillow, setup.py will no longer be executable.

radarhere avatar Jan 30 '24 08:01 radarhere

Can reproduce with pip3 install -v . (mac, or maybe brew, still has pip pointing to the old 2.7 stuff). Full repro procedure:

  1. Install Xcode CLT via xcode-select --install.
  2. Install Xcode the app from App Store.
  3. Run sudo xcode-select -s /Applications/Xcode.app/Contents/Developer to use the compiler from the App Store.
  4. Clone the repo
  5. pip3 install -v . &> ../log

I got a 20 MB log after about 1.5 minutes, so I called stop there. At least it compresses well. log.zip

(-v still doesn't have colors. It's hard to do terminal capture right, well.)

Artoria2e5 avatar Jan 31 '24 13:01 Artoria2e5

This is how I think it should be fixed:

  • Inject a CFLAG with the value of [f"--system-header-prefix={sdk_path}"].

I tried

diff --git a/setup.py b/setup.py
index 1bbd2c05c..3a384e38f 100644
--- a/setup.py
+++ b/setup.py
@@ -569,6 +569,9 @@ class pil_build_ext(build_ext):
             if sdk_path:
                 _add_directory(library_dirs, os.path.join(sdk_path, "usr", "lib"))
                 _add_directory(include_dirs, os.path.join(sdk_path, "usr", "include"))
+
+                for extension in self.extensions:
+                    extension.extra_compile_args = [f"--system-header-prefix={sdk_path}"]
         elif (
             sys.platform.startswith("linux")
             or sys.platform.startswith("gnu")

and that successfully added the flag, but didn't remove the warnings.

radarhere avatar Feb 23 '24 11:02 radarhere

I've created #7827 to resolve this by silencing the warnings.

radarhere avatar Feb 23 '24 13:02 radarhere

Huh... suboptimal. Maybe -isystem{os.path.join(sdk_path, "usr", "lib")} will do something, but I'm grasping at straws here. I also don't like how my suggestion is deviating more and more from the script's assumptions.

EDIT: not lib, include!

Artoria2e5 avatar Feb 23 '24 13:02 Artoria2e5

Maybe -isystem{os.path.join(sdk_path, "usr", "lib")} will do something

No, that also doesn't reduce the warnings.

radarhere avatar Feb 23 '24 13:02 radarhere

If it helps, I personally use Python from MacPorts by default, and using that version, these warnings aren't generated.

radarhere avatar Feb 24 '24 00:02 radarhere