-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Use setuptools in the Python builder #288
Conversation
I'm starting to remember some of the things I dislike about setuptools. Buildbot is reporting the following error when running
|
Right, so what should we do about that? On one hand, it might eliminate some user-board posts if people are forced to set everything up properly before they install (install to a proper directory or add the proper directories to PYTHONPATH). I can confirm that the installation works for the USER folder on Ubuntu and with the MSI's on Windows. On the other hand, this behavior is really annoying and having to hack in a fix just to get the buildbot to work doesn't seem useful. The most direct hack would be to override |
Well, we know the target installation directory in SCons, right? So I think we could just append that to the PYTHONPATH before running the setup.py install command. This isn't just a problem with the buildbot. It's a problem with any installation where a path is explicitly set for the Python module. This will probably also break building the Debian/Ubuntu packages, since those rely on installing to a stage directory which then gets rolled up into the .deb file, but that stage directory is of course never actually on the PYTHONPATH. I don't think there's anything improper about installing a module to a location not on the PYTHONPATH, despite what setuptools thinks. Sure, you need to add it to the PYTHONPATH before you can import the module, but that could be handled any number of ways. |
OK, I think we only need to add to BTW, when/how does the buildbot run? |
Yes, I agree that the logic with respect to the
The buildbot runs on commits to the Github |
On Windows, something screwy happens when the prefix is specified into Actually, in reading the docs related to this: https://pythonhosted.org/setuptools/easy_install.html#custom-installation-locations, it seems like the better option would be to set the
The disadvantage is that setuptools will install the package as an egg, so the egg directory (instead of the This seems like a lot of machinations for a really simple feature, i.e., executable scripts on Windows. Maybe it would be easier to just write a I just pushed some code that implements the |
Of course, I just noticed that on Windows using the That said, does anyone actually use the FWIW, specifying the |
I would like to avoid installing as an I guess the scripts should be installed to I've thought before about trying to get binaries distributed on PyPI, but I'm not sure how feasible that is. As far as I can tell, the new binary "wheel" format only really works for Windows, and don't know if that's any better than the MSI installers we already provide. Yes, if and when this is ready to merge, some rebasing and squashing is definitely warranted. The approach I've been taking for merging pull requests is to ignore the "merge pull request" button, and instead use the link for "command line instructions" to get the "patch" URL, so you can then do:
And then do whatever fixups / interactive rebasing you like before pushing the changes back. |
OK, the However, I cannot figure out how setuptools determines the path to install the Windows I guess I'll try to create a really simple test package and try to figure it out more directly. Then I could also ask about it on StackOverflow... |
OK so it seems like this is a bug in |
@speth can you please check that I added the right license classifier to setup.py.in? Also, I think it would make sense to add the license somewhere in the docs, both the actual license text and a generic description of what it allows people to do. |
Also, regarding the installing into the For example,
would import that version of |
Yes, I believe that is the correct license classifier, given that it doesn't seem to distinguish between the different flavors of BSD license. What are the "Programming Language" classifiers supposed to mean? Are they an indicator if implementation language or languages the package can be used from? If you want an Egg for the Conda recipe, we can just add a SCons option to do that and leave the default behavior as it is. I think doing |
I'm not sure... The complete list is here: http://pypi.python.org/pypi?:action=list_classifiers. I poked around on PyPI and it seems like most people choose the classifiers that are associated with the project in some way, whether by being written (partially) in that language, or just being related to that language. FWIW, I added the classifiers mostly for our internal use, on the principle that "documentation is better than no documentation." We could probably delete them though, if they're wrong/unnecessary, on the principle that "incorrect documentation is worse than no documentation." |
The buildbot (http://gir.mit.edu:8010/waterfall) now additionally follows the branch There is an issue with the OS X build, which gives the error:
Which is apparently a problem with Homebrew Python (see Homebrew/homebrew-python#187) that apparently has a workaround also setting an empty value of |
Add functions to ck2cti, ctml_writer, and a new file __main__.py to mixmaster to be the entry_points locations that setuptools scripts will call.
Replace distutils with setuptools in the Cython and python_minimal interfaces. Add console_scripts option to generate OS specific scripts to run ck2cti, mixmaster, and ctml_writer
Conform to 4 space tabs. Add classifiers about Cantera. Add long description.
Remove script files that are obsoleted by console_scripts from setuptools. Remove installation of the script modules from SConstruct.
Fix Python installers so that when a prefix directory is specified on the command line, setuptools doesn't throw an error. The setuptools documentation at [1] prefers setting PYTHONUSERBASE rather than PYTHONPATH. Use normpath to avoid bugs in setuptools on Windows [2]. Specify an empty "--prefix" if the compiler is clang to fix a bug with Homebrew Python on Mac OSX [3]. [1]: https://pythonhosted.org/setuptools/easy_install.html#custom-installation-locations [2]: http://stackoverflow.com/q/31629398 [3]: https://github.com/Homebrew/homebrew-python/issues/187
70e376e
to
9ffa722
Compare
OK I think I fixed the error, although I'm not sure that the check I do is appropriate. There is currently a build error with "ubuntu-1", but it exists on the master branch build too, so I don't think its anything that I did. I rebased these commits against the current master (6574117) so a merge should be pretty easy. Thanks for all your support/code review! |
The errors with the Ubuntu build were my fault (since fixed). I changed the test for OS X to look for Merged as 0b5a46f...2334410. |
Resolves #283