Restore support macOS prior 10.15 #1878
Conversation
Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
WalkthroughThis pull request introduces a new GitHub Actions job for building and testing on macOS using Clang with a specific deployment target. Additionally, it enhances Apple platform support in the build configuration by conditionally checking for the availability of specific compiler flags and updating related variables in both the main CMake configuration and the configure script. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Job as "macOS Clang (Target 10.10) Job"
GitHub->>Job: Trigger CI workflow
Job->>Job: Set environment (macos-13, clang/clang++, CMake deployment target 10.10, linker flag -ld_classic)
Job-->>GitHub: Return test/build results
sequenceDiagram
participant Config as Configure Script
participant Temp as Temporary Source File (test.c)
participant Compiler as C Compiler
Config->>Temp: Create minimal test.c file
Config->>Compiler: Compile test.c with flag -Werror=unguarded-availability
Compiler-->>Config: Return compilation outcome
Config->>Config: Update ADDITIONAL_CHECK_FLAGS if compilation succeeds
Config->>Temp: Create minimal test.c file
Config->>Compiler: Compile test.c with flag -Werror=unguarded-availability-new
Compiler-->>Config: Return compilation outcome
Config->>Config: Update ADDITIONAL_CHECK_FLAGS if compilation succeeds
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CMakeLists.txt (1)
439-445: Apple-Specific Compiler Flag Check for macOS SupportThis block conditionally checks whether the compiler accepts the flag
-Wunguarded-availability-newon Apple platforms and, if so, setsCMAKE_REQUIRED_FLAGSto include it along with-Werror. This approach helps ensure that on systems where the flag is supported—typically newer compiler versions—the build process will catch any unguarded API availability issues. In the context of restoring support for macOS prior to 10.15, it might be useful to add an inline comment explicitly stating that this conditional logic is aimed at avoiding issues on older macOS deployments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/cmake.yml(1 hunks)CMakeLists.txt(1 hunks)configure(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (96)
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: Windows ClangCl Win32
- GitHub Check: EL9 Clang S390X DFLTCC MSAN
- GitHub Check: macOS GCC (ARM64)
- GitHub Check: macOS GCC
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix & Compat
- GitHub Check: Ubuntu GCC AARCH64 No ARMv8 UBSAN
- GitHub Check: Ubuntu GCC AARCH64 ASAN
- GitHub Check: macOS Clang Symbol Prefix
🔇 Additional comments (3)
configure (1)
721-731: LGTM! Good addition for macOS compatibility.The added check for
-Wunguarded-availability-newcompiler flag helps ensure compatibility with older macOS versions by catching usage of APIs that might not be available. Adding-Werrorensures that any availability warnings are treated as errors, which is particularly important for functions likealigned_allocthat were introduced in macOS 10.15..github/workflows/cmake.yml (1)
580-586: LGTM! Well-configured job for testing macOS 10.10 compatibility.The new job is well-configured for testing compatibility with older macOS versions:
- Uses macOS-13 runner for latest toolchain
- Sets deployment target to macOS 10.10
- Uses
-ld_classicto avoid compatibility issues with the new linker in Xcode 15CMakeLists.txt (1)
450-451: Resetting Compiler Flags to Maintain a Clean EnvironmentThe explicit reset of
CMAKE_REQUIRED_FLAGSimmediately after the Apple-specific block ensures that the temporary flags used for the check do not leak into subsequent compiler checks. This is a sound practice to avoid side effects in later parts of the CMake configuration.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1878 +/- ##
========================================
Coverage 82.37% 82.37%
========================================
Files 141 141
Lines 12556 12556
Branches 2894 2894
========================================
Hits 10343 10343
- Misses 1237 1242 +5
+ Partials 976 971 -5 ☔ View full report in Codecov by Sentry. |
|
Thanks very much. Pillow's users have tested this and confirmed that it works. |
|
@radarhere, recheck this PR, please, because I update it. |
CMakeLists.txt
Outdated
| if(APPLE) | ||
| check_c_compiler_flag(-Werror=unguarded-availability-new HAVE_W_ERROR_UNGUARDED_AVAILABILITY_NEW) | ||
| if(HAVE_W_ERROR_UNGUARDED_AVAILABILITY_NEW) | ||
| set(ADDITIONAL_CHECK_FLAGS "-Werror=unguarded-availability-new") | ||
| endif() | ||
| endif() | ||
|
|
There was a problem hiding this comment.
For 10.10 and later, the flag should be -Werror=unguarded-availability, as the -Werror=unguarded-availability-new flag only supports 10.13 and later.
configure
Outdated
| # Check for -Werror=unguarded-availability-new compiler support | ||
| echo "" > test.c | ||
| cat > $test.c <<EOF | ||
| int main() { return 0; } | ||
| EOF | ||
| if $cc $CFLAGS -Werror=unguarded-availability-new -c $test.c >> configure.log 2>&1; then | ||
| echo "Checking for -Werror=unguarded-availability-new... Yes." | tee -a configure.log | ||
| ADDITIONAL_CHECK_FLAGS="-Werror=unguarded-availability-new" | ||
| else | ||
| echo "Checking for -Werror=unguarded-availability-new... No." | tee -a configure.log | ||
| ADDITIONAL_CHECK_FLAGS="" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Same as with cmake, should check for -Werror=unguarded-availability.
…availability-new" and add it to maybe affected symbol checking Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
phprus
force-pushed
the
macos-legacy-1
branch
from
b9baebc to
52e5243
Compare
Pillow's users have tested the version from 13 hours ago, and confirmed that it works. |
|
@phprus The build is still failing there on But the failure seems the same: Am I doing this wrong? Is that CI inappropriate to build zlib-ng? Or should we reopen? |
|
@iago-lito Because The full cmake output is required for diagnostics. |
|
Okay thank you @phprus. If the full cmake output is not available in these logs (is it not?), then I'm afraid I can't produce it for diagnostic until I get direct acces to a macOS machine. I'll keep you posted. Do you think this belongs to another issue? |
|
I'm sorry, for unknown reason the github search didn't find the required part of the log. As I see: Thus, the If the Lines 8 to 9 in 50e9ca0 and the Lines 19 to 22 in 50e9ca0 Can you try building with |
|
Confusing indeeed @phprus. Here are the logs produced with this verbose option on: https://github.com/bioconda/bioconda-recipes/actions/runs/13947794014/job/39039746164?pr=54435. Does it help? |
|
Looks like a broken environment. When compiling
But the compiler does not see the definition of the |
|
Thank you. I'm reporting this to Bioconda. |
Possible fix for #1877
@radarhere, check this PR, please.
Summary by CodeRabbit