Update Makefile.pdlibbuilder for Apple Silicon

[patricecolet]

Apple Silicon homebrew files location is /opt/homebrew ref: https://docs.brew.sh/Installation

[umlaeute]

i think a better approach would be to just add whatever paths are available.
e.g.

cpp.flags :=  -DUNIX -DMACOSX $(patsubst %,-I%,$(wildcard /sw/include /opt/homebrew/include))
c.ldflags := -undefined suppress -flat_namespace -bundle $(patsubst %,-L%,$(wildcard /opt/homebrew/lib))
cxx.ldflags := -undefined suppress -flat_namespace -bundle $(patsubst %,-L%,$(wildcard /opt/homebrew/lib))

also note that according to the GNU make documentation, the -L flags should go into LDFLAGS rather than LDLIBS

[patricecolet]

Thank for those advices, if I want to change this pull request, how do I proceed? I see two options:

1° Closing this Pull Request and create another one
2° Amending commit

[umlaeute]

In your case I would just amend the commit and force push it

[umlaeute]

In your case I would just amend the commit and force push it

do you need help with that?

[patricecolet]

In your case I would just amend the commit and force push it

do you need help with that?

Yes, I tried git commit --amend, but this is not happening good, it seems I have to be a contributor to do it.

[Ant1r]

you have to amend the commit on your local install, then push to your github fork https://github.com/patricecolet on the same branch patch-1.
The current PR will then be automatically updated.

[patricecolet]

Okay thank you, I've finally made the correction in patch-1 branch with my web browser.

[umlaeute]

i've squashed to two commits into a single one and force pushed them to your branch ...

[katjav]

Being unable to test this pr in practice, I do have question about it. The pr introduces homebrew search paths as an update for Apple silicon. Equivalent paths for macOS Intel were never specified in Makefile.pdlibbuilder, neither in the pull request. Probably because pd externals did not need those paths by default.

So my main question for @patricecolet is: how are homebrew paths more important for externals built on Apple silicon than on MacOs Intel?

By the way Pd's configure.ac does specify the "old" homebrew paths:

https://github.com/pure-data/pure-data/blob/3d3d2914870b447eac640761c4d01868aa1d34d5/configure.ac#L85

[residuum]

As far as I can tell from the CI builds of purest_json, /usr/local/(lib|include) are referenced by default.

The explicit paths to curl are necessary, because the CI machine has curl already installed.

https://app.circleci.com/pipelines/github/residuum/PuRestJson/57/workflows/fadb6972-2919-40a0-b24a-528adf9171a3/jobs/320/parallel-runs/0/steps/0-102

cc -undefined suppress -flat_namespace -bundle  "-L/usr/local/opt/curl/lib" -arch x86_64 -arch arm64 -mmacosx-version-min=10.6  -o urlparams.pd_darwin src/urlparams.o  -lc -lcurl -ljson-c -loauth
ld: warning: dylib (/usr/local/opt/curl/lib/libcurl.dylib) was built for newer macOS version (12.0) than being linked (10.6)
ld: warning: dylib (/usr/local/lib/libjson-c.dylib) was built for newer macOS version (12.0) than being linked (10.6)

[patricecolet]

I guess this is well explained here: https://apple.stackexchange.com/questions/437618/why-is-homebrew-installed-in-opt-homebrew-on-apple-silicon-macs

/usr/local is also used by other tools, not just Homebrew. This can lead to potential conflicts.
Installations in /opt/homebrew for Apple Silicon and /usr/local for Rosetta 2 can coexist.

[residuum]

I am unsure about setting the path in the template though, because not everyone will be using Hombrew, others might use MacPorts, or they might want to load custom built dependencies from some other locations.

[umlaeute]

