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

config_darwin.py improvements #2768

Merged
merged 2 commits into from Oct 16, 2021
Merged

config_darwin.py improvements #2768

merged 2 commits into from Oct 16, 2021

Conversation

jmroot
Copy link
Contributor

@jmroot jmroot commented Oct 10, 2021

The actual code of frameworks shipped with the OS is no longer present as a file in the filesystem (it's only in the shared cache).

The actual code of frameworks shipped with the OS is no longer present as a file in the filesystem (it's only in the shared cache).
Copy link
Contributor

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for submitting a PR to pygame! 👍

I don't have a mac, so I can't test this locally. AFAIK, we don't actually use FrameworkDependency on our mac builds ATM. From the code, I see only sdl1 stuff using this (which we don't use now, and are planning to drop in a future version) and also some porttime/quicktime stuff, which I am not sure whether we are using these ATM.

But that is fine, this PR should still be good to go!
My only other concern is that does this change break compatibility with older mac versions which have the frameworks as files?

@jmroot
Copy link
Contributor Author

jmroot commented Oct 12, 2021

My only other concern is that does this change break compatibility with older mac versions which have the frameworks as files?

It doesn't; the enclosing directory will still be there on any OS version.

Copy link
Contributor

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right! That makes sense. Then this PR LGTM! 👍

Also I will allow the CI to run on this

It looks like the only reason direct use of CoreMIDI was added in
e51ce10 was as a slightly odd workaround for the Dependency class's
inability to handle a dependency on a header without a corresponding
library. (porttime.h is installed by portmidi but there is no
libporttime.)

QUICKTIME doesn't appear to be used at all.
@jmroot jmroot changed the title Fix finding frameworks on recent macOS versions config_darwin.py improvements Oct 15, 2021
@jmroot
Copy link
Contributor Author

jmroot commented Oct 15, 2021

Updated to remove unnecessary framework dependencies, prompted by discussion in #2767.

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@illume illume merged commit c68e8a2 into pygame:main Oct 16, 2021
@jmroot jmroot deleted the patch-2 branch October 16, 2021 06:37
@illume illume added this to the 2.0.3 milestone Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants