Skip to content

./configure incorrectly adds mmacosx-version-min to iOS/watchOS/tvOS builds #6138

Closed
@hamstergene

Description

@hamstergene

Problem

./configure script contains this piece:

  19975     if test -z "$(echo $CFLAGS | grep m.*os.*-version-min)"; then
  19976       min="-mmacosx-version-min=10.8"                                                                                                   
  19977       CFLAGS="$CFLAGS $min"

It breaks iOS/watchOS/tvOS (any non-macOS) build if -miphoneos-version-min is given as part of CC rather than part of CFLAGS for example:

CC=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc  -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.2.sdk -miphoneos-version-min=9.0
CFLAGS=-g -fPIC

...or if the minimum version is provided via an environment variable instead:

export IPHONEOS_DEPLOYMENT_TARGET=9.0

What I expected

  1. curl configure script either needs better detection of *-version-min presence
  2. or maybe it needs to lose that mmacosx-version-min piece and let the user take care of that

Context

The reason I tried providing essential flags via CC instead of CFLAGS is #3189 — the true cause of that bug was that ./configure was invoking preprocessor as $CC -E without the essential -isysroot flag which caused some autoconf checks to fail to find any system headers. The advice inside #3189 to update command line tools is not correct way to do it; the correct solution is to give sysroot/target/etc to both CFLAGS, CPPFLAGS, and LDFLAGS.

Activity

bagder

bagder commented on Oct 28, 2020

@bagder
Member

Maybe we could just extend the check and also see if the user has set the option in CC? Like this:

diff --git a/acinclude.m4 b/acinclude.m4
index e7a36e4bd..f78bb547d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -2524,13 +2524,13 @@ AC_DEFUN([CURL_MAC_CFLAGS], [
 
   AC_MSG_CHECKING([for good-to-use Mac CFLAGS])
   AC_MSG_RESULT([$tst_cflags]);
 
   if test "$tst_cflags" = "yes"; then
-    AC_MSG_CHECKING([for *version-min in CFLAGS])
+    AC_MSG_CHECKING([for *version-min in CFLAGS or CC])
     min=""
-    if test -z "$(echo $CFLAGS | grep m.*os.*-version-min)"; then
+    if test -z "$(echo $CFLAGS $CC | grep m.*os.*-version-min)"; then
       min="-mmacosx-version-min=10.8"
       CFLAGS="$CFLAGS $min"
     fi
     if test -z "$min"; then
       AC_MSG_RESULT([set by user])
bagder

bagder commented on Oct 28, 2020

@bagder
Member

We could perhaps even check $IPHONEOS_DEPLOYMENT_TARGET in there?

added a commit that references this issue on Oct 29, 2020
ee695f0
hamstergene

hamstergene commented on Oct 29, 2020

@hamstergene
Author

Grepping $CC $CFLAGS should cover most users I think. I don't know how many users use environment variables, but on my memory I can't recall seeing that in the wild often.

Checking environment may be a bit more complicated:

  % strings /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang | grep DEPLOYMENT_TARGET
MACOSX_DEPLOYMENT_TARGET
IPHONEOS_DEPLOYMENT_TARGET
TVOS_DEPLOYMENT_TARGET
WATCHOS_DEPLOYMENT_TARGET
BRIDGEOS_DEPLOYMENT_TARGET
DRIVERKIT_DEPLOYMENT_TARGET

EDIT: the problem with environment variables is that we don't know if they will actually be used. Maybe IPHONEOS_DEPLOYMENT_TARGET is set, but the compiler will be targeting Mac? IMO that's bad design from Apple side, there is just no way for curl to have it 100% right in all cases.

added a commit that references this issue on Oct 29, 2020
8bf588b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @bagder@hamstergene

      Issue actions

        ./configure incorrectly adds mmacosx-version-min to iOS/watchOS/tvOS builds · Issue #6138 · curl/curl