pololu / nixcrpkgs

Tools for cross-compiling standalone applications using Nix.
Other
200 stars 17 forks source link

macos/sdk_builder: use plist, not json file #4

Closed euank closed 2 years ago

euank commented 4 years ago

Not all macos SDKs seem to bundle SDKSettings.json. Some only seem to have SDKSettings.plist.

Parsing the plist file without ruby is pretty easy, so it seems straightforward to switch over to that.

In addition, I removed the ls check at the end because it didn't seem to be correct for the SDK I'm using.

Specifically, with my 10.10 sdk, the path seemed to be '/usr/include/c++/v1/iterator' (note the v1).

However, since everything still worked without the check as best I can tell, it seems reasonable to just drop it entirely.

DavidEGrayson commented 4 years ago

Maybe you could just get a newer version of the SDK which has the /usr/include/c++/iterator and SDKSettings.json? I'm using 10.14 right now, but I have kept versions of 10.11 and 10.12 just in case. I think you can pass -mmacosx-version-min=10.10 to Clang to make sure your code will work on older operating systems.

I looked back at commit 024800d879eb0b4e3071e0612370b4cac9e21d68 to remind myself why I'm checking for /usr/include/c++/iterator: the wrapper I make for clang has to know where to find the C++ headers, and at some point, I started using a newer version of the macOS SDK where those headers moved from /usr/include/c++/v1 to /usr/include/c++.

DavidEGrayson commented 4 years ago

Oh! I think I might know why some versions of the SDK have a v1 folder, and it's not Apple's fault:

https://github.com/pololu/nixcrpkgs/blob/a9bd392c9b855f6def6b1e52e32d3b4562c35114/macos/gen_sdk_package.sh#L141

The cp command is actually very tricky to use right, because the meaning of it depends on whether the destination folder already exists or not. If the c++ directory happens to exist already, then v1 gets copied inside it. If the c++ directory does not exist, then v1 is renamed to c++ as it is being copied. Do you want to take a crack at fixing that script? What if we added rm -rf $TMP at the top of the loop instead of only having it at the bottom?

euank commented 4 years ago

Aah, I get what happened now. I actually am coming from using osxcross, and I reused the sdk package we were using for that previously. I assumed the format would be the same since I noticed the gen_sdk script came from osxcross.

However, it looks like the upstream osxcross project changed the gen_sdk_package.sh script to always include the v1 folder by adding an explicit mkdir right before copying here.

So people that build an sdk tarball with osx-cross can't reuse the sdk tarball they had previously here, unless we remove the check.

It looks like we get to make two choices:

  1. Do we want to diverge from the osx-cross format (exclude the v1 directory), or match the upstream change (update gen_sdk script)
  2. And do we want a check at all for the sdk being used having the right folder structure (either re-add, or update, it depending on the answer to 1)
DavidEGrayson commented 4 years ago

Let's try to use the osxcross format, assuming it doesn't change too rapidly. I'm hoping the format will stay pretty stable in the upcoming years now that they fixed that bug. So I think we should at least add a mkdir command to our copy of gen_sdk_package.sh, like the one you found in their copy. I'm not sure what other changes were made to the script, or how hard it will be for me to review them.

I think sdk_builder.sh should start accepting both types of SDK. It should check if the v1 folder is there, and if it is not, it would rename c++ to v1, create c++, and move v1 into c++. This is just a few lines of shell script so it seems easier than making me (and potentially other nixcrpkgs users) manually fix the format of their SDKs. There would be no need for a separate ls check after doing that. I guess you don't care much this part, so I could just do it myself if your PR doesn't include it. (Sometimes breaking changes are good in this project, but in this case I'd rather avoid it.)

macos/wrapper.cpp would need to be changed back to the way it was before, using .../c++/v1.

DavidEGrayson commented 2 years ago

Hi, I just updated nixcrpkgs to use the latest gen_sdk_package.sh script from the osxcross project and I updated my sdk_builder.sh script. I tested that macos.hello_cpp compiles successfully both using my old MacOS10.14.sdk.tar.xz and a MacOS12.3.sdk.tar.bz2 which I just generated today.

Both of these SDKs have SDKSettings.json, so I am hoping that from now on the SDK will generally always have this file and we don't really need to handle cases where it is missing. But if people continue to have trouble due to this file being missing, I'd definitely like to know about it.

euank commented 2 years ago

Yup, thanks for closing this out!

Sorry for not pushing this forward myself, I don't currently own any mac hardware, which made it harder to find time to come back to it :sweat_smile: