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

Remove use of non-public C functions in ctypes_patch #114

Merged
merged 3 commits into from
Jul 2, 2018

Conversation

dgelessus
Copy link
Collaborator

This should fix #113.

Also includes a small update to the tox.ini to remove the dependency on the development version of pyflakes.

dgelessus added 2 commits May 22, 2018 20:27
We can't remove the manual dependency completely yet, but it's better
to use a release version than the development version from the repo.
Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

The approach makes sense. Unfortunately, I'm currently on the road, and Xcode is insisting on me doing an OS upgrade before I can do an Xcode upgrade before I can test this on device 😠 I'll be back in my office in a couple of days and should be able to verify this patch then.

# FIXME Remove the manual pyflakes dependency once flake8 depends on pyflakes >= 2.0.0.
# Currently we need to manually update pyflakes in order to use __class__, because pyflakes <= 1.6.0
# doesn't understand __class__. This has been fixed in pyflakes 2.0.0, but flake8's requirements don't allow that
# version of pyflakes yet.
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment still valid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, flake8's current setup.py only allows pyflakes >= 1.5.0, < 1.7.0. The reason for the upper version limit is explained in the flake8 FAQ.

Copy link
Member

Choose a reason for hiding this comment

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

Oh - right, now I get it. This is about forcing the pyflakes version because of flake8's stated requirement.

@freakboy3742
Copy link
Member

@dgelessus FYI - I tried out this patch over the weekend and had no success; however, I wasn't able to get my other "working" example (which commented out the line from the ctypes_patch that cause problems) to work, either...

The issue is that this problem only manifests on AdHoc and App Store builds, those take 5-10 minutes each time to prepare, and you completely lose all stdin/stdout debugging assistance when that happens (because, in Apple's wisdom, they've discard all stdin/stdout in favour of their infernal logging framework). I'll have to keep digging...

@dgelessus
Copy link
Collaborator Author

because, in Apple's wisdom, they've discard all stdin/stdout in favour of their infernal logging framework

I've thought about this before - would it be of any use to provide a custom output stream class in Rubicon, which NSLogs all output that it receives? That could be used in place of Python's regular stdout and stderr to redirect/duplicate the output into the system-provided log. As long as we install it early enough, of course (i. e. at the very top of rubicon.objc.__init__).

@freakboy3742
Copy link
Member

@dgelessus In the past I've hacked together something messy to do exactly that. Historically, I've done it at the C level so that I can catch errors that come up as part of Rubicon instantiation; although as long as we can install the handler early enough, I suppose a Python implementation would be sufficient.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I'm going to accept this on the basis that it broadly makes sense, and works in CI. It's not resolving my problems with deploying on iOS, but at this point, it's not clear if those problems are being caused by something else, and this patch is necessary for GNUStep support.

@freakboy3742 freakboy3742 merged commit 481dd6b into beeware:master Jul 2, 2018
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.

How to run rubicon-objc on linux
2 participants