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

Explicit package_dir in setup.py considered harmful #10437

Closed
jhermann opened this issue Mar 11, 2015 · 9 comments
Closed

Explicit package_dir in setup.py considered harmful #10437

jhermann opened this issue Mar 11, 2015 · 9 comments

Comments

@jhermann
Copy link
Contributor

Consider this command sequence to install the release candidate.
When I don't do the setup.py fix, I get the import error (see below), else I don't.
Any reason you have the version with the explicit package name?

$ ansible --version
Traceback (most recent call last):
  File "…/.venv/ansible/bin/ansible", line 7, in <module>
    execfile(__file__)
  File "…/bin/ansible", line 36, in <module>
    from ansible.runner import Runner
ImportError: No module named ansible.runner
$ sed -i~ -r -e "s~'ansible': 'lib/ansible'~'': 'lib'~" setup.py
$ python setup.py develop -U
…
$ ansible --version
ansible 1.9.0 (detached HEAD bce4bb2ce2) last updated 2015/03/11 23:41:25 (GMT +200)
…

Also, a test-requirements.txt for pip would be helpful. If you agree, I can do a PR for both.

@abadger
Copy link
Contributor

abadger commented Mar 12, 2015

Change to setup.py Looks good. I'll merge a PR for that.

test-requirements.txt I think would be good but let me CC: @jlaska to make sure that won't step on anything that he's planning.

abadger added a commit that referenced this issue Mar 12, 2015
Generic package_dir mapping in setup.py (closes #10437)
@jhermann
Copy link
Contributor Author

@abadger
Copy link
Contributor

abadger commented Mar 12, 2015

Reopening since there's still the question of test-requirements.txt for feedback from jlaska

@abadger abadger reopened this Mar 12, 2015
@abadger
Copy link
Contributor

abadger commented Mar 12, 2015

Re: Travis: Nice!

@jlaska
Copy link
Contributor

jlaska commented Mar 12, 2015

@jhermann @abadger no complaints here, thanks for the fix!

@jhermann
Copy link
Contributor Author

Hi… which still leaves the open question whether I should do another PR for the changes from 2cdc46d to tip @ https://github.com/jhermann/ansible/commits/devel.

This basically makes hacking/env-setup jobless (or it could be rewritten to use "normal" Python tooling, namely virtualenv + pip), and allows easy Travis integration.

@abadger
Copy link
Contributor

abadger commented Mar 12, 2015

@jhermann: @jlaska is working on a somewhat different approach for travis integration. So for that, probably should hold off, see what he has when we merge it, and then see if there's any pieces that we should merge from what you have.

I will merge your test-requirements.txt now if you want to submit it though :-)

@jlaska
Copy link
Contributor

jlaska commented Mar 12, 2015

@jhermann @abadger thanks! Yeah, test-requirements.txt is a good thing and will help reduce duplication. Thanks for that addition! The .travis.yml should be running tests through tox on all supported python versions. I have a working local copy, but also needed to make a few test adjustments ... and review. I plan to post that for review soon-ish.

UPDATE: tox and travis support added in #10455

@abadger
Copy link
Contributor

abadger commented Mar 12, 2015

test-requirements.txt PR merged as well. Thanks!

Closing This Ticket

Hi!

We believe recent commits (likely detailed above) should resolve this question or problem for you.

This will also be included in the next major release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular
issue is resolved.

Thank you!

@abadger abadger closed this as completed Mar 12, 2015
@ansible ansible locked and limited conversation to collaborators Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants