libvpx: configure.sdkroot: command not found

[ryandesign (Ryan Carsten Schmidt)]

libvpx was patched to find the right sdk, but the patch is wrong. The patch makes this change to configure.sh

-      osx_sdk_dir="$(show_darwin_sdk_path macosx)"
+      osx_sdk_dir="$(configure.sdkroot)"

This makes no sense: configure.sh is a shell script, and ${configure.sdkroot} is a MacPorts Tcl variable. Shell scripts do not have access to Tcl variables, nor is that the syntax for accessing a shell variable; that's the syntax for running a shell function or command, and there is no shell command called configure.sdkroot, which configure.sh tells us if we run it:

./build/make/configure.sh: line 857: configure.sdkroot: command not found

[Gcenx]

Ah I hadn’t noticed since libvpx had still built with each test. I’ll change the patch and test it again so this won’t be an issue

Since the port still builds even with the issue I’m thinking of just removing the related section from configure.sh

Edit;
Remade the patch and just removed that section since macports built libvpx even with the incorrect usage of configure.sdkroot

--- build/make/configure.sh.orig.sh	2019-07-15 14:55:32.000000000 -0400
+++ build/make/configure.sh	        2019-10-16 22:59:42.000000000 -0400
@@ -767,37 +767,9 @@
 
     # detect tgt_os
     case "$gcctarget" in
-      *darwin10*)
+      *darwin*)
         tgt_isa=x86_64
-        tgt_os=darwin10
-        ;;
-      *darwin11*)
-        tgt_isa=x86_64
-        tgt_os=darwin11
-        ;;
-      *darwin12*)
-        tgt_isa=x86_64
-        tgt_os=darwin12
-        ;;
-      *darwin13*)
-        tgt_isa=x86_64
-        tgt_os=darwin13
-        ;;
-      *darwin14*)
-        tgt_isa=x86_64
-        tgt_os=darwin14
-        ;;
-      *darwin15*)
-        tgt_isa=x86_64
-        tgt_os=darwin15
-        ;;
-      *darwin16*)
-        tgt_isa=x86_64
-        tgt_os=darwin16
-        ;;
-      *darwin17*)
-        tgt_isa=x86_64
-        tgt_os=darwin17
+        tgt_os=darwin
         ;;
       x86_64*mingw32*)
         tgt_os=win64
@@ -881,65 +853,6 @@
         add_ldflags "-isysroot ${iphoneos_sdk_dir}"
       fi
       ;;
-    x86*-darwin*)
-      osx_sdk_dir="$(show_darwin_sdk_path macosx)"
-      if [ -d "${osx_sdk_dir}" ]; then
-        add_cflags  "-isysroot ${osx_sdk_dir}"
-        add_ldflags "-isysroot ${osx_sdk_dir}"
-      fi
-      ;;
-  esac
-
-  case ${toolchain} in
-    *-darwin8-*)
-      add_cflags  "-mmacosx-version-min=10.4"
-      add_ldflags "-mmacosx-version-min=10.4"
-      ;;
-    *-darwin9-*)
-      add_cflags  "-mmacosx-version-min=10.5"
-      add_ldflags "-mmacosx-version-min=10.5"
-      ;;
-    *-darwin10-*)
-      add_cflags  "-mmacosx-version-min=10.6"
-      add_ldflags "-mmacosx-version-min=10.6"
-      ;;
-    *-darwin11-*)
-      add_cflags  "-mmacosx-version-min=10.7"
-      add_ldflags "-mmacosx-version-min=10.7"
-      ;;
-    *-darwin12-*)
-      add_cflags  "-mmacosx-version-min=10.8"
-      add_ldflags "-mmacosx-version-min=10.8"
-      ;;
-    *-darwin13-*)
-      add_cflags  "-mmacosx-version-min=10.9"
-      add_ldflags "-mmacosx-version-min=10.9"
-      ;;
-    *-darwin14-*)
-      add_cflags  "-mmacosx-version-min=10.10"
-      add_ldflags "-mmacosx-version-min=10.10"
-      ;;
-    *-darwin15-*)
-      add_cflags  "-mmacosx-version-min=10.11"
-      add_ldflags "-mmacosx-version-min=10.11"
-      ;;
-    *-darwin16-*)
-      add_cflags  "-mmacosx-version-min=10.12"
-      add_ldflags "-mmacosx-version-min=10.12"
-      ;;
-    *-darwin17-*)
-      add_cflags  "-mmacosx-version-min=10.13"
-      add_ldflags "-mmacosx-version-min=10.13"
-      ;;
-    *-iphonesimulator-*)
-      add_cflags  "-miphoneos-version-min=${IOS_VERSION_MIN}"
-      add_ldflags "-miphoneos-version-min=${IOS_VERSION_MIN}"
-      iossim_sdk_dir="$(show_darwin_sdk_path iphonesimulator)"
-      if [ -d "${iossim_sdk_dir}" ]; then
-        add_cflags  "-isysroot ${iossim_sdk_dir}"
-        add_ldflags "-isysroot ${iossim_sdk_dir}"
-      fi
-      ;;
   esac
 
   # Handle Solaris variants. Solaris 10 needs -lposix4

[kencu (Ken)]

But we have systems using libvpx that go back to 10.5. It is going to need more testing that just on 10.14.

[kencu (Ken)]

You might set a variable in the Portfile to the value of ${configure.skdroot} and pass it in to the shell script on the build line.

I've seen Jeremy do that many times, eg <​https://github.com/macports/macports-ports/blob/master/lang/libcxx/Portfile#L192>

or similarly with setting a build env var.

[jmroot (Joshua Root)]

Replying to Gcenx:

Since the port still builds even with the issue I’m thinking of just removing the related section from configure.sh

Edit;
Remade the patch and just removed that section since macports built libvpx even with the incorrect usage of configure.sdkroot

It may have built, but did it build against the right SDK? If it uses the -isysroot flag that MacPorts puts in CFLAGS then it is indeed fine. If not it needs to be fixed to do that.

[Gcenx]

Replying to kencu:

But we have systems using libvpx that go back to 10.5. It is going to need more testing that just on 10.14.

The pull-request had passed all tests, while I'm compiling on 10.14 I'm using Xcode11 with the 10.13 SDK without installing the additional headers package. Even with the following found within the builders logs, it still built

./build/make/configure.sh: line 857: configure.sdkroot: command not found

I can personally test down to 10.8 maybe even 10.6.8, I don't have access to anything running 10.5

Replying to kencu:

You might set a variable in the Portfile to the value of ${configure.skdroot} and pass it in to the shell script on the build line.

I've seen Jeremy do that many times, eg <​https://github.com/macports/macports-ports/blob/master/lang/libcxx/Portfile#L192>

or similarly with setting a build env var.

Removed yeah bad idea.

Replying to jmroot:

It may have built, but did it build against the right SDK? If it uses the -isysroot flag that MacPorts puts in CFLAGS then it is indeed fine. If not it needs to be fixed to do that.

I'm using Xcode11 and Mojave . I haven't install the additional headers package onto my system so if -isysroot wasn't being passed the build would have definitely failed on my system. Within macports.conf I had added

macosx_deployment_target     10.13
macosx_sdk_version           10.13

So I could build as universal, removing the above libvpx still builts of course not as universal but it still built.

[kencu (Ken)]

didn't your first version of this use $(SDKROOT) in the shell script?

That should be set up right. Let me test that.

Please don't replicate base in the Portfile; we're trying hard to let base handle all the flag-setting.

[Gcenx]

Replying to kencu:

didn't your first version of this use $(SDKROOT) in the shell script?

That should be set up right. Let me test that.

Please don't replicate base in the Portfile; we're trying hard to let base handle all the flag-setting.

Yes i believe it did but I’m currently at work so away from my Mac, but I did build with the above patch that removed the entire section leaving base to handle everything for both a universal build then a stock 10.15 compile both finished without issue.

As the flags should be handled by base wouldn’t my above patch of removing -isysroot be better?

As going off both yourself and @jmroot are saying macports base should already be handling -isysroot injection, that would explain the error in the log while it still managed to compile through each test on Travis and on our systems

[Gcenx]

Replying to kencu:

didn't your first version of this use $(SDKROOT) in the shell script?

That should be set up right. Let me test that.

I got home and tested that same issue

:info:configure ./build/make/configure.sh: line 857: SDKROOT: command not found

as you pointed out the script won't know what $(SDKROOT) is since it's a shell script, unless its injected as an env variable.
Yet libvpx still builds (stock macports), the only time I see -isysroot being visible injected is when forcing an SDK version. (to enable +universal)

So the only way to be on the safe side is adding a new env variable that's passed onto configure.sh to ensure -isysroot flag is passed.

I would like some feedback on what's best since it compiles with the newer patch I provided above also with the older patch that give the error but still builds.

[dbevans (David B. Evans)]

Change the patch to replace the offending command with a unique string that can be replaced with the value of configure.sdkroot using reinplace in post-patch? I think that should do it.

[ryandesign (Ryan Carsten Schmidt)]

Replying to Gcenx:

I got home and tested that same issue

:info:configure ./build/make/configure.sh: line 857: SDKROOT: command not found

as you pointed out the script won't know what $(SDKROOT) is since it's a shell script, unless its injected as an env variable.

In shell syntax, $(SDKROOT) is not how you would refer to an environment variable named SDKROOT*; that's how you would refer to a shell command or function named SDKROOT, and there isn't one. If you had wanted to refer to an environment variable named SDKROOT, you would need to use ${SDKROOT} or $SDKROOT—but MacPorts doesn't set an environment variable called SDKROOT either. You could modify the port to do so. However:

Yet libvpx still builds (stock macports), the only time I see -isysroot being visible injected is when forcing an SDK version. (to enable +universal)

When MacPorts wants a port to use an SDK, it already supplies the needed -isysroot/-syslibroot flags to do so in CFLAGS/CXXFLAGS/LDFLAGS. If the build system is already picking up those values, there's no need for the build system to manually add more SDK flags, so I would just remove any code it has that does that. (And if the build system is not honoring the CFLAGS/CXXFLAGS/LDFLAGS MacPorts supplies, then that's what needs to be fixed.)

*$(SDKROOT) is one of the two valid ways to refer to a Makefile variable called SDKROOT (the other valid way being ${SDKROOT}), but the configure.sh script is not a Makefile.

[Gcenx]

Replying to ryandesign:

In shell syntax, $(SDKROOT) is not how you would refer to an environment variable named SDKROOT*; that's how you would refer to a shell command or function named SDKROOT, and there isn't one. If you had wanted to refer to an environment variable named SDKROOT, you would need to use ${SDKROOT} or $SDKROOT—but MacPorts doesn't set an environment variable called SDKROOT either. You could modify the port to do so. However:

I didn't even notice I hadn't replaced ( with { and ) with } dumb mistake on my part.....
I had added an environment variable within my offline Portfile the issue was the above.

When MacPorts wants a port to use an SDK, it already supplies the needed -isysroot/-syslibroot flags to do so in CFLAGS/CXXFLAGS/LDFLAGS. If the build system is already picking up those values, there's no need for the build system to manually add more SDK flags, so I would just remove any code it has that does that. (And if the build system is not honoring the CFLAGS/CXXFLAGS/LDFLAGS MacPorts supplies, then that's what needs to be fixed.)

*$(SDKROOT) is one of the two valid ways to refer to a Makefile variable called SDKROOT (the other valid way being ${SDKROOT}), but the configure.sh script is not a Makefile.

Thank you for the explanation, I'm still rather new to how macports handles things.
Since libvpx builds regardless then the patch in comment:1 that would be the correct way then.

[kencu (Ken)]

Replying to ryandesign:

but MacPorts doesn't set an environment variable called SDKROOT either.

It does since [dabf0a7a6043bd3b356f9b99df399159153d8c61/macports-base]

[kencu (Ken)]

Replying to Gcenx:

I got home and tested that same issue

:info:configure ./build/make/configure.sh: line 857: SDKROOT: command not found

as you pointed out the script won't know what $(SDKROOT) is since it's a shell script, unless its injected as an env variable.

One of the tricky things about MacPorts is you often can't just run stuff like that. It has to be run within the port command to pick up all the settings that base is doing in the background.

Edit: excuse me, I didn't notice that you must have run that within the port command for it to show the :info:configure part. Sorry I missed that first look.

adding a new env variable that's passed onto configure.sh to ensure -isysroot flag is passed.

There already is such an env var. SDKROOT .

[kencu (Ken)]

Well, that is _not_ working as easily as it should !

${configure.sdkroot} is apparently not being set by base on this 10.13 system to any value, and neither is $SDKROOT, and I was hoping base would always set it to something, including "/" as a last resort.

Presumably because it is not being set, $SDKROOT is in fact empty, the -isysroot in the configure.sh script is not set.

DEBUG: Environment: 
CC='/usr/bin/clang'
CC_PRINT_OPTIONS='YES'
CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_opt_macports-ports_multimedia_libvpx/libvpx/work/.CC_PRINT_OPTIONS'
CFLAGS='-pipe -Os -arch x86_64'
CPATH='/opt/local/include'
CPPFLAGS='-I/opt/local/include'
CXX='/usr/bin/clang++'
CXXFLAGS='-pipe -Os -stdlib=libc++ -arch x86_64'
DEVELOPER_DIR='/Library/Developer/CommandLineTools'
F90FLAGS='-pipe -Os -m64'
FCFLAGS='-pipe -Os -m64'
FFLAGS='-pipe -Os -m64'
INSTALL='/usr/bin/install -c'
LD='/usr/bin/clang'
LDFLAGS='-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64'
LIBRARY_PATH='/opt/local/lib'
MACOSX_DEPLOYMENT_TARGET='10.13'
OBJC='/usr/bin/clang'
OBJCFLAGS='-pipe -Os -arch x86_64'
OBJCXX='/usr/bin/clang++'
OBJCXXFLAGS='-pipe -Os -stdlib=libc++ -arch x86_64'

[kencu (Ken)]

Perhaps it's because these command line tools on 10.13 don't have a MacOSX10.13.sdk. I was hoping that we might count on a version matching SDK there going forward, but perhaps not.

$ ls /Library/Developer/CommandLineTools/SDKs
MacOSX.sdk	MacOSX10.14.sdk

So there is no match, but I thought it was to fall back and return MacOSX.sdk in that situation...

Yugh. I can't get this working. I wonder if I"m getting caught up in the cached value or something. Enough for tonight.

Until we can get the SDKROOT to be properly picked up during portconfigure, nothing in the Portfile env vars can be expected to work...

[ryandesign (Ryan Carsten Schmidt)]

Replying to ryandesign:

./build/make/configure.sh: line 857: configure.sdkroot: command not found

This is still happening.

[kencu (Ken)]

there is a pr in the queue <​https://github.com/macports/macports-ports/pull/5555> that is about a year sitting there meant to fix this. I haven't recently looked at it, tho.

[Gcenx]

Replying to kencu:

there is a pr in the queue <​https://github.com/macports/macports-ports/pull/5555> that is about a year sitting there meant to fix this. I haven't recently looked at it, tho.

I’ll force push an updated version to that later today, I’d forgotten about that one

[ryandesign (Ryan Carsten Schmidt)]

Replying to ryandesign:

The patch makes this change to configure.sh

-      osx_sdk_dir="$(show_darwin_sdk_path macosx)"
+      osx_sdk_dir="$(configure.sdkroot)"

This makes no sense:

Over a year later, this mistake is still in the port.

[Gcenx]

In 2eb3c38d139e0fc9fd565e17f4df2386bcb6766f/macports-ports (master):

libvpx-devel: respect configure.sdkroot

Fixes: #59360
Fixes: #62403

[Christopher Nielsen <mascguy@…>]

In 5c4e0c63e132073381e6ef111b623333882a4474/macports-ports (master):

libvpx: respect configure.sdkroot

Fixes: #59360
Fixes: #62403