Skip to content

macOS d_fat arch detection using Xcode version#69

Closed
danomatika wants to merge 12 commits intomasterfrom
feature/d_fat-arch-detection
Closed

macOS d_fat arch detection using Xcode version#69
danomatika wants to merge 12 commits intomasterfrom
feature/d_fat-arch-detection

Conversation

@danomatika
Copy link
Contributor

@danomatika danomatika commented May 10, 2021

This PR adds auto detection of the available target architectures when performing a "fat lib" build on macOS. An override is also provided if the arch variable is set when building.

Note: I notice that if I try to set the extension variable to trigger the fat lib build, it will only work from the command line and not from within a Makefile. I'd like to be able to do the following:

# build macOS fat lib
define forDarwin
  extension ?= d_fat
endef

EDIT: The Xcode version detection comes from Caffee.

Also, I'm pretty sure pkgutil is available all the way back to OSX 10.6 as the man page is dated March 2011 and 10.7 was release in summer of 2011. Can someone try on a 10.6 machine to make sure?

@danomatika
Copy link
Contributor Author

danomatika commented May 10, 2021

Oh and I can confirm x86_64 + arm64 builds fine on macOS 11 with Xcode 12.

@katjav
Copy link
Contributor

katjav commented May 10, 2021

Note: I notice that if I try to set the extension variable to trigger the fat lib build, it will only work from the command line and not from within a Makefile.

This is because the variable defined in the makefile is overridden by the last definition (the default platform extension), while a variable definition from command line has precedence over the makefile.

