jordansissel / fpm

Effing package management! Build packages for multiple platforms (deb, rpm, etc) with great ease and sanity.
http://fpm.readthedocs.io/en/latest/
Other
11.16k stars 1.07k forks source link

Python: requirements.txt shouldn't take precedence over the install_requires metadata #308

Closed brutasse closed 11 years ago

brutasse commented 11 years ago

Currently fpm uses requirements.txt as dependency information if it finds such a file, whatever metadata it finds when running setup.py get_metadata. This approach seems a bit too optimistic: requirements.txt can be used for different things:

In some cases, install_requires is dynamic and depends on the version of python being used. So there are few cases where requirements.txt actually contain what needs to be installed as a package dependency.

To give an example, Fabric has the install_requires metadata and specifies its development dependencies in requirements.txt: https://github.com/fabric/fabric/blob/master/requirements.txt

I end up doing weird things to package this in an proper way:

FABRIC_VERSION="1.5.1"
wget http://pypi.python.org/packages/source/F/Fabric/Fabric-$FABRIC_VERSION.tar.gz
tar xvzf Fabric-$FABRIC_VERSION.tar.gz
rm Fabric-$FABRIC_VERSION/requirements.txt
fpm -t deb -s python -v $FABRIC_VERSION -d python-paramiko Fabric-$FABRIC_VERSION/setup.py

(the -d python-paramiko might not even be needed here since setup.py has that information).

When installing a python package with pip, the package manager doesn't care if the package contains a requirements.txt. The information is only extracted from setup.py.

I'd say this feature should just be dropped, but I might be missing some use cases. What was the reason to include this in the first place?

(related: #261 should probably be closed).

brutasse commented 11 years ago

Any opinions on this? requirements.txt really isn't a packaging standard in Python and this behavior of FPM leads to unexpected results. IMO these lines should be removed: https://github.com/jordansissel/fpm/blob/master/lib/fpm/package/python.rb#L182-L199

jordansissel commented 11 years ago

Some projects, from what I can tell, only specify a requirements.txt - pip documents its purpose: http://www.pip-installer.org/en/latest/requirements.html - it's not strictly for "development requirements"

As for it "packaging standards" I can't comment on that. There are "standards" and there are things people use in the wild, and I have many observed encounters with requirements.txt files ;)

That said, the intent of requirements.txt and the actual usage of it may not necessarily be the same things.

I'm open to adding a flag that disables (or enables, whichever should be the default) requirements.txt consumption by fpm.

brutasse commented 11 years ago

I'll explain a bit more the context of this, sorry if I'm stating the obvious but I hope to make things a bit clearer :)

Python packages are specified using a setup.py file. This file declares various metadata information about the package, and is the only file taken into account by packaging tools when installing python packages. When I pip install Fabric or easy_install Fabric, the packaging tool (pip or easy_install) looks at the install_requires parameter passed in the setup.py. To keep the example of Fabric: https://github.com/fabric/fabric/blob/master/setup.py#L40

Now setup.py being just a python script, it is easy to load external resources. Some projects write a requirements.txt file which is easy to use with pip for development environments and load that same requirements file in the setup.py. I do that myself, e.g. https://github.com/feedhq/feedhq/blob/master/setup.py#L8-L12

For Python installers, setup.py is the only source of information. If there is a requirements.txt file in the package but that file is not loaded and passed to setup.py's install_requires, installers do not take that file into account. With Fabric, when you pip install or easy_install it, the requirements.txt file is never loaded nor taken into account.

pip install Fabric
<snip>
Successfully installed Fabric paramiko pycrypto
easy_install Fabric
<snip>
Finished processing dependencies for Fabric

Even though Fabric has a requirements.txt file asking for other dependencies, they are never installed by python installers.

The bottom line is, when there is a setup.py (is fpm -s python even able to install things without setup.py files?) it's the only thing that should be taken into consideration. Sometimes reading requirements.txt provides the same result as interpreting the setup.py but this only happens by accident.

From my understanding of the Ruby world, requirements.txt acts like Gemfile / Gemfile.lock and setup.py like the package.gemspec file: gem install only cares about deps from the gemspec (I think), pip install only cares about deps from setup.py.

jordansissel commented 11 years ago

The code itself came at the request of a python user/developer who wanted requirements.txt support, so it's not as though the code was simply added without a feature request ;)

This is simply a case where a feature was speifically requested, and later, someone either doesn't understand why a feature exists or wants the feature removed. This is common because not everyone has the same requirements, use cases, or background!

So my proposal is follows:

The above would align the common case of "pip install Fabric" not obeying requirements.txt (right?) and thus fpm -s python -t deb Fabric should not either if it must download Fabric from pypi.

Further, having a flag explicitly disable requirements.txt support could help the other use cases where it's still enabled by default.

The result should be happiness for all users, regardless of their understanding or needs of requirements.txt support.

Thoughts? Otherwise, this is what I will implement.

brutasse commented 11 years ago

I'd live very happily with a flag to enable/disable requirements.txt :)

My gut feeling is that it should be disabled by default to follow what python installers do… Either way having that switch seems to be the best solution indeed.

r4um commented 11 years ago

Fixed with e252d91 (See #384), there is a --python-obey-requirements-txt flag now, off by default.

brutasse commented 11 years ago

@r4um thanks!