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

Don't import the package to get the version #1

Closed
pydanny opened this issue Aug 9, 2013 · 17 comments
Closed

Don't import the package to get the version #1

pydanny opened this issue Aug 9, 2013 · 17 comments

Comments

@pydanny
Copy link
Collaborator

pydanny commented Aug 9, 2013

Apparently, this can cause issues. Not clear how, but there you are. Here is a mitigation:

https://github.com/jezdez/envdir/blob/master/setup.py#L23-L33

@audreyfeldroy
Copy link
Owner

Is there any documented proof of this? @jezdez or @dstufft, got any links to an explanation?

@dstufft
Copy link

dstufft commented Aug 11, 2013

The explanation is that setup.py is ran prior to the dependencies being installed (in fact it is ran to determine what the dependencies are). So if, for example, you're writing a library called foo, and doing import foo requires the "requests" library to be available, it will fail to an import error because the installer cannot know what is available prior to running setup.py If you're able to import whatever you're attempting to import without any requirements besides what's in the package and the standard library it will work alright.

That being said I have a slightly nicer trick that involves putting an __about__.py file that I exec(open().read()) in my setup.py, and from __about__ import * in my __init__.py. See here: https://github.com/dstufft/bcrypt/blob/master/setup.py#L33-L36

@audreyfeldroy
Copy link
Owner

@dstufft Thanks, this is very helpful.

@jezdez
Copy link

jezdez commented Aug 11, 2013

@dstufft WTF? I don't think this is "nicer", exec'ing an arbitrary file in a setup.py is contrary to how metadata like a version number should be handled. It's like you're asking for trouble. I find it funny you're doing that in your crypto package's setup.py.

@audreyr I would strongly recommend using a safe approach like json.loads with a local JSON file or a parser like approach like my regex solution linked above.

@dstufft
Copy link

dstufft commented Aug 11, 2013

@jezdez shrug, at that point you're already executing arbitrary python (setup.py). Not like adding a little bit extra is any more or less dangerous. I prefer to keep the business of tokenzing and parsing a .py file to the python interpreter instead of attempting to create a regex to parse it out. A json.loads would work, but then if you want import foo; foo.__version__ to work you pay the price of json parsing on every import without a bytecode cache for it.

@Nekroze
Copy link
Contributor

Nekroze commented Aug 12, 2013

@dstufft I agree. By the time it gets to setup.py anything could be run there anyways.

The original problem seems to be solving the importing dependencies before they are installed. Why can't we just have the foo.__about__ module that just has the version, author name, whatever else, and just do from foo.__about__ import __version__ in setup.py and else where. This way its the same usage to get the version, for example, everywhere.

This should avoid the imports that could be in foo.__init__ because there is no good reason to put an import into foo.__about__ and still get the data without reading a file specifically.

@dstufft
Copy link

dstufft commented Aug 12, 2013

from foo.__about__ import __version__ will also execute foo/__init__.py. You have to use the exec trick.

@Nekroze
Copy link
Contributor

Nekroze commented Aug 12, 2013

@dstufft Ah, i wasn't sure on that one. My bad, rescinded.

@jezdez
Copy link

jezdez commented Aug 12, 2013

@dstufft So your argument is that adding another way of how setup.py can be abused is not a problem? I beg to differ, any extension of what setup.py does (other than build the kwargs for the setup function locally) further solidifies the antipattern setup.py itself is. I'm sad you don't seem to realize that the best practices you're advocating here and in other places are the exact reason we're still stuck with setup.py after years of PEPs with new metadata standards -- making them toothless tigers at best.

I hoped PEP 426 would encourage a new non-executable metadata file to be advocated instead of using hacks like using exec or my regex solution. But if one of the coauthors of the PEP even thinks that it's fair game to ignore this new effort, I'm not sure I want to even be part of this discussion anymore. Since all we get instead is more pseudo standards that make the PEP system laughable.

For the others -- and for completeness sake -- there is also d2to1 which was supposed to be the transitional package for projects supporting both distutils and distutils2/packaging (when distutil2/packaging was still a thing). https://pypi.python.org/pypi/d2to1

I strongly believe that a similar solution should be created for the metadata standard that was proposed with PEP 426.

dstufft referenced this issue in pyca/cryptography Aug 12, 2013
@dstufft
Copy link

dstufft commented Aug 12, 2013

@jezdez PEP426 has nothing to do with how the metadata is derived simply what it looks like inside of the packaged file. A big part of it's goals is to enable any type of source for the data as long as whatever you use to build the package generates a package with the proper JSON file.

Their is no sdist 2.0 yet, however there is Wheel. So the way that it is expected to migrate is that setup.py will be used to create a Wheel file which will (eventually) have a pymeta.json file inside of it. So as long as the setup.py is deterministic it will always create the same Wheel file and anything you write inside of it is fine. Once there is a sdist 2.0 it will similarly work by not having a setup.py inside of it but instead having a pymeta.json and will similarly work fine as long as setup.py is deterministic.

The real power behind the approach of defining how a package looks versus what the developers of a program interact with is that it enables developers to generate the packages however they want as long as it ends up with the simple package file with static metadata. So that means continueing to use a setup.py file that simply generates a sdist 2.0 and a Wheel file (possibly using exec to pull values out of an __about__.py) is perfectly valid as in the typical use case the only place that setup.py will get executed is on the developers own machine. It also means that somebody else can use a different tool to create packages that looks at setup.cfg for example as distutils2 proposed and generate a similar package again using a pymeta.json in the built packages.

So I look at having setup.py as a reasonable thing to do in setup.py's role as a developer oriented build script. In it's role as an entry point for sdist 2.0 metadata I look at it as merely an extension of existing brokenness but one that is easily replaced with no effort of the developer once setuptools is able to generate sdist 2.0 files as easily as it is replaced in Wheel files today.

@dstufft
Copy link

dstufft commented Aug 12, 2013

In other words I find the conflation of the developer oriented build tooling that creates packages and the meta-data inside of the package to be one of the problems with the existing solutions and with distutils2. I also don't find setup.py itself to be a bad thing (although I don't think it's the best possible thing) for a developer to run on his machine, but I do think it's a horrible thing for the packaging formats to standardize on.

@jezdez
Copy link

jezdez commented Aug 12, 2013

@dstufft Yeah, we disagree in that regard. I detest setup.py files with the heat of thousand suns (and I've written a lot of them) because it not only allows as you say flexibility for developers how to create packages but actually forces them to think about packaging as a part of the development. But packaging and deployment problems are unrelated to the problems developers solve with their software, so why should they even care? Who decided that developers ought to be able to fiddle with how a language level packaging tool creates a package?

The conceptual discrepancy between a pretended choice of implementation and the real world requirements of an easy and simple packaging standard, led developers to opt for relying on cargo culted shims instead of clearly defined metadata standards. Why can't we store the metadata in a file that is machine readable and can be created agnostically with whatever tool you like?

Don't get me wrong, @dstufft, I've had this discussion about the best Python packaging pattern a hundred times and always had to reply "it depends". I've just had to iterate over my "solutions" too many times and have contributed to the cargo culting too often. It's a broken system, one that hurts Python. And I'm not willing to keep silent about this now that we're making progress in other parts of the packaging stack.

@dstufft
Copy link

dstufft commented Aug 12, 2013

@jezdez "Store metadata in a file that is machine readable and can be created agnostically with whatever tool you like" is exactly what PEP426 metadata is and setup.py is a perfectly valid tool for creating that file.

If you take the method I suggested and create a Wheel there will be zero setup.py and people installing that file will be getting the metadata from a static source. The same will be true with sdist 2.0. In the future with zero changes on the developer machine besides upgrading to a newer version of setuptools but with no code changes this method (or any method that relies on code in setup.py) will continue to work just fine and will have a static meta-data file inside the actual packages with setup.py becoming similar to make.

The benefits of my system is that most of the metadata is easily introspectable from a REPL, and is available for easy programmatic access without needing to depend on pkg_resources or distlib.

@jezdez
Copy link

jezdez commented Aug 12, 2013

@dstufft Sorry but you just don't seem to get what I'm saying. Why do we need a shim around creating the metadata file if the developer of a package can create the file directly next to the package? Why is the standard not proposing to let me write the pydist.json file?

@dstufft
Copy link

dstufft commented Aug 12, 2013

Well that's an option, but I was strongly against making that the requirement. The things you want from a file designed to be parsed by machines to determine the metadata (and figure out how to install a particular package) is different than the things you want from a file designed to be user facing. Just because they contain the same data doesn't mean the same format is appropriate. The pydist.json (and other generated files) are designed to be far more verbose and are optimized for machines to create it and read it.

To be frank I don't think having humans edit a pydist.json to be a very good API. It's common to want to include at least the version information encoded inside of a __version__ variable at the top level of the package namespace and if the user facing API for package creation forced the use of a json file then it would require duplication of version data. More importantly pushing the editing of a pydist.json as the only way to do it removes peoples ability to do something different then what we anticipated originally. A good example is the common practice of reading in the README.rst into the long_description. If the pydist.json did not have provisions for that then those users would be SOL.

However the standard does let you write a pydist.json if you want, however you'll need to create your own tool that pulls that in and creates the appropriate package formats. The plan as of right now is to primarily support using setuptool as the tool that creates packages but the use of a static pydist.json in the package format makes it simple for anyone to create their own tool to create packages with whatever workflow makes sense for them.

@audreyfeldroy
Copy link
Owner

Hey, I just wanted to say that this discussion has been very helpful to me, thanks.

If you're curious, after considering the various setup hack options, I'm going for the simple setup: changing the setup.py created by cookiecutter-pypackage to simply version='0.1.0' or whatever the user supplies as cookiecutter.version initially.

Having the ability to generate a pydist.json would be nice for various reasons.

I'm still learning how wheel works, but once I figure it out, I can update this if needed. Likewise with any other packaging changes.

@merwok
Copy link

merwok commented May 2, 2014

FTR another common work-around here is to use lower-level functions from the imp module to get the info from a package/_version.py file that contains just one assignment. It amounts to the same as Donald's read/exec trick though; maybe people like it because it feels a little smarter and a little less dirty (but isn't really).

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

No branches or pull requests

6 participants