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

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

Closed
Artoria2e5 opened this issue Jan 29, 2024 · 10 comments · Fixed by #7827
Closed

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

Artoria2e5 opened this issue Jan 29, 2024 · 10 comments · Fixed by #7827

Comments

@Artoria2e5
Copy link

Artoria2e5 commented Jan 29, 2024

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.

@hugovk
Copy link
Member

hugovk commented Jan 29, 2024

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

@Artoria2e5
Copy link
Author

Artoria2e5 commented Jan 29, 2024

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.

@nulano
Copy link
Contributor

nulano commented Jan 29, 2024

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 .

@radarhere
Copy link
Member

Continuing the suggestion of using pip install in the future, #7760 has now been merged, so that in the next version of Pillow, setup.py will no longer be executable.

@Artoria2e5
Copy link
Author

Artoria2e5 commented Jan 31, 2024

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.)

@radarhere
Copy link
Member

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
Copy link
Member

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

@Artoria2e5
Copy link
Author

Artoria2e5 commented Feb 23, 2024

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!

@radarhere
Copy link
Member

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

No, that also doesn't reduce the warnings.

@radarhere
Copy link
Member

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

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

Successfully merging a pull request may close this issue.

4 participants