fredrikaverpil / pyside2-wheels

Unofficial PySide2 wheel building with Travis CI and AppVeyor
41 stars 6 forks source link

"pyside2-lupdate" and "pyside2-rcc" Missing, Because "pyside2-setup" Is Hell #94

Closed leycec closed 6 years ago

leycec commented 6 years ago

Catch! Here's yet another problem you aren't paid enough to fix.

Unsurprisingly, the pyside2-lupdate and pyside2-rcc commands installed by the pyside2-setup repository don't appear to be bundled with binary wheels. Surprisingly, I actually need these commands for our million-dollar actually pennies multiphysics simulator to actually do anything.

Because I'm the only human alive or dead who cares about this, I delved deep into the pustulating innards of setup.py – yes, that setup.py – to see if I couldn't resolve this myself. In theory, I got pretty close. In actuality, I think I need you to clean up my sed mess again. :sob:

The underlying issue appears to be that setup.py fails to pass a scripts keyword listing the pyside2-lupdate, pyside2-rcc, and shiboken2 commands to the setup() function call at the tail end of the file. If my reading of the bdist_wheel subcommand is correct (it's probably not), passing this keyword the correct list should suffice to resolve this issue. Maybe?

These commands are executable binaries rather than Python scripts and hence cannot be appended to the existing entry_points/console_scripts list. Instead, the Command Line Scripts section of the Python Packaging documentation suggests use of the scripts keyword:

When we install the package, setuptools will copy the script to our PATH and make it available for general use. This has advantage of being generalizeable to non-python scripts, as well: funniest-joke [an example executable] could be a shell script, or something completely different.

On POSIX-compatible platforms, lines 961 through 970 of the prepare_packages_posix() method in setup.py install these executables from the repository-local pyside-package/PySide2 directory into a system-wide executable directory – typically, /usr/local/bin. Assuming these executables also reside in the former directory at bdist_wheel building time (I'm half-certain they do), we might consider replacing:

# This last line of `setup.py`...
)

# ...with the following two lines declaring these scripts.
    scripts=["pyside-package/PySide2/pyside2-lupdate", "pyside-package/PySide2/pyside2-rcc", "pyside-package/PySide2/shiboken2",],
)

To do so, the following sed statement* might have the desired effect: * completely untested, likely broken

sed -i -e '$ s~^)$~    scripts=["pyside-package/PySide2/pyside2-lupdate", "pyside-package/PySide2/pyside2-rcc", "pyside-package/PySide2/shiboken2",],\n)~g' setup.py

Clearly, this is yet another critical upstream issue. Since upstream is out to lunch in the Bavarian Alps, I wonder if we might be able to tag-team this one. I know you're terrifyingly busy, but would you mind giving the above a go at some point?

I'd do so myself... if not for my intransigent laziness and desire to sleep tonight.

leycec commented 6 years ago

Note to self: an upstream issue should be opened. Not that it particularly matters. I'll probably do it tomorrow, if nihilistic cynicism doesn't take me first. </sigh>

fredrikaverpil commented 6 years ago

@leycec I really understand your frustrations, but yes, you should definitively not report this here and instead take this upstream – and you can always cc me there and we'll have the discussion where the devs can also join in.

I'm setting up gerrit today so that I can quickly send patches their way, and not just open new bug reports. I think we're being a little unfair to the PySide2 devs not doing this and instead ranting in the comments of this project (no offence intended towards you, I do this all the time). If our patch is not 100% perfect, it'll be perfect once it goes through their review. We can always ask for their suggestions if we don't understand the big picture of things and just force a patch through much more quickly since we have personal/corporate/educational interests in doing so.

I once did contribute to PySide2 and it wasn't painful or time consuming at all. I just remember it was cumbersome to read up on how to get started with gerrit for PySide2 contribution. I might have to write down how I did it on my blog so I'll be able to remember it later on... (since I have now forgotten how to get set up and how to submit a patch).

fredrikaverpil commented 6 years ago

Adding; your issue not something that I particularly is having problems with, which explains my luke-warm response to you... sorry.

leycec commented 6 years ago

Hey, no worries. Submitting Gerrit patches absolutely is the ideal approach. Sadly, I lack the time, motivation, or resources to "do it right." Upstream devs are much better positioned to resolve this sort of thing. It is, after all, their job; it's what they're ostensibly paid to do.

Doing this right really entails installing, maintaining, and testing patches on both the 5.6 and 5.9 branches and hence installing multiple concurrent versions of Qt, LLVM, and <insert-who-knows-what-here>. It is unfathomable dependency hell.

I'm also not entirely confident that the sed expression embedded above is entirely safe in the general case, as setup.py already successfully installs these commands. Passing a scripts keyword listing these commands will reinstall these commands – possibly over the existing installed commands or possibly elsewhere, possibly with incorrect permissions or ownership. You really don't want to do both, which means that the existing approach taken by setup.py to install these commands should ideally be replaced by the scripts keyword-style approach delineated above. That's the right way. Again, the core issue is that their setup.py script is balls crazy and non-trivially difficult to maintain safely.

I was hoping for a quick GitHub fix – but, hey, that's cool. I'll probably end up manually cloning, compiling, and installing pyside2-tools within our client-side installation scripts. It's not ideal, but I simply need this to work. Months of work are in jeopardy of being flushed all away.

fredrikaverpil commented 6 years ago

I'm also not entirely confident that the sed expression embedded above is entirely safe in the general case, as setup.py already successfully installs these commands.

I definitively understand your perspective. I would argue the same way. But I think that if you are the driving force behind this having to be changed, that's what's required in this case. The devs (or me) doesn't feel that his particular issue is a priority, but you do.

So during a review, I'm sure you'll get enough info from them to understand what to do or not to do, and they'll most likely pitch in to help guide the patch get implemented in the finest way possible, fitting with the grander schemes of PySide2. At least, that's what I would assume.

I wrote a quick overview on how to get set up with Gerrit, you may want to check it out: https://fredrikaverpil.github.io/2017/09/30/contributing-to-pyside2/

I was hoping for a quick GitHub fix

Feel free to file a PR here, no problem! If your sed works, I'm guessing you'll be able to see that quite quickly in Travis? If you need a wheel to be built, you can just run the docker container locally.