something unspecified goes wrong using infi.recipe.application_packager with buildout + mock #310

Closed
wiggin15 opened this Issue Aug 5, 2015 · 8 comments

Comments

Projects
None yet
2 participants

wiggin15 commented Aug 5, 2015

According to the documentation, setup_requires is used for packages the need to have dependencies installed before 'setup' is run, usually for packages that have distutils extensions that need to be compiled. install_requires is the better choice for declaring dependencies.
See https://pythonhosted.org/setuptools/setuptools.html

Owner

rbtcollins commented Aug 5, 2015

We use install_requires for our runtime requirements and setup_requires for our build time requirements, as setuptools intends.

@rbtcollins rbtcollins closed this Aug 5, 2015

wiggin15 commented Aug 5, 2015

I'm sorry, but this isn't what setuptools "intends".
You use setup_requires in setup.py. Please read the documentation I linked, or refer to what other pure-python packages are using. You should be using install_requires in setup.py, not setup_requires. Please reopen this ticket.

Can you please take a closer look?

Owner

rbtcollins commented Aug 18, 2015

Why? I answered your question. I think you don't understand how setuptools plugins such as pbr work - we specifically do use runtime requirements, correctly. You can check the egg metadata that is produced to verify this.

If you've encountered an actual bug please do share it so we can figure it out and fix it for you.

I asked you to take a closer look because if you take a moment to read the documentation I linked or look at other Python packages, you'll find that although both 'install_requires' and 'setup_requires' work, 'mock' should use the former and not the latter.

setup_requires is used only for binary packages that need to be compiled and require the dependencies to be installed prior to running setup.py. This is not the case in mock. The documentation of setuptools clearly state that install_requires is the correct way to declare dependencies, even with examples. Please please read it:
https://pythonhosted.org/setuptools/setuptools.html

I looked at the top 20 packages and all those that have dependencies use install_requires because that is the correct way. Here are some examples:
https://github.com/dateutil/dateutil/blob/master/setup.py#L35
https://github.com/mitsuhiko/jinja2/blob/master/setup.py#L70
https://github.com/pika/pika/blob/0.9.14/setup.py#L32
https://github.com/paramiko/paramiko/blob/master/setup.py#L43
https://github.com/pyca/pyopenssl/blob/master/setup.py#L116
... and practically any other non-binary Python package.

You mentioned pbr. From a quick look I see that pbr uses C extensions (ext_modules), so they may need this for compilation. mock does not.

This is an issue because setup_requires confuses our build process. We are using buildout and a recipe called infi.recipe.application_packager, which read setup.py, and treat packages that use setup_requires as binary packages that should be compiled and be installed differently (they do the right thing by trying to install the packages in setup_requires before running setup.py, but they shouldn't do that for mock).

Owner

rbtcollins commented Aug 18, 2015

I keep repeating that mock uses both setup_requires and install_requires. The install_requires is not apparent to you because its injected via pbr. Its still in use. Your statements that mock does not use it are factually incorrect. Linking to documentation I'm thoroughly familiar with doesn't change that fact.

Thank for you finally describing the issue you're having though. Excellent! That buildout recipe has a design bug: you cannot infer the correct behaviour of a setuptools using package statically today, you need to actually run it.

That said, installing pbr and setuptools versions as specified in mock's setup.py is appropriate to do before installing mock, because if you do not, it will download them on demand from the internet, or may fail to install if you have incompatible versions locally installed. So I think your recipe is doing the right thing: installing the dependencies that mock's setup.py needs before running. Well done.

In other words: your tooling is right - mock needs to be treated specially by your tooling, because it declares a setup_requires (and it does correctly do so).

Is mock failing to install for you? Or failing to run?

@rbtcollins rbtcollins reopened this Aug 18, 2015

@rbtcollins rbtcollins changed the title from use install_requires instead of setup_requires to something unspecified goes wrong using infi.recipe.application_packager with buildout + mock Aug 18, 2015

The recipe creates packages for installation on different operating systems - deb, rpm, msi etc.. The msi packages now fail to install, without a specific error message. It's hard to debug the msi but it looks like it is caused because the recipe packages mock as a bdist_egg instead of an sdist (because of the use of setup_requires). mock 1.0.1 works.
I understand now that pbr requires using the setup_requires mechanism. This may be helpful for some packages - I don't know enough about pbr. In the case of 'mock' it only adds noise.
Debugging the msi proved harder than simply pinning mock to 1.0.1, so I'm afraid I don't have further details about the problem. I hoped that switching to install_requires will be a solution.

It looks like our tools don't match so it's ok if you re-close this as "won't fix", and we'll either stop using mock or fork it to not use pbr.

Owner

rbtcollins commented Aug 19, 2015

Ok so - like I keep saying. mock via pbr is using install_requires. It sounds like your packaging toolchain has a bug - you might be a lot better off building a wheel and then writing something to wrap wheels in .msi files. mock builds a universal wheel reliably.

I don't know why setup_requires implies using a bdist_egg - but thats probably the thing to fix, IMO.

pbr doesn't 'require' using setup_requires - as long as one is willing to tell folk to install pbr before installing the package, but that a poor user experience, so we use setup_requires. The actual glue into the hook point happens through the setuptools entry_points which pbr implements, plus the use of the pbr=True keyword to setup().

Its possible your msi failure is due to the bug in pbr 1.4 that required git to be present - you might try anew with pbr 1.5 and see if that fares better.

However, I think we've established that there isn't actually a bug in mock other than, perhaps, the use of pbr - but pbr is generally low impact and saves a raft of developer time, so I'm loath to go through the effort implied by ditching it at this stage, since mock's development is entirely backporting from cPython trunk these days.

@rbtcollins rbtcollins closed this Aug 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment