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

Use latest Mac OS X SDK #371

Closed
wants to merge 15 commits into from
Closed

Use latest Mac OS X SDK #371

wants to merge 15 commits into from

Conversation

cjh1
Copy link

@cjh1 cjh1 commented Jul 30, 2020

Set WITH_LATEST_OSX_SDK so that the latest SDK is used.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Fixes #365

Some projects require newer versions of the OSX SDK to be used, even though the deployment target may be older. Qt is one of those cases, and using an older version of the OSX SDK to build Python, for instance,
is causing some bugs when used with PySide2.

The ability to do this was added recently in conda-forge/conda-forge-ci-setup-feedstock#106. This PR enables that functionality.

Set WITH_LATEST_OSX_SDK so that the latest SDK is used.
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@cjh1
Copy link
Author

cjh1 commented Jul 30, 2020

@conda-forge-admin, please rerender

@cjh1
Copy link
Author

cjh1 commented Jul 30, 2020

It seems like the rerender wiped out my changes?

@mingwandroid
Copy link
Contributor

This change breaks the ability to run the built python on old installs of macOS.

@cjh1
Copy link
Author

cjh1 commented Jul 30, 2020

@mingwandroid I don't think is does, it doesn't change the deployment target which is still set to 10.9. My understanding is the Mac SDK are designed so you can still target older version of the OS.

@psavery
Copy link

psavery commented Jul 31, 2020

@mingwandroid Apple set up the SDKs so you can build with a newer one and specify an older runtime target. See here.

@mingwandroid
Copy link
Contributor

Apple set up the SDKs so you can build with a newer one and specify an older runtime target. See here.

I have been developing Apple cross compiler tool chains for about 8 years now. That stuff plain does not work for anything non-trivial.

@cjh1
Copy link
Author

cjh1 commented Jul 31, 2020

@mingwandroid You have run into specific problems with python? We have used it with non-trivial Qt applications. If this is not an approach that will work, is there a way that we could build two versions, one using the latest SDK and upload it will a label? Our application it pretty much broken on the Mac without using the latest SDK, it looks like others are also running into these issues. We are just looking for a decent solution.

@mingwandroid
Copy link
Contributor

You have run into specific problems with python?

If you make a Python that links to a newer SDK that calls functions from that newer macOS SDKs APIs then that Python (or at least those APIs) will crash or not work on older versions of macOS. If you make a python that links to a newer SDK but does not call any of those functions then you may as well compile it against the oldest SDK that you can to guarantee compatibility with older macOS.

I'm very aware that Qt project (via chromium) use new macOS SDKs and APIs and yes, our Qt libraries are built against those, out of necessity. A pyqt then built against that and loaded by such an old-SDK python will work just fine on your more modern macOS systems.

We could make a new Python but please, show me what it would fix. The issue you linked to is orthogonal to this issue, it's about Qt, and yes, I agree, there is a need to use new SDKs there.

@mingwandroid
Copy link
Contributor

A variant against new SDKs can for sure be created and it's not a huge effort, but if we then need to rebuild all compiled extension modules against this new-SDK-python then it is a problem.

But please, present me with tangiable facts, reprodicuble failures and test-cases and I will look into this more.

@psavery
Copy link

psavery commented Jul 31, 2020

@mingwandroid A lot of this is documented already in the issues that are cross-referenced in #365.

The main issue is that GUI elements do not update properly. You have to resize the application window or make the application lose focus in order for them to update. However, compiling python with a newer OS X SDK makes the issues go away.

Here's one example I just tried that illustrates it. First, make a fresh conda environment and run this code:

conda install -c conda-forge python=3.8 pyside2

Then, run this test.py script linked in one of the cross-referenced issues (note, they used pip there, but we are not using pip here), pasted below.

import sys

from PySide2.QtCore import QSize, Qt
from PySide2.QtWidgets import *


