Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making minimum Mac OS X version configurable. #22

Closed
wants to merge 6 commits into from
Closed

Making minimum Mac OS X version configurable. #22

wants to merge 6 commits into from

Conversation

nettoyeurny
Copy link

This is necessary for building externals that use C++11.

@katjav
Copy link
Contributor

katjav commented Sep 30, 2016

10.4 is the latest OSX for powerpc, therefore if ppc is in a (multi-arch) build, -mmacosx-version-min=10.4 should be passed to make it work. Note that this ppc is only in the default mix when the build is done on an old i386 machine.

But for i386 binaries it is different, they can still run on recent OSX, right? I was considering to get rid of Makefile.pdlibbuilder's default multi-arch builds (see issue #16 and #21), and not pass any arch.c.flags at all. But that would probably mean that minimum osx version is set to whatever the build machine has installed, and users with older OSX versions could run into trouble with binaries built that way. So this must be reconsidered.

What makes sense? I'm now thinking:

  • If ppc is in the build, pass -mmacosx-version-min=10.4 non-overridable.
  • If i386 is in the build (but not ppc), pass -mmacosx-version-min=10.5 by default but overridable.
  • If only x86_64 is in the build, pass -mmacosx-version-min=10.6 by default but overridable.

@nettoyeurny
Copy link
Author

Thanks, Katja! I've been thinking about your comment, and I decided that I wouldn't feel comfortable making such a sweeping change because I don't understand all the implications.

I have, however, made a revision that isn't quite what you suggest but it's similar in spirit, I think. The basic idea is this: (a) Given the minimum OS version, build the fattest binary possible, and (b) if the minimum OS version is unspecified, retain the behavior of the current master branch.

What do you think?

@katjav
Copy link
Contributor

katjav commented Oct 7, 2016

I'm thinking of a different route to support C++11. A lib makefile using C++11 should define 'cflags = -std=c++11', right? Otherwise it would only work with compiler versions that have C++11 or higher as the default. Makefile.pdlibbuilder can check for this and set the minimum OSX version accordingly. Much easier than introducing yet another 'API' variable. Who knows what other settings may be needed for C++11. Did you try on Windows and Linux?

katjav added a commit that referenced this pull request Oct 9, 2016
Makefile.pdlibbuilder sets a default minimum OSX version for optimal
support of older OSX versions. With this commit, '-mmacosx-version-min=*'
in variable 'cflags' from the lib makefile is respected, not overridden.

A lib makefile can now determine the minimum OSX version (for example to
support C++11, which was the direct motivation for this change).

This is a response to pull request #22 'Making minimum Mac OS X version
configurable', but note that it is implemented in a different way.
@katjav
Copy link
Contributor

katjav commented Oct 9, 2016

Yet another solution, see commit a7d3098 in branch 'configurable-minimum-osxversion'. It does not introduce a new API variable, but respects definition in 'cflags' from the lib makefile for the same purpose. If I did it right, you should be able to define the flag like so:

define forDarwin
cflags = -mmacosx-version-min=10.9
endef

Makefile.pdlibbuilder doesn't do a validity check in the code as here proposed, as I realized that it would clutter the makefile too much. While we know that 10.4 is available for all OSX i386 machines and 10.5 for all x86_64, -mmacosx-version-min=10.4 is valid for i386 and 10.5 for x86_64 builds, higher versions may or may not be available depending not only on architecture but also on native operating system. If not valid, the compiler will probably warn about it soon enough.

Can you test if it works for you?

@katjav
Copy link
Contributor

katjav commented Oct 9, 2016

Extra advantage of the above mention approach is, cflags aren't overriddable by CFLAGS, unlike the default minimum OSX version as defined by Makefile.pdlibbuilder.

@nettoyeurny
Copy link
Author

That works --- thanks, Katja! This is indeed much better. I'll close this pull request.

katjav added a commit that referenced this pull request Oct 27, 2016
This merges commit a7d3098 which responded to issue #22. With this commit,
cflag '-mmacosx-version-min=*' from a lib makefile is respected, not
overridden by a default minimum version.
@katjav
Copy link
Contributor

katjav commented Oct 27, 2016

Topic branch 'configurable-minimum-osxversion is now merged in master, I'm going to delete the topic branch.

@nettoyeurny
Copy link
Author

Thanks, Katja! I updated the pd-lib-builder submodule in pd-for-ios and it's working nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants