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

Added 'bdist_ext' cmdclass implementation #171

Merged
merged 11 commits into from
Oct 9, 2020

Conversation

pkittenis
Copy link
Contributor

For modules with native code extensions, versioneer needs a build_ext command class implementation to support wheels and other binary distributions.

Added integration tests for module with extension for setuptools and pip.

Resolves #170

…e extensions. Added integration tests for module with extension for setuptools and pip.
@pkittenis
Copy link
Contributor Author

The _editable and _develop test failures look unrelated and also happen on master branch.

Copy link

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

Current master branch accepts a dictionary for the get_cmdclass call, perhaps update th PR for that too?

@pkittenis
Copy link
Contributor Author

Hi @s0undt3ch - not sure I understand what you mean. There have been no changes to get_cmdclass. It is used only in tests, same way as in other tests.

Can you please clarify what changes are needed?

@pkittenis
Copy link
Contributor Author

Oh right, sure, thank you.

@pkittenis
Copy link
Contributor Author

Changes requested have been made - ready for review.

@effigies
Copy link
Contributor

Hi @pkittenis. We're working on reviving this project. Can you rebase/merge master to get the CI working?

@pkittenis
Copy link
Contributor Author

  • Updated for latest master.
  • Updated cython source for py 3.8 compatibility.

@pkittenis
Copy link
Contributor Author

Merged and cleaned up changes.

@effigies
Copy link
Contributor

Thanks for doing this. Out of curiosity, can the .c file be generated on the fly, rather than including it directly in the repository? I'm not terribly familiar with cython, so this may be a dumb question.

@pkittenis
Copy link
Contributor Author

It can, but it would require Cython as a dependency, hence why it is included.

Copy link
Contributor

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Overall this looks reasonable to me. We've got some future-proofing to worry about with wheel, and I've made suggestions. They require you also to add packaging to the testenv:

[testenv]
# virtualenv>=14.0.0 is incompatible with python 3.2
# flake8>=3.0.0 and pyflakes>=2.0.0 are incompatible with 2.6,3.2,3.3
deps =
pyflakes
flake8
flake8-docstrings
wheel
setuptools
virtualenv<20
pep8

test/git/test_invocations.py Outdated Show resolved Hide resolved
test/git/test_invocations.py Outdated Show resolved Hide resolved
test/git/test_invocations.py Outdated Show resolved Hide resolved
@effigies
Copy link
Contributor

It can, but it would require Cython as a dependency, hence why it is included.

Fair enough. A couple KB shouldn't make a huge difference. If we're going to need to re-render it periodically, it might be worth figuring out how to get a Cython dependency into the virtualenvs.

@effigies
Copy link
Contributor

effigies commented Oct 6, 2020

@pkittenis Sorry to bug you, but could you check out my suggestions? If you don't have time, I can create a PR based on this.

@pkittenis
Copy link
Contributor Author

Thanks, yes, will take a look.

pkittenis and others added 4 commits October 9, 2020 14:32
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@pkittenis
Copy link
Contributor Author

pkittenis commented Oct 9, 2020

Re: cython - I'd like to hand write a simple extension function so this can be tested without needing cython.

The use of cython was more to make it compatible with future Python versions, but a simple enough native extension should remain compatible with future C-API versions.

@effigies
Copy link
Contributor

effigies commented Oct 9, 2020

Pushed a fix to the testing environment. I'm going to go ahead and merge. Thanks for this!

@effigies effigies merged commit f4b99f0 into python-versioneer:master Oct 9, 2020
@effigies effigies added this to the 0.19 milestone Oct 9, 2020
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.

Wheel packages with extensions do not get static version file
3 participants