the fun part about this PR is, that it only uses /opt/homebrew/* if that path actually exists, and if the corresponding /sw/* does NOT exist.

As far as I can tell from the CI builds of purest_json, /usr/local/(lib|include)

that's because /usr/local/ are searched by the compiler itself (Makefile.pdlibbuilder does not interfere at all)

[umlaeute]

@katjav wrote:

By the way Pd's configure.ac does specify the "old" homebrew paths:

that doesn't mean exactly much.

• Pd normally doesn't use any external libraries that could be installed by homebrew (so it could check for files on the moon instead of /usr/local/ and builds will still succeed)
• Pd builds do not have the same "we care for you on platform XY" attitude as pd-lib-builder (we do care. but if things break, they only break for a single project (Pd). so we fix it there as problems occur. pd-lib-builder has a much larger audience)

[umlaeute]

Like so often I'm wrong about many things:

1. Pd does (optionally) use one external library even on macOS: libjack. But that is installed manually - presumably into /usr/local - (at least in the CI) rather than via homebrew
2. I think purest_json gets the /usr/local path via pkg-config
3. This commit adds /opt/homebrew even if /sw/* exists - the only condition is that the /opt/homebrew/* path exists

[danomatika]

It worked with Homebrew before, since Homebrew installed into /usr/local. In many ways, it's an improvement that they changed it since other packages and installers also install to /usr/local.

My 2 cents: This is a documentation issue. As /opt/homebrew is non-standard, I err on the side of not including it by default or even checking if it exists, but adding a note to the readme/examples to include additional paths as required. This is standard practice with Makefiles in general. I don't see this being any real additional workload to external developers.

OTOH This doesn't add any major overhead to pd-lib-builder... as opposed to my #69...

[patricecolet]

For building externals on Apple Silicon with homebrew we should use make CFLAGS="-I/opt/homebrew" LDFLAGS="-L/opt/homebrew/lib" instead?

[danomatika]

For building externals on Apple Silicon with homebrew we should use make CFLAGS="-I/opt/homebrew" LDFLAGS="-L/opt/homebrew/lib" instead?

Yes, which you as an external author can put in your Makefile...

# Makefile for foo

lib.name = foo
class.sources = foo.c
datafiles = foo-help.pd README.md LICENSE.txt

# include paths for homebrew
define forDarwin
  cflags += -I/opt/homebrew/include
  ldflgs += -L/opt/homebrew/lib
endef

PDLIBBUILDER_DIR=.
include $(PDLIBBUILDER_DIR)/Makefile.pdlibbuilder

[danomatika]

Regarding pkg-config: definitely use it if the libs you are using support it. Something like the following if linking to "libbar":

# Makefile for foo which links to libbar

lib.name = foo
class.sources = foo.c
datafiles = foo-help.pd README.md LICENSE.txt

# get paths & libs for libbar via pkg-config
define forDarwin
  cflags += $(shell pkg-config --cflags bar)
  ldflgs += $(shell pkg-config --libs bar)
endef

PDLIBBUILDER_DIR=.
include $(PDLIBBUILDER_DIR)/Makefile.pdlibbuilder

For instance, on my system pkg-config reports the following for libsdl2:

% pkg-config --cflags sdl2
-D_THREAD_SAFE -I/opt/homebrew/include -I/opt/homebrew/include/SDL2
% pkg-config --libs sdl2
-L/opt/homebrew/lib -lSDL2

[umlaeute]

@danomatika suggested:

# include paths for homebrew
define forDarwin
  cflags += -I/opt/homebrew/include
  ldflags += -L/opt/homebrew/lib
endef

honestly that seems backward, as the same stanza is required for each and every external that has any dependencies (beyond Pd) that needs to be compiled for a new mac.
this kind of defeats the purpose of pd-lib-builder (as I see it), which is to hide away the system specifics as much as possible, providing a common interface for all systems.

but i think this requires a fundamental discussion about what pd-lib-builder actually is: #75

[umlaeute]

just a nitpick:

define forDarwin
  cflags += $(shell pkg-config --cflags bar)
  ldflgs += $(shell pkg-config --libs bar)
endef

i think this is backward insofar as pkg-config is supposed to be platform independent (and indeed it is).
so it really should read more like (at least if bar is to be used on all platforms):

cflags += $(shell pkg-config --cflags bar)
ldflgs += $(shell pkg-config --libs bar)

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

[umlaeute]

This is a documentation issue. As /opt/homebrew is non-standard

non-standard according to which standards?

It's obviously the standard for "standard" homebrew installations on macOS/arm64.
And homebrew is de facto the "standard" package manager of macOS (even if it is not sanctioned by Apple).

OTOH, the used compiler and linker does not know about it (that's why the issue is here in the first place), which is arguably a fault on the homebrew side (e.g. they should provide a way to enable these non-standard paths by default; but that is beyond this issue).

Also note that homebrew's /opt/homebrew/include is a lot more standard than Fink's /sw/ or MacPort's /opt/local/ - at least according to the FHS (the latter two just (completely resp. half-way) ignore the FHS and invented paths on their own) - and yet pd-lib-builder automatically adds the fink-path

pd-lib-builder/Makefile.pdlibbuilder

Line 511 in e6cff66

cpp.flags := -DUNIX -DMACOSX -I /sw/include

why is that?

[umlaeute]

and yet pd-lib-builder automatically adds the fink-path

to keep my tirade going (but I'll stop soon): why does pd-lib-builder add -I/sw/include (which makes it find fink-provided headers) but not -L/sw/lib? Probably an oversight, but I think the only reason why nobody noticed is, that nobody actually used pd-lib-builder together with fink packages.

[umlaeute]

to keep my tirade going (but I'll stop soon)

so to conclude: i think the patch in this PR is really a no-brainer (in relation to prior art in pd-lib-builder (regarding fink)).

[danomatika]

I guess I should have stayed out of this. I was a bit annoyed that it’s fine to add extra behavior to smooth out build defaults here but not other places. Whatever. As far as “standards” I suppose it’s always the wrong term to use since Pd about 10 different meanings for it. enohp ym morf tnes ----------- Dan Wilcox danomatika.com robotcowboy.com

…

On Dec 6, 2022, at 9:00 AM, umläute ***@***.***> wrote:  to keep my tirade going (but I'll stop soon) so to conclude: i think the patch in this PR is really a no-brainer (in relation to prior art in pd-lib-builder (regarding fink)). — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[danomatika]

As a developer, I would rather not have extra paths added beyond what is already added by the compiler and my own shell paths. I could imagine running into an issue where I want to use a locally compiled lib but then I actually link to one installed on the path, however this is such an edge case as to not actually happen in practice and there are other approaches which mitigate it, ie. linking the static .a directly etc. At this point, I often appreciate having to be explicit about certain things, as we do in other areas, ie. compiling with certain options etc.

In all, you are right, this likely doesn't hurt anything.

[umlaeute]

how about this:

add a new variable extrasysdirs (or somesuch), that holds additional base directories (with include/, lib/,... subdirectories).
Any existing include/ directory within these $(extrasysdirs) is automatically added via -I, and the same of lib/ directories (via -L).

The user can override this variable to their likings (on the cmdline).
The default is (debatable; but i would suggest to be progressive and just add the known paths, so): /sw/ /opt/homebrew /opt/local.
(I shortly thought about /usr/local/, but that is probably futile, as most compilers/linkers include it by default, and there really is no way to turn this off)

If you do not want to have any of these paths, just run

make extrasysdirs=""

If you only want the shiny homebrew/m1 paths, run:

make extrasysdirs=/opt/homebrew

this variable should only be set from the cmdline, and not as part of the Makefile.

[patricecolet]

This

how about this:

add a new variable extrasysdirs (or somesuch), that holds additional base directories (with include/, lib/,... subdirectories). Any existing include/ directory within these $(extrasysdirs) is automatically added via -I, and the same of lib/ directories (via -L).

I've no reason to oppose this proposal, more Makefile.pdlibuilder builds on different architectures and build systems, less I'll have to edit Makefiles or make arguments, as end user, developper, or package manager.

[danomatika]

A extrasysdirs is not needed. I only noted one edge reason why we might not want to add all available paths, etc but I next wrote that it hasn't been a problem so far, so really don't worry about it.

Sorry for throwing too much into this. Merge the PR and lets move on.

[katjav]

Reading a few more times through this issue I find the suggestion to use pkg-config most interesting. In all practical examples I've seen so far, the base homebrew paths are not sufficient so you would still need to define lib-specific homebrew paths in the lib makefile. Adding homebrew paths in Makefile.pdlibbuilder does not seem useful to me therefore.

What if Makefile.pdlibbuilder would expose a variable where the lib makefile can just specify dependency name(s) like:

`pkg.config = sdl2`

and let Makefile.pdlibbuilder use pkg-config to get appropriate flags for preprocessor and linker? Not sure yet how to evaluate pkg.config, as it should still be possible to do this in the lib makefile:

define forDarwin
    `pkg.config = sdl2`
endef

But first question is whether a pkg-config route would be useful.

[katjav]

About -I/sw/include I will say that it is a historical remainder from the days when Makefile.pdlibbuilder could be used in the context of Pd-extended. I can't tell the reason anymore. Maybe Pd-extended had its include files stored there? And maybe some Pd-extended forks have inherited this behavior. Anyway let's not take this as prior art for the current discussion.

[umlaeute]

What if Makefile.pdlibbuilder would expose a variable where the lib makefile can just specify dependency name(s) like:

honestly i do not see the point of this.

as a user you:

• have to be aware that your lib's dependency uses pkg-config
• you have to know the exact pkg-config name of the dependency

so once you have figured this out, what do we gain from adding a single line

pkg.config = sdl2

instead of the slightly more convoluted

cflags += $(shell pkg-config --cflags sdl2)
ldlibs += $(shell pkg-config --libs sdl2)

adding the latter is not exactly rocket science.
otoh, it allows you provide a sensible fallback in case pkg-config is not available (not all packages provide .pc files; not all versions of packages provide .pc files; pkg-config might not even be installed), e.g.

ldlibs += $(shell pkg-config --libs sdl2 || echo -lSDL2)

so I do not really see the advantages of providing another feature that you need to maintain.

esp I don't know how it (EDIT: using pkg-config in general) helps us with the problem at hand (finding stuff in /opt/homebrew).
(it might help us; e.g. if on homebrew-enabled Darwin/arm64 systems /opt/homebrew/bin was automatically added to the search PATH and pkg-config from this folder was therefore automatically picked by the system and this would then somehow inject /opt/homebrew/include in the header search paths. but again, this would only work for dependencies that actually provide a .pc snippet)

EDIT the last paragraph has actually already been shown to be correct in #74 (comment) (i should have re-read the thread before posting)

[umlaeute]

Re: /sw/include:

Maybe Pd-extended had its include files stored there?

iirc hans was somehow engaged in Fink (or at least, he had Fink on the radar, as back then it was practically the (usable) package on OSX), so it was somewhat natural to support it out-of-the-box.

[danomatika]

Actually, there is another package manager for mac, I forget the name already. My point is that, yes, there are some useful solutions which have set install locations, but I feel that, like building any C/C++ program, if you are using a lib installed to a non-standard (my meaning: /usr/lib or /usr/local/lib), you would specify the header and lib loading location. I don't see this as onerous to potential developers and simplifies maintenance for us if the makefile doesn't need to chase too many platform-specifics beyond the important stuff like possible architectures and Pd install locations.

I also don't see a strong need for a pkg-config helper, although it may improve accessibility for some. I feel like documentation about this is probably enough. For instance, I didn't know about IOhannes || trick if pkg-config is not available, nice.

UPDATE: Actually, there are two other current package managers, pkgsrc and nix, and of course, the historical Fink.

UPDATE: OTOH, we can simply set install locations for everything and hope there are no conflicts or people using multiple managers at the same time. :)

[katjav]

honestly i do not see the point of this.

and

I also don't see a strong need for a pkg-config helper

Sorry for my pavlov response to pkg-conf. Indeed documentation or links to libs using existing features in creative ways will be more helpful than new features at this point. In this context I've responded to @umlaeute 's discussion about mission statement #75.

[danomatika]

Thinking about this again, I feel it's fine and good to just add /opt/homebrew. I have done this already for several of own auto tools-based builds and it should just be noted that those paths are set in the platform-specific docs section of the either the readme or tips and tricks.