class GraphicsView(QGraphicsView):
    def sizeHint(self):
        # Return scene size by default
        return QSize(500, 500)

    def __init__(self):
        super().__init__()
        self.setScene(QGraphicsScene())
        self.scene().addText('ABC')
        self.scale(20, 20)

    def test(self):
        self.scale(0.9, 0.9)
        # self.update() <= This doesn't help either


class Test(QWidget):
    def __init__(self):
        super().__init__()

        layout = QHBoxLayout()
        self.setLayout(layout)

        view = GraphicsView()
        layout.addWidget(view)

        auto_button = QPushButton('auto')
        auto_button.clicked.connect(view.test, Qt.AutoConnection) # Also doesn't work with DirectConnection
        layout.addWidget(auto_button)

        queued_button = QPushButton('queued')
        queued_button.clicked.connect(view.test, Qt.QueuedConnection)
        layout.addWidget(queued_button)


app = QApplication(sys.argv)
view = Test()
view.show()
sys.exit(app.exec_())

When you click the auto button, the window does not update immediately:
not-adjusted

But then if you resize the window or make the window lose focus, it then updates:
adjusted

If we compile python on the machine using code like this (the machine has SDK 10.15 installed on it):

git clone https://github.com/conda-forge/python-feedstock
mkdir output
conda install conda-build -y
conda config --append channels conda-forge  # required for clang-9
conda build --output-folder output -m python-feedstock/.ci_support/osx_python3.8.____cpythontarget_platformosx-64.yaml python-feedstock/recipe

The issue goes away.

@mingwandroid
Copy link
Contributor

Great, thanks for this. I'll study it carefully when I get Qt 5.15.0 packages built and a few other work bits. If a new python variant does fix this that's great, but I'd like to see if there is an easier way.

@cjh1
Copy link
Author

cjh1 commented Jul 31, 2020

@mingwandroid Thanks for taking a look at this.

@rggjan
Copy link

rggjan commented Aug 3, 2020

@mingwandroid I looked at this extensively, and this is the only way I found it can be fixed. It makes PySide2 on conda plain unusable, because a lot of signal-slot stuff just doesn't fire.

The reason is that OSX (or more specifically AppKit) enables certain OS features only for applications built with a newer SDK. One example is dark mode: As mentioned on these older Qt docs, OSX enables dark mode support only when linking against SDK 10.14 or newer, meaning you can "opt-out" of dark mode by linking against SDK 10.13 or older. This was supported for a time in Qt, however, the newest official Qt docs clearly state that only the 10.15 SDK is supported, with a target platform of at least 10.13. Unfortunately, this enabling of features by OSX is decided based on the version of the binary (in this case python) and not of the library using it (Qt). The current issue is that some of the signal-slot dependency seems to depend on functionality not enabled by OSX (possibly layer-backed views or something similar) when linking against older SDK's.

I've been investigating and trying to find a fix to make PySide2 usable on OSX for a year now (this is the first of many bug reports and PR's filed in the process). Would be great to finally get this approved now that a solution is so close at hand...

See also:
https://bugreports.qt.io/browse/PYSIDE-1083
conda-forge/conda-forge-ci-setup-feedstock#67
conda-forge/conda-forge-ci-setup-feedstock#106
conda-forge/conda-forge.github.io#891
conda-forge/pyside2-feedstock#45
conda-forge/pyside2-feedstock#46
#365
#275
#288
ContinuumIO/anaconda-issues#11297

@mingwandroid
Copy link
Contributor

Great. Thanks for all the references. As soon as I get a new conda build release done and a few other tasks I will lobby to give this my full attention. In general macOS needs some love!

I'm on my phone, traveling so haven't looked at them yet.

I'm not sure why dark mode needs to depend on the os version or if it's a good reason to bifurcate things here. The signals slots issue is serious though for sure. Do you think a test could be added to the pyqt recipe at conda-forge that captures this failure?

@rggjan
Copy link

rggjan commented Aug 4, 2020

Thanks @mingwandroid!

The dark mode was mainly meant as an example. It's another thing that doesn't work currently due to the same problem, but I agree that it is much less severe.

Looking at the pyqt OSX build, I don't see pyqt_test.py being executed (as opposed to the pyqt linux build, where it is run like this: xvfb-run -a bash -c 'python pyqt_test.py'), so I assume that the OSX build server doesn't allow running GUI stuff for some reason...

@mingwandroid
Copy link
Contributor

so I assume that the OSX build server doesn't allow running GUI stuff for some reason...

Maybe .. it could just be an oversight though. Qt apps should support the --platform offscreen option too.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@cjh1
Copy link
Author

cjh1 commented Aug 26, 2020

Here is the list of symbols for the package: symbols.txt

@isuruf
Copy link
Member

isuruf commented Aug 26, 2020

Is that python executable only or all the standard library modules as well?

@cjh1
Copy link
Author

cjh1 commented Aug 26, 2020

@isuruf That is just the python executable.

@rggjan
Copy link

rggjan commented Nov 2, 2020

@mingwandroid just wanted to quickly check what the status is?

@rggjan
Copy link

rggjan commented Nov 13, 2020

On macOS Big Sur, Qt doesn't work at all anymore. See here:
https://bugreports.qt.io/browse/QTBUG-87014

It seems that linking against this old, unsupported SDK breaks something to do with layer-backed and surface-backed views. So installing python from conda and running any PyQt/PySide2/... application now crashes on the newest mac OS.

This PR would fix this issue as well, based on my testing. As written in the linked bug report:

In any case, as recommended both by the Qt docs and Apple, you should always build against the latest SDK, so I highly recommend that.

@mingwandroid
Copy link
Contributor

This PR would fix this issue as well, based on my testing. As written in the linked bug report:

In any case, as recommended both by the Qt docs and Apple, you should always build against the latest SDK, so I highly recommend that.

@rggjan, so long as people with older OSes can still run things then fine. Different organizations and endeavours recommend different things for their own reasons.

  1. Apple care inversely about supporting old hardware and old OSes. It is in their interests for you to upgrade both. Unfortunately some people cannot afford to stay on the shiny-update-treadmill.

  2. The Qt Company is beholden to go along with whatever Google decides SDK-wise due to QtWebEngine. Their recommendation largely stems from this (and excludes the option to remove these new, breaking features) and, I expect, from not wanting to support a lot of different OSes on end-user machines.

I want those people who cannot afford new Apple hardware to continue to be able to perform data-science tasks within the Anaconda ecosystem.

That is all.

@mingwandroid
Copy link
Contributor

And I should say, this has always been a core feature of Anaconda, the ability to depend on it to continue running on your hardware for a reasonably long time.

@rggjan
Copy link

rggjan commented Nov 13, 2020

@mingwandroid Thanks for the comment. I don't want to start an old argument... I basically just wanted to let you know that:

  1. qt on conda is broken completely on the newest OSX (as opposed to not behaving correctly but not crashing, as it was before)
  2. this PR would fix the issue

@isuruf
Copy link
Member

isuruf commented Nov 13, 2020

@rggjan, I've been using this option lately in other packages and I'm open to do this. I checked and it looks like the autoconf script thinks clock_gettime is available when building with the 10.15 SDK and targetting 10.9, but this is only available >=10.12. We'll have to go through each option and turn those off. If we can do this, and also fix the tests, I'll be happy to merge.

@mingwandroid
Copy link
Contributor

You cannot just say "I don't want to start an old argument" then proceed to make argumentative points.

You cannot break everybody else just so that your stuff works. As @isuruf said, we need to take care of these matters for our users. We cannot be so reckless.

@isuruf
Copy link
Member

isuruf commented Nov 13, 2020

@mingwandroid, I don't see that as an argumentative point. Those 2 are facts and I'd like to add the 3rd point that this PR at its current state would break older macos version

@mingwandroid
Copy link
Contributor

Sure thanks @isuruf, hopefully we can satisfy everyone's needs here. That is the end goal of course!

