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

Rewrite setup.py for smoother builds and more platforms #34

Closed

Conversation

danielballan
Copy link
Contributor

Hi Mike,

I just got back from SciPy 2014 in Austin, where a major theme was the python "build problem," the difficulty of installing packages with non-Python dependencies. Software developers from Microsoft, Enthought, and ContinuumIO were there all week helping us make progress. I spent the last two days in a room with people writing reliable recipes for building difficult packages. The community is generally rallying around conda's build recipes and binary installers, in conjunction with binstar, as a solution. (Hashdist is also doing good work in this area.) I'd like to be able to conda install ffmpeg and conda install av on all three platforms. (I'm not a Windows user, but a lot of my scientific collaborators are.)

There were also many people doing imaging work -- the scikit-image lead devs were there -- and I promoted PyAV as the far-and-away best solution for getting video into Python. Scikit-image recently dropped their support for video because OpenCV is so problematic. I'd like to see scikit-image adopt PyAV as a dependency. Your approach is the right the one, and no one else has put in the work to do this right.

To get wider adoption, I think PyAV needs a more standard setup.py. This is my attempt to simplify it. Some notes:

  • I am working off of v0.1 for simplicity. I can rebase on your master later.
  • Did pip install av ever actually work? It has never worked for me. It seems like make was required. My revised setup.py does not need the Makefile. I'm envisioning that you might keep Makefile for all its testing capabilities, but that a basic install would never touch it.
  • If the build encounters a problem like a missing library, I think it should raise informatively, not just print a message and continue. It was confusing to have builds finish "successfully" only to error on import because, say, _core.so wasn't built.
  • This draft of a simpler setup.py does not handle PYAV_HAVE... headers. I'll need to add that, I know.

Sorry for the long-winded message. Are you receptive to this kind of change? I'm no build expert, but I think there is enough interest and support in the community to work toward a reliable cross-platform build.

@mikeboers
Copy link
Member

Hi Dan!

Thanks for your enthusiasm and support!

As I'm sure you can tell, the build process was somewhat hacked together as I figured out how to bend it to my will. I'm certainly not sold on it, and welcome improvement.

Your Points:

I am working off of v0.1 for simplicity. I can rebase on your master later.

There hasn't been anything too significant in the build since then (minus small additions to the macros that you haven't reimplemented).

Did pip install av ever actually work? It has never worked for me. It seems like make was required.

It currently works for me. That doesn't mean that I don't believe you, because I know it is very fragile. The Makefile is required for developers of PyAV to cross-compile the Cython into C. (This isn't actually required, as your patch doesn't do that, but it allows me to distribute a version on the PyPI that does not require Cython).

If the build encounters a problem like a missing library, I think it should raise informatively, not just print a message and continue. It was confusing to have builds finish "successfully" only to error on import because, say, _core.so wasn't built.

Totally with you here.

This draft of a simpler setup.py does not handle PYAV_HAVE... headers. I'll need to add that, I know.

Yeah. The FFmpeg vs. Libav thing is a nightmare. We used to depend on a configure script too...

My Observations

I have never seen dist.Distribution(dict(setup_requires='pkgconfig')) before. It seems to install that package as an egg in the current working directory. Is that correct? Is that safe?

I'm ready to be easily convinced, but is the automatic discovery of the modules to build really that bad? =P

Cheers,

Mike

@danielballan
Copy link
Contributor Author

Great. This is exciting.

For shipping pip packages that don't require cython to build, pandas has a less brittle (but still convoluted) approach. I will dig into that.

As for dist.Distribution, that is basically a path to the setuptools parameter install_requires, which is widely used. But there are a few cleaner ways to implement it -- leaving that egg sitting in the source directory is not what we want. Once I'm sure I've found the best way, I'll update the PR.

I expected this PR to stay a work-in-progress until it is thoroughly tested on OSX and Travis. I dream of Windows, but maybe not in the first round.

@danielballan
Copy link
Contributor Author

Oh, yeah, maybe automatic discovery is fine. This way was part of my learning process. :- D

@nicosmik
Copy link

Ok I understand.
I have the same issue with pkg-config under Cygwin.
I installed pkg-config.exe in my Cygwin environment, but when I start the setup.py script it does not found the ffmpeg librairies (even if they are in my environment variable PATH).
I also tried to build the code, maybe it succeeded (no errors) but I don't know how to setup the plugin in the site-packages python section.
Thanks, I will follow your commits.

@mikeboers mikeboers changed the title BLD: Refactor setup.py so make is not needed. Rewrite setup.py for smoother builds and more platforms Jul 17, 2014
@mikeboers
Copy link
Member

For shipping pip packages that don't require cython to build, pandas has a less brittle (but still convoluted) approach. I will dig into that.

@danielballan: I don't understand why you think what this project does is brittle. The Makefile is for PyAV authors to generate the C source, and is not touched by the end user. Automatic module discovery aside, it is just C sources.

@mikeboers
Copy link
Member

As for dist.Distribution, that is basically a path to the setuptools parameter install_requires, which is widely used. But there are a few cleaner ways to implement it -- leaving that egg sitting in the source directory is not what we want. Once I'm sure I've found the best way, I'll update the PR.

@danielballan: How about just using install_requires? We would have to have the building of the Extension objects and pyav.h header conditional on pkg_config, but we could enforce what with a tiny extension to the build_ext distutils command.

@danielballan
Copy link
Contributor Author

Fair questions.

About the Makefile: PyAV authors aren't the only ones who might want to generate the C source. My first encounter with PyAV was trying python setup.py develop, anticipating that I would dig into the source once it was up and running. As I reported above, the build "succeeds" but doesn't build the sources. This is obvious once you take a close look, but it was confusing to me, and it can make packaging the software in other ways (conda, HashDist, etc.) convoluted. So, in addition to making sure the installation raises if the sources aren't built, I'd like our solution -- however we do it -- to enable python setup.py install and python setup.py develop to build the sources if they haven't already been built.

About install_requires: OK, that sounds like a good suggestion. I may have over-thought that part of things.

@mikeboers
Copy link
Member

I think the reason I went with a Makefile instead of the cythonize method was that in my very basic tests all of the C sources were being generated on every invocation instead of just when they changed, which starts being a real pain with so much code.

But I am more than happy to investigate it again. Especially because you raise very good points about being a good packaging citizen.

On Jul 21, 2014, at 9:49 AM, Dan Allan notifications@github.com wrote:

Fair questions.

About the Makefile: PyAV authors aren't the only ones who might want to generate the C source. My first encounter with PyAV was trying python setup.py develop, anticipating that I would dig into the source once it was up and running. As I reported above, the build "succeeds" but doesn't build the sources. This is obvious once you take a close look, but it was confusing to me, and it can make packaging the software in other ways (conda, HashDist, etc.) convoluted. So, in addition to making sure the installation raises if the sources aren't built, I'd like our solution -- however we do it -- to enable python setup.py install and python setup.py develop to build the sources if they haven't already been built.

About install_requires: OK, that sounds like a good suggestion. I may have over-thought that part of things.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants