Good point. I'm also wondering, what if (for whatever reason) MACOSX_SDK_PATH is empty?
That will cause the -isysroot argument to be invalid. I can either make an undefined MACOSX_SDK_PATH a hard error (in configure) or I can wrap all uses of -isysroot/-iframework with ifneq blocks. Personally, I'd prefer the former, since there should never be a case where it can't find the SDK, if there is then the build environment is not set up correctly. I've not yet hit a case where it's been undefined, so I find it unlikely to be an issue if we make it a hard error. -DrD- > We have several platform specific tools defined that way already (CYGPATH, > MT, RC to name a few), so no worries. The error would be trying to use them > on the wrong platform, but protecting the assignment won't help with that. > > /Erik > > On 2014-05-21 18:33, David DeHaven wrote: >> As long as XCODEBUILD=@XCODEBUILD@ doesn't result in an error on other >> platforms, that was my only concern there. >> >> -DrD- >> >>> Hello David, >>> >>> Mostly looks good. I don't see a reason to add ifeqs on OS around variables >>> in spec.gmk.in. We usually just leave them there for all platforms. The >>> only difference that will make is the variable will be undefined instead of >>> empty. >>> >>> Variable name as suggested by Mike seems fine to me. >>> >>> /Erik >>> >>> On 2014-05-21 06:56, David DeHaven wrote: >>>> I can do both of those. It's MacOSX<version>.sdk, e.g., MacOSX10.9.sdk so >>>> MACOSX_SDK_PATH does make more sense. >>>> >>>> -DrD- >>>> >>>>> [not a build project Reviewer] >>>>> >>>>> The only change I would make is to put a conditional around the other new >>>>> variable in spec.gmk.in. >>>>> >>>>> I wondered if the variable should be MACOS_SDK_PATH since macos.sdk is >>>>> the name used in the path. >>>>> >>>>> Mike >>>>> >>>>> On May 20, 2014, at 9:30 PM, David DeHaven <david.deha...@oracle.com> >>>>> wrote: >>>>> >>>>>> JBS Issue: >>>>>> https://bugs.openjdk.java.net/browse/JDK-8043340 >>>>>> >>>>>> Summary: >>>>>> We currently hard code the path to JavaVM.framework to >>>>>> /System/Library/Framworks. This worked when Apple installed header files >>>>>> in those frameworks when the command line tools were installed. Those >>>>>> headers are no longer installed so we need to change our paths to use >>>>>> the Mac OS X SDK path instead. >>>>>> >>>>>> Most of the changes here are just adding $(MAC_SDK_PATH) to the JavaVM >>>>>> (and one case of ApplicationServices) framework. If MAC_SDK_PATH is >>>>>> undefined, then it will simply revert to the current behavior of >>>>>> building against the system frameworks. If configure cannot determine >>>>>> the path to the macosx SDK, then it will spit out a warning but >>>>>> otherwise proceed as normal unless it cannot find headers inside >>>>>> JavaVM.framework, then it will fail. >>>>>> >>>>>> Changes of note: >>>>>> - Added MAC_SDK_PATH, set to the absolute path to the macosx SDK as >>>>>> determined by "$(TOOLCHAIN_PATH)/xcodebuild -sdk macosx -version" >>>>>> - Changed all -F arguments to -F$(MAC_SDK_PATH) >>>>>> - I had to add -iframework$(MAC_SDK_PATH) to force it to compile and >>>>>> link all frameworks from the same SDK, otherwise great big gobs of >>>>>> deprecation warnings filled the screen. >>>>>> - XCODEBUILD was added as we need to use the copy of the tool at the >>>>>> location specified by --with-tools-dir if provided, otherwise we would >>>>>> end up with the completely wrong path. If it's not found in the tool >>>>>> path then it uses the default /usr/bin/xcodebuild (if that's not there >>>>>> then the system is not configured properly...) >>>>>> - Removed JavaVM.framework from the header search path (and changed >>>>>> MacosxDebuggerLocal.m accordingly) in hotspot, we shouldn't be using >>>>>> those headers >>>>>> - I made not being able to find JavaVM.framework a fatal error in >>>>>> configure, since we can't build without those headers >>>>>> >>>>>> Tested on 10.8 with Xcode 5.1 and Xcode 4.6.3 and on a clean 10.9 system >>>>>> with Xcode 5.1 against jdk9/hs. I will submit a JPRT sanity run shortly. >>>>>> >>>>>> Patches: >>>>>> http://cr.openjdk.java.net/~ddehaven/8043340/v0/top >>>>>> http://cr.openjdk.java.net/~ddehaven/8043340/v0/hotspot >>>>>> http://cr.openjdk.java.net/~ddehaven/8043340/v0/jdk >>>>>> >>>>>> -DrD- >>>>>> >
