search

faithfracture / Apple-Boost-BuildScript

Script for building Boost for Apple platforms (iOS, iOS Simulator, tvOS, tvOS Simulator, OS X)
279 stars 111 forks source link

Fix shellcheck warnings and errors #47

Closed guillaumealgis closed 4 years ago

guillaumealgis commented 4 years ago

This :

The problems I saw the most were :

The diff is quite long but should be relatively straightforward to review. Let me know if anything isn't clear :)

faithfracture commented 4 years ago

This looks OK to me, but could you also submit your PR for #45 so I can actually build & test everything?

guillaumealgis commented 4 years ago

I'm a bit reluctant to open another PR before this one is merged because there's a lot of changes here so I'd be building on sand in another PR.

But you can build the project with juste a few patches :

First you'll have to fix MACOS_SDK_VERSION because Xcode 11.4 now includes the patch number in its output :

< MACOS_SDK_VERSION=$(xcrun --sdk macosx --show-sdk-version)
> MACOS_SDK_VERSION=$(xcrun --sdk macosx --show-sdk-version | awk -F. '{ print $1 "." $2 }')

Then download and apply the patch linked by @mjpan in https://github.com/faithfracture/Apple-Boost-BuildScript/issues/45 :

curl 'https://raw.githubusercontent.com/apache/kudu/e6fa761a4a60118bb61883fe8342943253f350d2/thirdparty/patches/boost-issue-440-darwin-version.patch' | patch -p0 -d src/boost_1_72_0

And it should build :)

faithfracture commented 4 years ago

Ok great, that's working fine, thanks!

Please update the changelog and I'll merge this. :)

FYI, when you do submit a PR for that patch, I don't want it to be pulled from that URL - I'm going to want it to be a local patch file added to the repository so this won't break if that repository goes away some day. I'd also like it to reference the actual Boost.Build PR that fixes this issue (https://github.com/boostorg/build/pull/560 -> https://github.com/boostorg/build/pull/560.patch). You'll have to apply it a little differently: patch -p1 -d src/boost_{version}/tools/build < boost_pr_560_macos_compiler_version_check.patch. I've already done this on my end, so if you'd rather I just commit that change I can.

guillaumealgis commented 4 years ago

Please update the changelog and I'll merge this. :)

Sure, done.

FYI, when you do submit a PR for that patch, I don't want it to be pulled from that URL - I'm going to want it to be a local patch file added to the repository so this won't break if that repository goes away some day.

Yup, it's what I had in mind too, the curl thing was just for a quick fix.

I've already done this on my end, so if you'd rather I just commit that change I can.

Well if you've already done it, sure, there's no benefits to do the work twice :)