-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Update hook-gi.repository.GdkPixbuf.py
for Great Glory
#1843
Conversation
It would appear that Travis utterly hates dynamically defined tests (i.e., tests defined at runtime via Very well, Travis; you win this round. I'll fix this shortly. |
Good observation! I knew about the
I know that Aha! pytest has its own |
Yup! But Perusing the PyGObject docs, any brute-force solution will fall down in the face of sheer overwhelming numbers at some point. There are an absurd number of
Sweet leaf incarnate! That function improves upon our own by also supporting version checking, ala:
Our I propose refactoring our
I absolutely have not tested this. But it might work. The worst that could happen is Great Cthulhu rising from the ancient depths of R'lyeh. |
Brute force for the violent win. Travis now passes. AppVeyor we ignore, of course. Windows. Always Windows. |
Nailed it. The existing Additionally, I've refactored our We shall see how Travis feels about all this. I'm watching you, Travis. |
Houston, we have ignition. |
And that should do it. Our suite of Matplotlib tests were also heart-breakingly broken. Despite being parametrized, the
If the default Matplotlib backend is a Qt package (as it is on my machine), these tragicomic bullet points imply that Travis silently accepted this, which is almost worse. I haven't investigated why. Maybe Travis refuses to install Qt packages and hence silently skips these tests. Maybe? Anyway, they work now. Three cheers for the Japanese goblin! 👺 |
The Windows test failures appear to be unrelated issues. On Windows, the
Since In theory, these test failures should be ignorable with respect to this pull request. |
O.K.! That should do it. Our SciPy test was broken for SciPy >= 0.16.0. The Godslayer and baby narwhals willing, no further commits will be appended to this pull request. 🐳 |
@htgoebel I summon thee, Teutonic wizard! @springermac Thanks for heroically tackling the Python 3-specific It's not your fault. You did a bang-up job. (That's good.) Your elegant solution calls our homebrew
|
Filenames:
|
Sorry for the wall of text. The executive summary is that the fundamental problem you found with |
Save us, Jabberwocky. This rabbit hole is deep and nefarious. I appreciate the Shield-Wall of Text and raise my rusty vorporal sword as a salute in your general direction.
Backwards compatibility! Sense is made. I whole-heartedly agree. Existing workiness must be preserved. Nonetheless,
Naturally,
I would someday like Python 2.7 to go away. Permanently. Until then, I giddily support a gradual transition from
For all other purposes (...pretty much everything), only I have no code and I must scream.
...that's insane. See: "I have no code and I must scream."
It's not simply encoding errors, however.
I am vomitting into my mouth as we speak. Converting to That said, a very gradual conversion to
Everything you just wrote blew up my precarious mind.
wat
Yay! Wait. I'm correct about a thing? Galactic implosion imminent.
O.K., so I'm actually incorrect. Galactic stability restored.
"...as we know, there are known knowns; there are things we know we know. We also know there are known unknowns; that is to say we know there are some things we do not know. But there are also unknown unknowns – the ones we don't know we don't know." – an unpleasant human being All of which is to say... You've Convinced Me.You're also awesome. (You know. Thought you should know.)
For orthogonality, I should probably create a new tl;drA new |
Thankfully, this is the one case where we can absolutely, positively know how the called process' text output will be encoded. For Python 3, at least. In Python 3, the stdio streams are wrapped with encoders that convert incoming text to bytes using a given encoding. The encoding defaults to the locale's preferred encoding, and can be overridden using the
This is true. I blame the C language for this hell. Who thought it was a good idea to use
I'd like to find out more about these cases, and about any specific problems that arise on Python 2.7 because the output is handled as |
You're probably right. But that "usually" concerns me, a bit. Though I have no specific counterexamples at the moment, I can't help but involuntarily cringe whenever I see a I was especially concerned about the Asiatic encodings still in common use (e.g., Shift-JIS, GBK), but you've destroyed that fear. I'm delighted to learn they sensibly avoid conflict with the lower half of the ASCII spectrum. How noble! How sane! I then realized with a gnawing horror in my soft intestine that there are extremely common encodings that don't avoid conflict with the lower half of the ASCII spectrum. Indeed, their codepoints are explicitly incompatible with ASCII codepoints. UTF-16 and UTF-32: We're Summarily FuckedUnlike UTF-8, UTF-16 and UTF-32 are both ASCII-incompatible. Since neither shares the ASCII codepoints for the The ASCII codepoints for "Lookin' good. What's the problem, pops?" Pops shall now tell you. The upper byte of the UTF-16 encoding for any ASCII character is Attempting to pass UTF-16-encoded
That's good. You just can't do anything with the resulting paths in Python, except feed them to something else that isn't Python. That's bad. I expect similarly bad issues with UTF-32-encoded pathnames. You Volunteering to Fix It?Nope. I ain't volunteering for nuthin' except my head on this pillow. 😴 |
Absolutely. I might have glossed that bit over because of how unlikely it is that we would ever get anything in either of those encodings, because...
Therefore, you will never find a Unix-like system where Anyway, we're getting a bit too far off topic. I'll keep this discussion in mind for when I eventually try to convert everything over to This PR looks good to me! |
Actually, I was wrong. You can't even safely split UTF-16 encoded pathnames on either The pain. It hurts.
Yup. What's oddball about that is OS X. As a BSD derivative, it's Unix-like and thus prohibits UTF-16 locales. But its default filesystem, HFS+, encodes pathnames as UTF-16. Bizarro World strikes again.
We both secretly know which of those two eventualities is likelier. 😉 Thanks for the erudite commentary, again. You rocked my encoding world, especially with all of the extremely fine-grained Windows effluvium. What is it with that platform? Don't answer that. |
ac7a741
to
39b49a9
Compare
Tests fail, because #1885 broke the build. It's not my fault. Even though I incited htgoebels to do it with my |
This pull request is now good to go. A new To the end of all bugs, and beyond! 👽 |
Rebased without modification in hope of passing tests. Let's see! |
4f60d4d
to
2f5f629
Compare
Tests pass. 👻 |
Fixed the related issue #1867, just because. This completes the noble tale of this pull request. No additional commits will be added. No existing commits will be revised. All's well that finally ends. @codewarrior0 You know what to do, my friend. |
All right, I looked over the changes again and everything looks good. I'll wait for the CI to finish before I merge this in. I'm going to add a note to the docstring for |
Yeah, that's pretty essential. Apologies for neglecting that. I suppose a pragmatic compromise between these two warring camps might be to:
Want me to edit that in? I hate stepping on toes, but I could probably whip through that carnage in a jiffy. |
Sure, be my guest! FYI, if you write |
The failures on AppVeyor are due to failing to build Numpy from source. It looks like Numpy 1.11.0 was just released hours ago, and they haven't got around to publishing Windows binary wheels (which they had for the previous version). I'll trust that the tests pass on Travis. When the numpy guys get around to publishing a Windows binary wheel, I'll give it a rebuild. |
(Did you rebase on the wrong branch?) |
Ugh. I didn't rebase on the wrong branch, so I'm not quite sure why the insanity. Let's try a pull and see if that cleans my |
A new compat.open_file() utility function simplifying file handling across both Python 2 and 3 has been added. Under Python 3, this aliases the open() built-in; under Python 2, this aliases the io.open() built-in, which is a backport of the Python 3 open() built-in and hence usually preferred to the mostly obsolete Python 2 open() built-in.
A new compat.exec_command_stdout() utility function reliably capturing standard output from external commands in a portable manner has been added. The legacy compat.exec_command() and compat.exec_command_all() utility functions do not reliably capture either standard output or error. For backward compatibility, these functions have been preserved as is but are now deprecated when called for their return values. Docstrings have been significantly improved. [ci skip]
A new compat.which() utility function has been added. For Python 3.x, this is synonymous with the shutil.which() function introduced by Python 3.3. For Python 2.7, this a backported copy of the shutil.which() function defined by the most recent stable version of Python 3 as of this commit: Python 3.5.1.
The "gi.repository.GdkPixbuf" hook now supports Python 3 under all supported platforms by decoding loader cache output only when necessary (like under OS X) in a robust, portable manner enabling the "universal_newlines" option and calling the Python 3-style open() builtin. This hook is also now ignored with a non-fatal warning if GDK and hence GdkPixbuf is unavailable on the current system. This fixes pyinstaller#1867.
All PyGObject functional tests have been moved from the existing "test_libraries" module into a new "test_gi" module. All tests requiring PyGObject >= 2.0 are now parametrized, trivializing test creation and maintenance -- including the addition of a new "gi.repository.GdkPixbuf" test.
The test_matplotlib() tests repeatedly implicitly tested the default backend rather than the currently parametrized backend. These tests now test the latter by explicitly configuring Matplotlib to do so, and have been moved from the existing "test_libraries" module into a new "test_matplotlib" module.
The @importorskip test decorator now supports fine-grained skipping of tests requiring minimum module versions by internally deferring to the official pytest.importorskip() helper. Doing so requires @importorskip to accept only one rather than multiple module names. Existing tests passing multiple module names have been refactored in the trivial way to pass only one such name.
The "scipy.lib" package imported by the test_scipy() test has been privatized by SciPy 0.16.0 to "scipy._lib". This test now conditionally imports the desired package depending on SciPy version.
That looks sane(r). |
Ugh. Thanks for the |
As always, your help is much appreciated, @leycec. |
Looks like you hunted down another beast of legacy. Thanks @leycec (and of course @codewarrior0 ). |
Greetings and wintry salutations, PyInstaller goodfellows.
This pull request generalizes wailinygn's open pull request for the
hook-gi.repository.GdkPixbuf
hook – now with 43% more support for:What? But... There's Already a Pull Request.
Yes. Yes, there is. And it's awe inspiring in its simple simplicity. Unfortunately, it also:
This is a blocker for my lazy hook loading work. The current version of this hook causes other seemingly unrelated tests (e.g.,
test_matplotlib()
) to fail, making it difficult for me to differentiate tests failing due to my work from tests failing due to this erroneous hook. It's also a blocker for my Matplotlib-based application.Since the original pull request was moving a little... lethargically, I thought I'd pick up the slack, amp up the volume, and jack into a new reality. Thanks for laying the groundwork, wailinygn.
Sane File Handling
Performing safe file handling across both Python 2 and 3 is insane.
The crux of the insanity is Python 2's
open()
built-in. It sucks; it's fragile; it's unreliable; it's uninformative; it's broken by design. To fix the suckage, Python >= 2.6 backported Python 3'sopen()
built-in under a different name:io.open()
. Under Python 2.6 and 2.7,io.open()
should typically be called in lieu ofopen()
. Under Python 3,io.open()
andopen()
are the same function. Hence:To sanitize this insanity, a new
compat.open_file()
utility function simplifying file handling has been added. Under:open()
built-in.io.open()
function.Of course, since
open()
isio.open()
under Python 3,compat.open_file()
is technicallyio.open()
under both Python 3 and 2. But no one ever remembers to callio.open()
(as evidenced by the absence ofio.open()
calls in the PyInstaller codebase), including that fat, lazy Canadian leycec. By the thunder hammer of Thor, may the existence ofcompat.open_file()
be a light in the darkness encouraging others to stop calling theopen()
built-in!Where's Thor's right foot? I don't even know.
Python 3 under OS X
codewarrior0 astutely notes that the
hook-gi.repository.GdkPixbuf
hook currently fails hard on Python 3 under OS X:Inspired by that, this pull request writes
cachedata
as follows under OS X:universal_newlines=True
option to thesubprocess.check_output()
call, silently decodingcachedata
with the user's preferred locale (e.g.,utf-8
) into a:unicode
object on Python 2.str
object on Python 3.cachedata
in a type-preserving manner. The original version of this hook rejoinedcachedata
with'\n'.join(cd)
, which accidentally convertedcachedata
from aunicode
tostr
object on Python 2. Whoops. This pull request changes that tou'\n'.join(cd)
, preservingcachedata
as aunicode
object.cachedata
using our newly definedcompat.open_file()
helper, ensuring that string is implicitly encoded with the same character encoding that thesubprocess.check_output()
call used to decode that string.Did I mention that file handling is insane? 'cause... it is. Insert sad face.
Python 2 or 3 under Linux or Windows
Under all other supported platforms, codewarrior0 again observes that
cachedata
is not modified and hence may be written to disk as is (i.e., as a raw encoded byte string) without extraneous decodes or encodes.The prophet is right, of course. And we have little choice but to implement his sage wisdom.
Test Us Up the PyGObject Bomb
To ensure this never, ever happens again, a new
test_gi_gdkpixbuf_binding()
functional test has been added. Like every other PyGObject test, it's boring to the point of eye-gouging: it just attempts to import thegi.repository.GdkPixbuf
module and hence load thehook-gi.repository.GdkPixbuf
hook. That's it.For maintainability, all PyGObject tests have been shifted from the
test_libraries.py
grab bag into a new dedicatedtest_hooks/test_gi.py
script. Because! It must be done. (Cue furious hand-waving.)That's It – But That's Enough
Thus ends this sordid chapter in the unsung saga of PyInstaller.