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

setup.py changes for 2.0 #4975

Merged
merged 12 commits into from Feb 5, 2014
Merged

setup.py changes for 2.0 #4975

merged 12 commits into from Feb 5, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Jan 31, 2014

  • always compute 'requirements'
  • use platform-specific dependencies (for wheels)
  • add bdist_wheel subclass for getting the right tags and requirements
  • whitelist components to be installed

The main point of this PR:

IPython source distributions are growing, due to the inclusion of many javascript dependencies. A large portion of these files are not actually used at install time, so a bdist (wheel) can be much smaller than an sdist. Currently, testing this PR shows a wheel being 3.3MB, while an sdist is 9MB.

Notes:

This should be rebased after #4893 is merged, since it moves font-awesome.

We are currently only shipping manpages to usr/share, but the function that does this also claims to install examples, but it doesn't because the search path is wrong. I removed the failing code,
but I could just as easily fix it to actually install examples, if that is what we want it to do.

Still testing to do, to make sure the whitelist isn't too exclusive.

The ickiest part of the changes required is mangling Requires-Dist, for turning old-style requirements into PEP-345 Requires-Dist, which is the only way to specify platform-specific dependencies. Only wheels actually support PEP-345, so we have to specify these requirements in at least two places. This was the smallest change I could come up with, with the minimal code duplication.

Bonus wtf points for discovering that PKG_INFO is an instance of email.Message (?!)

@takluyver
Copy link
Member

#4893 is in - Min, do you want to rebase and update the location of FA?

# setuptools requirements

extras_require = dict(
parallel = 'pyzmq>=2.1.11',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be cleaner to just make these always lists, even if they have just one element?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably

@takluyver
Copy link
Member

Besides the expected distutils mess, I think this looks OK.

and only check for it on posix
results in 'py2-none-any' instead of 'py27-none-any'.

This is supported by pip and PEP 425, but not available from bdist_wheel by default.
we don't need most of what's in components to be installed.
Much of it is included just for sdists or development.
let the function reflect this, rather than having several entries with no files.
because an empty distutils command must implement three no-op methods and one class attribute.

Which makes perfect sense.
and fix errors caught by the validation
simpler, that way
@takluyver
Copy link
Member

Min's dealt with my suggestions, so I'm merging this now so people have time to spot any flaws in it before release.

takluyver added a commit that referenced this pull request Feb 5, 2014
setup.py changes for 2.0
@takluyver takluyver merged commit d4d34f7 into ipython:master Feb 5, 2014
@minrk minrk deleted the t2pp branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants