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

CMakeLists improperly configured for older versions of MacOS #67

Closed
dar-io-p opened this issue Dec 9, 2023 · 4 comments · Fixed by #69
Closed

CMakeLists improperly configured for older versions of MacOS #67

dar-io-p opened this issue Dec 9, 2023 · 4 comments · Fixed by #69

Comments

@dar-io-p
Copy link

dar-io-p commented Dec 9, 2023

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
Copy link
Owner

sudara commented Dec 11, 2023

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
Copy link
Author

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
Copy link
Owner

sudara commented Dec 12, 2023

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
Copy link
Owner

sudara commented Dec 12, 2023

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)

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 a pull request may close this issue.

2 participants