@mbargull
Copy link
Member

@rggjan, if you'd be able to work on the option outlined by @isuruf, that'd be helpful because it seems to be a non-controversial solution to the immediate issue at hand which could probably be deployed right away (as opposed to figuring out first what newer restrictions we want to impose on the users' systems).


(@mingwandroid, BTW we discussed when/why/how changing targeted minimum macOS versions in the last conda-forge/core meetings. If you're interested and schedule permits, I'd like to (voice) chat with you about this a bit. Please let me know if that'd be possible for you.)

@mingwandroid
Copy link
Contributor

@mbargull Sure, ping me on gitter and we'll set something up.

@isuruf
Copy link
Member

isuruf commented Nov 26, 2020

@rggjan, good news. cpython master now supports building using a newer SDK than the deployment target. See https://patch-diff.githubusercontent.com/raw/python/cpython/pull/22855.patch

@mbargull
Copy link
Member

@cjh1, @rggjan, @mingwandroid, PTAL at gh-412 from @isuruf which addresses this (for the upcoming 3.9.1 December release).

@mbargull mbargull mentioned this pull request Nov 27, 2020
5 tasks
@mingwandroid
Copy link
Contributor

Well sounds fine if it works!

@mbargull
Copy link
Member

mbargull commented Dec 4, 2020

Done in gh-412.

@mbargull mbargull closed this Dec 4, 2020
@rggjan
Copy link

rggjan commented Dec 4, 2020

How can we verify this worked?

For me, PySide2 still crashes with

  python             conda-forge/osx-64::python-3.7.8-h4f09611_3_cpython

built 5 days ago.

Also, looking at the binary with otool -l I see:

      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.9
      sdk n/a

and not the expected

      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.9
      sdk 10.15

@rggjan
Copy link

rggjan commented Dec 4, 2020

Oh, I get it... this was for python 3.9.1 only.

The necessary patches seem to be included in python 3.7.9 as well, though, so we could put the changes on this PR onto the 3.7 branch to make it work there as well...

@mbargull
Copy link
Member

mbargull commented Dec 4, 2020

The necessary patches seem to be included in python 3.7.9 as well, [...]

Are they? Can you give a reference? IIUC, upstream the patches have been ported to the 3.9 branch and are planned to be backported to 3.8 as well, though I couldn't see any reference to 3.7.

@rggjan
Copy link

rggjan commented Dec 4, 2020

Here:
https://docs.python.org/3/whatsnew/3.9.html

It says:

macOS 11.0 (Big Sur) and Apple Silicon Mac support
As of 3.9.1, Python now fully supports building and running on macOS 11.0 (Big Sur) and on Apple Silicon Macs (based on the ARM64 architecture). A new universal build variant, universal2, is now available to natively support both ARM64 and Intel 64 in one set of executables. Binaries can also now be built on current versions of macOS to be deployed on a range of older macOS versions (tested to 10.9) while making some newer OS functions and options conditionally available based on the operating system version in use at runtime (“weaklinking”).

(Contributed by Ronald Oussoren and Lawrence D’Anna in bpo-41100.)

and bpo-41100 is also included in 3.7.9:
https://docs.python.org/release/3.7.9/whatsnew/changelog.html#changelog

But I'm not 100% sure... as you say, it doesn't seem to be explicitly mentioned...

@mbargull
Copy link
Member

mbargull commented Dec 4, 2020

Ah, I see what you mean. Yes, all those changes reference bpo-41100. However, the ones that have been included in 3.7 earlier this year are only some of those that landed into 3.9.1 now and exclude the backwards compatibility patches.
Meaning, you'd have to backport patches from python/cpython#22855 yourself if you'd want this for 3.7 builds. (Plus, IDK what the expected time frame for 3.8 backports upstream are.)

@rggjan
Copy link

rggjan commented Dec 4, 2020

Oh I see, thanks for digging into this!

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.

Build with the latest SDK on macOS
9 participants