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

dist creation commands #232

Closed
pytoxbot opened this issue Sep 17, 2016 · 16 comments
Closed

dist creation commands #232

pytoxbot opened this issue Sep 17, 2016 · 16 comments
Labels
area:package-building feature:new something does not exist yet, but should needs:discussion It's not quite clear if and how this should be done

Comments

@pytoxbot
Copy link

in order to support creation/installation of wheels as well as custom distribton generators it seems sensible to add a global option for the sdist creation command

it should default to python setup.py sdist but allow people to override
with something like python setup.py sdist bdist_wheel

in addition a global option is required to check if all requirements for the sdist creation are availiable

i propose to add the keys

dist_command for the command and dist_requires for the requirements needed to create the distribution (this would also allow to add requirements for recent setuptoolsand setup time dependencies)

whether the implementation creates a virtualenv to install those or tells the user to do so is up for implementation

in case of virtualenv creation we can also support specifying exact/desired tox versions there

@pytoxbot
Copy link
Author

Original comment by TonasJ

Yes in fact I just had to do the same - and when I saw tox simply runs an external command with "/usr/bin/python" at the start to do sdist I thought "hum, it would be neat if it was simply possible to tell it to run python3 instead, wouldn't it?".

@pytoxbot
Copy link
Author

Original comment by @The-Compiler

Case in point - I had to make my setup.py python2 compatible when I started using tox (with a Python3-only project) ;)

@pytoxbot
Copy link
Author

Original comment by TonasJ

+1. This seems also very helpful for packages that have setup.py files that only run in Python 3, while tox is commonly installed by linux distributions with Python 2. (rare I suppose, but not unthinkable) Running the sdist command in a virtualenv with the same python versions as the actual tests/allowing to specify the python interpreter for running sdist would solve/prevent this.

@pytoxbot
Copy link
Author

Original comment by robmessick

+1 on this. In our workflow, artifacts are not uploaded to devpi unless the tests pass. We need the source distributions to use .tar as the archive format since .zip doesn't preserve filemode.

@pytoxbot
Copy link
Author

Original comment by @hpk42

not sure i follow. I currently don't consider having tox generate different packages and testing them. rather i'd put this into something connected to devpi.

@pytoxbot
Copy link
Author

Original comment by @RonnyPfannschmidt

setup_requires will run easy_install and create a mess, avoiding triggering that is desirable

a env to create packages seems indeed the best solution
i wonder if we can map artifact extensions to factors (after all the common use case is whl vs sdist)

@pytoxbot
Copy link
Author

Original comment by @hpk42

doesn't a package define with setup_requires what it needs? The main points about making the package creation configurable are IMO:

  • specifying an env where to run the build/packaging command
  • specifying the command to be executed
  • what if multiple artifacts are created?

@obestwalter obestwalter added area:package-building feature:new something does not exist yet, but should needs:discussion It's not quite clear if and how this should be done and removed enhancement labels Sep 4, 2017
@gaborbernat
Copy link
Member

This is now solved by having PEP-517/518 support. We offer sdist for now. Wheel support is encouraged to be added as a tox package plugin.

@ionelmc
Copy link

ionelmc commented Jan 8, 2019

@gaborbernat did anyone work on this wheel plugin? how come the wheel support was dropped (i saw some prs with it but the discussion is confusing)?

@gaborbernat
Copy link
Member

gaborbernat commented Jan 8, 2019

@ionelmc not as far as I'm aware of it. It wasn't dropped I just ended up conceding the benefit for projects I work on would be small. 😄 so it's just a matter of prioritization. Feel free to create a plugin doing this 👍

@ionelmc
Copy link

ionelmc commented Jan 9, 2019

@gaborbernat Ok so I've looked at the internals, looks like tox_package is the hook but it's very coarse - I'd only need to override the build_package function. I mean it's fine for now, i'll just copy-paste tox_package+get_package+acquire_package for now but maybe we want some more granular hooks at some point.

@gaborbernat
Copy link
Member

@ionelmc true that 👍 as a more proof safe implementation I would suggest you just monkeypatch the build_package? Feel free to add me as a reviewer once you have it done 👍

@ionelmc
Copy link

ionelmc commented Jan 9, 2019

@gaborbernat So I've looked a bit more now, so it looks it's really hard to also support wheels for pep517 projects (I'd need to copy-paste perform_isolated_build basically).

Also, isolated_build=true doesn't work without pyproject.toml - is that intentional?

@gaborbernat
Copy link
Member

gaborbernat commented Jan 9, 2019

Also, isolated_build=true doesn't work without pyproject.toml - is that intentional?

PEP-517 requires PEP-518 which mandates pyproject.toml so yeah that's by PEP design.

@gaborbernat So I've looked a bit more now, so it looks it's really hard to also support wheels for pep517 projects (I'd need to copy-paste perform_isolated_build basically).

What do you mean really hard? You can try to inspire from perform_isolated_build but a wheel build is a different packaging system per se. If you mean hard in sense of you need to write 100+ lines and have a lot of edge cases, then yes, that's why we decided to not add it to the core yet.

@ionelmc
Copy link

ionelmc commented Jan 9, 2019

What do you mean edge cases? From my perspective having builtin support in tox is way simpler.

@gaborbernat
Copy link
Member

gaborbernat commented Jan 9, 2019

For universal wheels yes, however in case of C extensions wheels things get a lot more complicated as you need to build multiple wheels depending on the target you're planning to test. This complicates things quite a lot, hence why we haven't gone down this path yet. I've had the first go at it inside #852, but decided against it because of these impediments. I'm not saying it's not possible, however, it's non-trivial, so I would like to first have it as a plugin. Once with the plugin, we figure out all the edge cases, we can merge into the core. And yes, in the plugin feel free to copy as much code as needed from the core, we can unify things once we merge it into master.

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:package-building feature:new something does not exist yet, but should needs:discussion It's not quite clear if and how this should be done
Projects
None yet
Development

No branches or pull requests

4 participants