One solution would be to define default extension conditionally in Makefile.pdlibbuilder (i.e. on the condition that it wasn't defined yet).

@danomatika
Copy link
Contributor Author

One solution would be to define default extension conditionally in Makefile.pdlibbuilder (i.e. on the condition that it wasn't defined yet).

Done: extension ?= pd_darwin

@katjav
Copy link
Contributor

katjav commented May 10, 2021

extension ?= pd_darwin

A (possibly welcome) side effect is that extension can then be defined as environment variable. Maybe it should be done like this for all platforms, but I've been hesitant about that for a long time because essentially it means exposing them as explicit API variables. And the more API variables, the more headache for developer to solve later puzzles.

@umlaeute
Copy link
Contributor

a gut feeling tells me to agree with katja that we should avoid envirnment variables whenever possible. (esp. if they have such a generic name as extension).
currently this isn't backed up by any evidence though.

@danomatika
Copy link
Contributor Author

danomatika commented May 11, 2021

I agree that it would be better not to expose too many internal variables, when possible. I started with using what was already there, namely setting extension.

A better approach would be a simple boolean like suppress-wunused. Perhaps something like a fat-libs switch?

ifeq ($(system), Darwin)
  ...
  extension = pd_darwin
  ifdef fat-libs
    extension = d_fat
  endif
  ifeq ($(extension), d_fat)
    # set arches from Xcode version
    ...
  endif
  ...
endif

The making a d_fat build would be as simple as:

# build macOS fat lib
define forDarwin
  fat-libs
endef

or even just the switch as it would only be used for build systems which support it:

fat-libs

Perhaps a better name could be build-multi-arch to build a "fat" multi-arch build? OTOH "fat" is already used in the extension naming.

@danomatika
Copy link
Contributor Author

Ok, now the d_fat build is triggered with make-lib-fat=yes which works similarly to make-lib-executable. I also moved the usage info into the tips & tricks md and added a section documenting the new variable in the makefile itself.

@umlaeute
Copy link
Contributor

hmm, this seems to be a regression to me.

the current behaviour is to build fat binaries if extension=d_fat. simple as that.

why do we now need a separate switch?
what's wrong with katjas suggestion of explicitely testing whether extension is empty instead of using ?=?

@danomatika
Copy link
Contributor Author

hmm, this seems to be a regression to me.

Hrmm, well Katja wrote:

but I've been hesitant about that for a long time because essentially it means exposing them as explicit API variables. And the more API variables, the more headache for developer to solve later puzzles.

and you wrote:

a gut feeling tells me to agree with katja that we should avoid envirnment variables whenever possible. (esp. if they have such a generic name as extension).

So I read that as "we don't want to use extension for this" and as there is already a similar convention with make-lib-executable, I'm not sure I see this as a regression. It would seem more dangerous to me to require people to know what the Pd-specific output extension has to be just to basically say "I want a multi-arch build."

@danomatika
Copy link
Contributor Author

danomatika commented May 11, 2021

Making extension overridable could be added via:

ifeq ($(origin extension), undefined)
  extension = pd_darwin
  ifeq ($(make-lib-fat),yes)
    extension = d_fat
  endif
endif

Then make extension=d_fat would still work...

@umlaeute
Copy link
Contributor

So I read that as "we don't want to use extension for this"

ah, i read this: "we don't want to use environment-variables for this".

extension (as a make-variable) is fine for me.

@danomatika
Copy link
Contributor Author

Regarding the supported architecture detection, we can also use the macOS version such as used in the libpd makefile: https://github.com/libpd/libpd/blob/master/Makefile#L16

@umlaeute
Copy link
Contributor

using the macOS version doesn't strike me as particularly correct.

ideally we should ask the compiler what it supports (but that gets quickly down the road towards autotools/cmake/...)

@danomatika
Copy link
Contributor Author

What do you think @katjav? Should I return it to extension=d_fat or keep make-lib-fat=yes? In the end, I'll do whatever as long as the project builds.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

What do you think @katjav? Should I return it to extension=d_fat or keep make-lib-fat=yes?

Introducing a proper switch for building fat binaries seems a good thing to do if it doesn't break current behavior, but notice that make-lib-executable builds a multi-class lib instead of default single-class binaries so this is not a variable name that you want to associate with. Instead, multi-arch=yes would be sufficiently specific. That gives us the option to define other types of fatness later (multi-precision, who knows).

(By the way I doubt whether a lib makefile should build non-native architectures without explicit user interference.)

@danomatika
Copy link
Contributor Author

Would make-multi-arch suffice? My thinking was along the lines of naming it an action starting with a verb, similar to "suppress-wunused" and "make-lib-executable".

@danomatika
Copy link
Contributor Author

danomatika commented May 12, 2021

(By the way I doubt whether a lib makefile should build non-native architectures without explicit user interference.)

Actually, this is less of a problem for my use case as I have another makefile calling the individual external makefiles, so I can set the flag/variable there.

EDIT: One issue is that if you forget to pass the flag/variable, running the clean target will clean the wrong extension, however that's less of an issue as long as it's communicated.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

Would make-multi-arch suffice? My thinking was along the lines of naming it an action starting with a verb, similar to "suppress-wunused" and "make-lib-executable".

That is consistent indeed, excellent.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

I have another makefile calling the individual external makefiles, so I can set the flag/variable there

Calling submake process is a command line so you can override makefile definitions after all.

@danomatika
Copy link
Contributor Author

Ok, switch renamed to "make-multi-arch" and I removed the tips & tricks reference to overriding the internal arch variable.

As to IOhannes' point earlier:

the current behaviour is to build fat binaries if extension=d_fat. simple as that.

This still works.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

One issue is that if you forget to pass the flag/variable, running the clean target will clean the wrong extension, however that's less of an issue as long as it's communicated.

That is a general consequence with non-default build products, also with multi-class lib or alternative extension; you have to specify the same for targets all, install and clean.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

@danomatika do you have access to old setups for testing all detection cases?

We also need to be sure that cross-compilation still works as before.

@danomatika
Copy link
Contributor Author

danomatika commented May 12, 2021

@katjav I am working from home and have 3 laptops available here, but nothing too old:

  • macOS 10.14 Intel x86_64
  • macOS 11.3 Intel x86_65
  • macOS 11.3 Apple arm64

I see also that relying on detecting the CLITools installed will not work with older versions of Xcode in 10.6 and possibly 10.7, so I will also add checking the output of xcodebuild -version as a backup.

UPDATE: This should also handle the case for systems which have Xcode installed from the App Store without installing the separate CLI tools package. For people who only compile on the command line, it's more likely they will only have the CLI Tools installed, for example as we suggest in the Pure Data INSTALL.txt.

@danomatika
Copy link
Contributor Author

danomatika commented May 12, 2021

We also need to be sure that cross-compilation still works as before.

What cross-compilation setups are there for macOS... via Linux?

UPDATE: As far as I can tell looking through the new behavior, everything should still build as before, except that there will be a warning printed if Xcode is not detected and the old default architectures of i386 x86_64 are used.

UDPATE2: I also don't think I'm inadvertently using any newer Make-isms than what is already there, ie. chained "else ifeq" etc.

@katjav
Copy link
Contributor

katjav commented May 12, 2021

What cross-compilation setups are there for macOS... via Linux?

On Linux indeed it is possible to cross-build for MacOs using this project:

https://github.com/tpoechtrager/osxcross

In 2019 I was, for a short while, ambitious about a "one-click-build" method to get Pd externals for "all" platforms (obviously not including arm64 for Mac):

https://github.com/electrickery/pd-dekencross

I could try retrieve my cross-build setup to see how it behaves with your detection method.

@danomatika
Copy link
Contributor Author

danomatika commented May 12, 2021

When this PR is merged, can the commits be squashed? There quite a few back & forth commits in there.

@danomatika
Copy link
Contributor Author

This PR is working for me with our (customized) externals in https://github.com/zkmkarlsruhe/ZirkoniumSpatializationServer/tree/update/external-sources. Let me know if there are other changes needed.

@danomatika
Copy link
Contributor Author

danomatika commented Jun 1, 2021

As related to the discussion at #51, does anyone think the solution in this PR is getting too complicated? Should I/we think of a simpler approach which relies more on library developers, ie. no simple flag but specify the architectures manual via something like

ifeq ($(origin extension), undefined)
    # chooses extension d_fat automatically when set 
    multi-arch = arm64 x86_64
endif

This relies a bit more on documenting the flags, but they are simple enough.

OTOH we use a similar Xcode version detection already in the libpd makefile for a couple of years now and there haven't been any major issues yet. However it could be a (small) maintenance point as Apple shifts their SDKs around, although I think things are stable enough after the move to the bundled Xcode.app and separate Command Line Tools around OSX 10.7.

@umlaeute
Copy link
Contributor

umlaeute commented Jun 1, 2021

i think this is different from #51, as there is typically no problem with building for a given architecture as long as the compiler supports it (unlike the -mmacosx-version-min that might have a negative impact on the usability of the successfully produced binary).

i cannot think of a reason why anybody would actively want to prevent a build for powerpc or arm64.

@danomatika
Copy link
Contributor Author

Does this need any updates?

@danomatika
Copy link
Contributor Author

danomatika commented Nov 19, 2021

I just fixed a bug where detecting the Xcode version using xcodebuild would fail. Now working in our external object builds.

@danomatika
Copy link
Contributor Author

danomatika commented Dec 6, 2022

I'm closing this for now as it appears to be going nowhere.

I would suggest, at a minimum, removing the extension handling which checks for d_fat and sets the outdated arch = i386 x86_64 in favor of checking if there is more than 1 arch set for Darwin and, if so, setting the extension. However, since the current mechanism relied on people explicitly setting the extension, I imagine most pre-compiled fat externals for macOS are actually using the default of pd_darwin. This makes me wonder if d_fat is really needed in the end.

In any case, the current solution I am now using is to explicitly set arch and extension via my main wrapper Makefile which also checks for multiple architectures and sets d_fat automatically:

zkmkarlsruhe/ZirkoniumSpatializationServer@d27671e

@danomatika danomatika closed this Dec 6, 2022
@katjav
Copy link
Contributor

katjav commented Dec 15, 2022

What if we just replace the outdated arch = i386 x86_64 with arch = x86_64 arm64. That would require overriding of the arch definition only if i386 and/or ppc is to be included.

On the other hand I don't know if pdlibbuilder should define architectures or even extension when "fat binary" is ambiguous.

@umlaeute
Copy link
Contributor

On the other hand I don't know if pdlibbuilder should define architectures or even extension when "fat binary" is ambiguous.

i tend to agree.
with a "fast" moving-target like macOS-architectures and a slow-moving persecutor (lie pd-lib-builder) there will always be problems with people not being able to build the right architecture.

this PR was an attempt to lift the targetting to the tools, but these are still moving.

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.

3 participants