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

test.py relies on python #255

Open
ignatenkobrain opened this issue Aug 12, 2018 · 6 comments
Open

test.py relies on python #255

ignatenkobrain opened this issue Aug 12, 2018 · 6 comments

Comments

@ignatenkobrain
Copy link

Please use sys.executable instead.

@evanunderscore
Copy link
Collaborator

evanunderscore commented Aug 15, 2018

What problems is this causing for you? Those parts are exercising the global completion code that is matching on python* and pypy* so I don't think they would work with the full path from sys.executable.

(As you can see by the test failures in #257)

@evanunderscore
Copy link
Collaborator

@hroncok Are you able to comment on whether this has been an issue for you when packaging for Fedora?

@hroncok
Copy link
Contributor

hroncok commented Apr 10, 2019

Somehow. We have a workaround:

sed -i -e "1s\|#!.*python.*\|#!/usr/bin/python3\|" test/prog scripts/*
sed -i -e "s\|python \|python3 \|" test/test.py

This makes sure that our packaged python3-argparse RPM is only used for our python3 things.

@evanunderscore
Copy link
Collaborator

I have a branch which should fix these issues here: https://github.com/evanunderscore/argcomplete/commits/dynamic-test-shebang

It has a few problems:

  1. Moving those scripts is rather disruptive to the code base, so I'd want @kislyuk's blessing before doing something like that.
  2. The package now has to be installed in order to run the tests since it needs the console_scripts to be created (it could previously read them straight from the scripts folder).
  3. It's difficult to work this into the CI in a way that we'd know if we broke it again in future (all of the current jobs seem to invoke the interpreter as python even when using python3).

@hroncok Do you think it's worth pursuing this, or are you happy to keep seding the code?

@hroncok
Copy link
Contributor

hroncok commented Apr 13, 2019

I think that it's worth pursuing for multiple reasons*, however I'm not sure if this is the best way. I haven't yet read the code and will not have the tiem to do it until next week.

Wearing my Fedora, we can live with the current solution. If you only motivation would be to make our packaging job easier (thanks!), it is probably not worth the effort.

* I'm not quite sure how argcomplete is supposed to work in "multiple Pythons installed" environment.

@evanunderscore
Copy link
Collaborator

Until recently, the fact that anything under scripts/ ran in Python was an implementation detail, and provided that you could run python they would work. Unfortunately I didn't notice that #237 now means that register-python-argcomplete won't work unless argcomplete is installed in whatever interpreter python refers to.

So it seems like we now have two problems:

  • Systems don't necessarily have python installed any more
  • If they do, argcomplete isn't necessarily importable there

I agree these are probably worth fixing. Once you get time to take a look over my branch and confirm the changes work in your environment, we can go from there.

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

No branches or pull requests

3 participants