search

sudara / pamplejuce

A JUCE audio plugin template. JUCE 7, Catch2, Pluginval, macOS notarization, Azure Trusted Signing, Github Actions
https://melatonin.dev/blog/
MIT License
372 stars 36 forks source link

CMakeLists improperly configured for older versions of MacOS #67

Closed dar-io-p closed 6 months ago

dar-io-p commented 7 months ago

the call to "include(PampleJuceMacOS)" is after the call to "project(...)" which causes broken binaries to be built for older versions of macos. if you include the MacOS config before the call to project then the issue is fixed.

sudara commented 7 months ago

Thanks, I've been using FORCE to solve this for my own projects, only reason I haven't updated Pamp is actually.... not sure what to make the default. Leaning towards specifying 10.13 after reading https://forum.juce.com/t/macos-version-market-share-data/59088/10 — what do you think?

dar-io-p commented 7 months ago

I think 10.13 is a good base deployment target, however I'm unsure why you wouldn't simply put the "include(PampleJuceMacOS)" before the call to "project()", because the cmake documentation clearly states so. https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_DEPLOYMENT_TARGET.html

I spent quite a large amount of time trying figuring out why my plugins weren't working on one of my tester's machines and it was just because of this misordering that I inherited from the pamp repo. I wouldn't want anyone else doing the ol' run around because of this.

sudara commented 7 months ago

Sorry you had trouble! Yes, I think moving it to before the project is the right call, I just happened to be using FORCE to get it to work in a recent project of mine.

sudara commented 7 months ago

Ah, the reason I was considering the FORCE option for Pamp (which works fine too) is that then people pulling cmake-includes would get the update and I could keep existing behavior while adding a comment or something for when they want to increase compatibility. But in the end, I think it's more pragmatic to have the template force compat down to 10.13.

I think what I'll have to do is setup some BREAKING_CHANGES.md or something to list out manual things that people might need to check while upgrading. There's sort of a big difference between "works for me" and "works for everyone" and "works for people upgrading over time" (lots of moving parts, not sure if this is reasonable, but it's what I do for my projects)