binaryen has started to require CMAKE_OSX_DEPLOYMENT_TARGET of 10.14

[sbc100]

When we started using std::variant we inadvertently started requiring a 10.14 macOS deployment target.

This went unnoticed on the emscripten-releases builder because there we use out own version of libc++.

However for other users, such as those who want to build from source using emsdk, this is a new requirement and emsdk
currently pins CMAKE_OSX_DEPLOYMENT_TARGET to 10.11:

https://github.com/emscripten-core/emsdk/blob/2b3e0b91761ed8962a7c4eb84d8a288819e106d7/emsdk.py#L1061-L1063

I guess we either need to remove the requirements of binaryen or relax the requirements of emsdk.

[sbc100]

One option is to temporarily revert the use of std::variant in binaryen? @tlively would that be reasonable?

[dschuff]

However for other users, such as those who want to install using emsdk, this is a new requirement and emsdk
currently pins CMAKE_OSX_DEPLOYMENT_TARGET to 10.11:

I think this should be "those who want to build using emsdk". Those who just install are using the Chromium builds and are unaffected.

[sbc100]

However for other users, such as those who want to install using emsdk, this is a new requirement and emsdk
currently pins CMAKE_OSX_DEPLOYMENT_TARGET to 10.11:

I think this should be "those who want to build using emsdk". Those who just install are using the Chromium builds and are unaffected.

Thanks, clarified.

[kripken]

One option is to temporarily revert the use of std::variant in binaryen?

A variation (edit: no pun intended) on that could be to replace the current std::variant with a userspace replacement with similar syntax.

[dschuff]

We also might be able to work around it: https://stackoverflow.com/questions/52521388/stdvariantget-does-not-compile-with-apple-llvm-10-0

[sbc100]

Nice! That looks like it could work.

[tlively]

Yikes! It looks like std::visit is also affected: https://stackoverflow.com/questions/52310835/xcode-10-call-to-unavailable-function-stdvisit/53868971.

Using std::get_if exclusively should work ok technically, but do we have a way to automatically enforce that?

[sbc100]

This should enforce it: #4300

[sbc100]

This should enforce it: #4300

Once we set it to something lower..

[sbc100]

If we land #4300 then part of the fix for this issue will be to lower CMAKE_OSX_DEPLOYMENT_TARGET back to 10.11 (which is what emsdk currently needs/wants).

[juj]

Currently Unity 2021.2 requires macOS 10.13 or newer: https://docs.unity3d.com/2021.2/Documentation/Manual/system-requirements.html . (With new Emscripten releases, we are targeting Unity 2021.2 and higher)

Asked the question internally about when Unity would be bumping the minimum requirement up to macOS 10.14, and it looks like the now upcoming LTS version "2021.3" will still have macOS 10.13 support, and LTSes are supported for two years minimum, so that means that we'll be seeing macOS 10.13 based support phase out at the end of 2023 the earliest.

As a sidenote, the current CMAKE_OSX_DEPLOYMENT_TARGET of 10.11 in emsdk was set to that just because that was the lowest observed number that we knew that LLVM would successfully build with. We do not actually care about macOS < 10.13, so we are comfortable with hoisting that 10.11 to 10.13 as the minimum required version if that helps any.

[juj]

However for other users, such as those who want to install using emsdk, this is a new requirement and emsdk
currently pins CMAKE_OSX_DEPLOYMENT_TARGET to 10.11:

I think this should be "those who want to build using emsdk". Those who just install are using the Chromium builds and are unaffected.

I don't think that is how CMAKE_OSX_DEPLOYMENT_TARGET works. CMAKE_OSX_DEPLOYMENT_TARGET chooses the target macOS version that one will use to run the resulting output executable. So if Chromium builds are setting e.g. CMAKE_OSX_DEPLOYMENT_TARGET=10.14 when they build, then that does mean that people on macOS < 10.14 will not be (guaranteed to) be able to run the Chromium built artifacts.

E.g. in LLVM case, I recall if LLVM was built with CMAKE_OSX_DEPLOYMENT_TARGET = 10.14, then on macOS 10.13 it will fail to start clang executable with the symbol futimens not being found in the system dylibs, due to build time CMake lookup taking some alias that of that function differently since it was assuming that Xcode dylibs 10.14 would be available on the target machine.

[sbc100]

However for other users, such as those who want to install using emsdk, this is a new requirement and emsdk
currently pins CMAKE_OSX_DEPLOYMENT_TARGET to 10.11:

I think this should be "those who want to build using emsdk". Those who just install are using the Chromium builds and are unaffected.

I don't think that is how CMAKE_OSX_DEPLOYMENT_TARGET works. CMAKE_OSX_DEPLOYMENT_TARGET chooses the target macOS version that one will use to run the resulting output executable. So if Chromium builds are setting e.g. CMAKE_OSX_DEPLOYMENT_TARGET=10.14 when they build, then that does mean that people on macOS < 10.14 will not be (guaranteed to) be able to run the Chromium built artifacts.

E.g. in LLVM case, I recall if LLVM was built with CMAKE_OSX_DEPLOYMENT_TARGET = 10.14, then on macOS 10.13 it will fail to start clang executable with the symbol futimens not being found in the system dylibs, due to build time CMake lookup taking some alias that of that function differently since it was assuming that Xcode dylibs 10.14 would be available on the target machine.

The emscripten-releases builder that builds the emsdk binaries sets CMAKE_OSX_DEPLOYMENT_TARGET=10.12. It is able to do so because it bundles its own copy of libc++ so it does suffer from the problem discussed in this issue.

Thus the official binaries are not effect by this issue, only those using emsdk (or other build scripts) to build from source.

[sbc100]

@tlively / @kripken would you have time to look at this soon(ish).. would be nice to get this fixed ASAP to avoid breaking unity releases.

[kripken]

@sbc100 Sorry, I'm not sure what you're asking here. I saw various PRs related to this land - is there more work we need to do after those, and if so, what is it?

[sbc100]

We need to reduce this back to 10.11 (or 10.13 I guess). To do this we will probably need to implement the workaround suggest by @dschuff in #4299 (comment) or use std::get_if as suggest by @tlively in #4299 (comment).

Setting CMAKE_OSX_DEPLOYMENT_TARGET=10.13 in CMakeLists.txt should be enough to cause the build to fail so we easily verify that that fix actually does work.

[dschuff]

The emscripten-releases builder that builds the emsdk binaries sets CMAKE_OSX_DEPLOYMENT_TARGET=10.12. It is able to do so because it bundles its own copy of libc++ so it does suffer from the problem discussed in this issue.

Thus the official binaries are not effect by this issue, only those using emsdk (or other build scripts) to build from source.

I just wanted to reiterate this. It's not actually that hard to build your own libc++ and link it into your own program such as Binaryen, especially if you do it statically.
Doing this is a little extra work setting up the build scripts, but it makes everything else optimal. You can target really old MacOS versions (we do 10.12), and you can use any C++ library features you want, without worrying about what library bugs might be present on those old OS versions. The reason we set this up for emscripten-releases is that we thought those benefits outweighed the costs. That was before this particular issue surfaced, but I'm actually not quite convinced that it changes the calculation. If we roll back to 10.13 we'll be limited to some subset of the standard library (we don't actually know yet how limited it will be, there could be other things missing besides get and visit).
I think this might still be a good tradeoff. @juj do you just use the emsdk scripts straight off the shelf to build? I'd be willing to help with making those work if it means we can keep using all of C++17.

[juj]

@juj do you just use the emsdk scripts straight off the shelf to build?

Yes, we basically run emsdk install sdk-upstream-master-64bit and package up the artifacts that result. If a custom libc++ resolves the issue, then that sounds an ok workaround, though then it would be great to be integrated to emsdk install step out of the box, without needing a custom side by side configuration set up?

[dschuff]

I don't suppose just using our binaries directly would be an option for you?

[sbc100]

I think we need to keep the build-from-source option around, not least to support folks who can't use our pre-built binaries (e.g. non-glibc-based distros, or architectures we don't pre-build for).

[juj]

Maybe the way forward here would be to bundle the same libstdc++ into emsdk that is required by binaryen?

Currently it looks like emsdk install sdk-upstream-master-64bit is broken in emsdk even for up to date macOS, because of the conflicting MACOSX_DEPLOYMENT_TARGET directives.

[dschuff]

That would work, yeah. In theory I think it ought to be sufficient to just statically link libc++ into Binaryen (since the problem comes from the fact that the libc++ on the old systems is missing stuff). But the error happens at compile time and can't take that into account. I wonder if there's a way to work around that, that would be easier than working from a local build or install of libc++ with its own headers...

[juj]

This is no longer a relevant issue to Unity, as Unity 2022.2 will target macOS 10.14 as minimum.

Though next time that minimum macOS is due to get bumped, I hope that it won't happen as "accidentally" as this bump from 10.11 to 10.14 occurred, but instead it will be caught on the CIs. It would be good to have the waterfall builds be the same as the end user builds, so that when the waterfall builds, it will produce the same output as end users do.

[dschuff]

Thanks for the update, and sorry I haven't gotten to this. I don't think it's possible to run the bots on such an old version of MacOS, but I agree that we shouldn't update deployment targets without agreement (and one thing that went wrong here was that I didn't realize the emsdk build couldn't match the waterfall build).

[sbc100]

Now that we don't need to support 10.11.. can we revert back to using system libc++ (like the emsdk does). Seems like it would make the waterfall build simpler and easier to replicate if we didn't need to build libc++ first!

[sbc100]

i.e. not building our own libc++ would actually bring the two builders back into parity which would address @juj's concern (the emscripten-releases waterfall would not be able to feature creep).

[dschuff]

We could. Right now emscripten-releases actually targets 10.12, which goes back to the first time the build was checked in, so I don't really know exactly why we had that version in particular. If we want to require 10.14, according to https://gs.statcounter.com/macos-version-market-share/desktop/worldwide that would cover just about 90% of all installs. So if you figure developers are a bit more likely to be up-to-date than the average user, that probably gets about everyone?

[sbc100]

Lets go for 10.14 to stay in sync with unity and emsdk and binaryen itself (when building without custom libc++)

[juj]

I don't think it's possible to run the bots on such an old version of MacOS

There is no need to install an old macOS to target old OS versions. That is what CMAKE_OSX_DEPLOYMENT_TARGET is for. For example, we also use modern macOS 11 or 12 to build, but pass CMAKE_OSX_DEPLOYMENT_TARGET=10.14 to require that the build is targeting that version as minimum.

i.e. not building our own libc++ would actually bring the two builders back into parity

That would be great!

[dschuff]

Sorry, I meant I don't know of a good way to test on the bots (i.e. make a test that fails when we build with the wrong deployment target).
But since we already explicitly specify the deployment target in the build script, and as of yesterday the Chromium emscripten-releases build is in sync with emsdk (and there's a comment to remind us that we should keep them in sync), we are at least in the position where a change to that would be caught by review. So I think we are in pretty good shape now.